Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project Labyrinth - Johanna Leonsson, Hannah Ek, Sammy Olsson #210

Open
wants to merge 52 commits into
base: master
Choose a base branch
from

Conversation

Nahnahke
Copy link

No description provided.

Copy link

@jonsjak jonsjak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall a very good job! Good looking design and cleeeeeean code! The only remark we've got is about accessibility, that the contrast on the headings (on starterpage) and the action description was a bit small and also a bit hard to see in contrast to the background.

Well done on completing all the goals you set up, yay!

Comment on lines +1 to +9
Project Labyrinth - https://project-labyrinth-jhs.netlify.app/

Replace this readme with your own information about your project.
🏁 Goals:

Start by briefly describing the assignment in a sentence or two. Keep it short and to the point.
In this week's project, we were tasked with building a project which allows a user to navigate a maze, in the form of a text-based adventure, provided by a backend in React and Redux and we have successfully completed what we set out to do.

## The problem
The goal for this week was to use the API specified to build a frontend that gives the user control over what to do next. We focused on using thunks and redux to build the communication with the backend.

Describe how you approached to problem, and what tools and techniques you used to solve it. How did you plan? What technologies did you use? If you had more time, what would be next?
Overall, we found this project to be a great opportunity to practice the use of thunks, bring together different parts of React and working with fetching and posting to an API. We are very pleased with the outcome and looking forward to using the skills and knowledge we gained from this project in future endeavors.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good looking readme. Clear goals and descriptions of how you solved the task.

)
}

export default UserNameInput;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well structured component. Good job on the consistent commenting.


const ActionDescription = styled.div`
color: var(--text-color);
font-size: 13px;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Text a bit too small and difficult to read in contrast to the background.

default: 'https://i.postimg.cc/7LpMWgsh/start.png'
};

const imageUrl = images[coordinates] || images.default;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart way to render images depending on state of coordinates. Impressive syntax!

Comment on lines +74 to +87
<Background imageUrl={imageUrl}>
{isLoading ? (
<LoadingMaze />
) : username ? (
<GameBoard />
) : (
<section className="start-section">
<HeaderText>The Labyrinth</HeaderText>
<EnterText>You are now entering the labyrinth, beware and tread carefully</EnterText>
<UserNameInput />
</section>
)}
</Background>
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very clean and compact way of rendering the specific content. The styled components makes it easy to read and follow what's happening.

Comment on lines +14 to +32
& > div {
width: 15vw; // Set the width of the Lottie animation to 25% of the viewport width
height: 15vh; // Set the height of the Lottie animation to 25% of the viewport height
}

.lottie {
margin: auto; // Center the Lottie animation within its container
}
`;

// Create a component for the loading animation using the Lottie animation library
export const LoadingMaze = () => {
return (
<LottieContainer>
<Player
loop // Set the animation to loop
autoplay // Set the animation to play automatically
src="https://assets2.lottiefiles.com/private_files/lf30_ployuqvp.json" // Set the source of the animation
className="lottie" // Add a class to the Lottie animation for styling purposes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job on the styling! Great job of styled-components making the code very kort och konsis :)

Check the comment on row 15 and 16, it's seems like the comments should say 15% not 25% (Very minor mistake).

useEffect(() => {
store.dispatch(generateGame());
}, [store]);

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this useEffect add to the code? Wouldn't it run without it when the user triggers an event by submitting a username?

Are we missing something?

initialState,
reducers: {
setUsername: (store, action) => {
store.username = `${new Date().getTime()}+${action.payload}`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice way to make username unique. Good job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants