-
Notifications
You must be signed in to change notification settings - Fork 323
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
Labyrinth App Andreas & Peki #197
base: master
Are you sure you want to change the base?
Conversation
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 is a super nice project! The comments and sectioning made it easy to follow. It was cool to see that you've used some other techniques than we did. And I LOVE the design of the images, really cool! Well done!
const coordinates = useSelector((store) => store.labyrinthMango.coordinates) | ||
let imageToShow = null; | ||
|
||
if (coordinates === '0,0') { |
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 was a nice way of displaying different images depending on where in the labyrinth you were. I see that you've set the imageToShow to null first. We returned the whole function (with switch case) as the background image. I'm wondering if this way (that you've done), of making the image a variable, instead might be a better way performance-wise.
// Set user name and generate unique user ID | ||
setUserName: (store, action) => { | ||
const input = action.payload; | ||
const userId = uuidv4(input); |
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.
What does this mean? const userId = uudv4(input)
We only had username but I see you have username, name and userID, how does that work? Is it a way of making the username random?
|
||
return ( | ||
<GameWrapper> | ||
{userName === '' ? <Startpage /> : <Gamepage />} {/* Ternary Operator */} |
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.
Oh, you used username in the ternary operator for rendering the game components. Smart!
export const Gamepage = () => { | ||
// Set up hooks for dispatch and store | ||
const dispatch = useDispatch() | ||
const description = useSelector((store) => store.labyrinthMango.description) |
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.
I see you stored all values in separate properties in the store. We decided to store the current position into one single object, that we called currentPosition. That way, we only needed one useSelector to get that object that held all the information. If you want less code, that might be a possible way of doing it. :)
// /////////////// COMPONENT //////////////////////// // | ||
|
||
export const Loader = () => { | ||
// /////////////// OUTCOME //////////////////////// // |
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.
I like the different sections! It makes it really easy to read the code!!
|
||
// /////////////// GLOBAL //////////////////////// // | ||
|
||
export const GlobalRules = createGlobalStyle` |
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.
I saw that you hade these rendered in the App-component. I've never seen that before? What would be the benefit compared to having these rules in index.css? (Sorry for a lot of questions hahah! I'm just very curious!)
return ( | ||
<GameWrapper> | ||
<CoordImage src={hej15} alt="Displayed" /> | ||
<Protip src={protip} alt="Displayed" /> |
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.
I really LIKE this little floating circle! Reminds me of your portfolio!
{history.length === 0 ? `Welcome, ${name}.` : ''} {/* name comes from the state */} | ||
<p>"{description}"</p> | ||
</DescriptionWrapper> | ||
<DirectionWrapper id={moves.length <= 1 ? 'oneMove' : 'twoMoves'}> {/* styling for 1or2 buttons from gamepage.styled */} |
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.
I'm curious of what is happening here. What is the id? I understand that the styling has to do if there are two or one button.
No description provided.