-
Notifications
You must be signed in to change notification settings - Fork 321
Week 12 Labyrinth project Josefin & Emma #199
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
base: master
Are you sure you want to change the base?
Conversation
Alexander-Gabor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We thoroughly enjoyed and were immensely entertained by the eye-pleasing theme that you chose. The lotties are used in a great way but perhaps it's better to use ones that take less time to load and lighter to display. We noticed some took more time than others to show, and maybe it's a good idea to test them during deployment.
Overall you did a great job and showed a good understanding of the Redux requirements for this week.
| <SpaceBackground /> | ||
| <StartScreen /> | ||
| </Provider> | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We took notice of the neat organization of the components and keeping the App.js readable and concise.
| import GameScreen from './GameScreen'; | ||
| import Loading from './Loading'; | ||
| import GameScreenLotties from './GameScreenLotties'; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Engaging use of the lotties, fitting of the theme and leading the user what to expect. We think using the lotties in more than one place made it super pleasing to the eye.
|
|
||
| // save the username to global state | ||
| dispatch(labyrinth.actions.setUserName(userNameInputValue)); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Global state was a wise choice for the user name, and excellent comments that made us understand the thinking and coding swiftly.
| export default labyrinth; | ||
|
|
||
| // a thunk to handle api call. | ||
| export const getLabyrinth = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good display of thunk understanding.
| }, | ||
| setUserName: (state, action) => { | ||
| state.username = action.payload | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code below would allow the project to work even when using same user name without bugs.
setUserName: (store, action) => {
store.username = `${new Date().getTime()} ${action.payload}`
| dispatch(labyrinth.actions.setCoordinates(gameData.coordinates)) | ||
| dispatch(labyrinth.actions.setDescription(gameData.description)) | ||
| dispatch(labyrinth.actions.setActionOption(gameData.actions)) | ||
| setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not sure and curious about the reasoning behind having these 3 dispatch actions here.
No description provided.