-
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
Project Labyrinth by Annika and Fiona #204
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.
Overall, the game looks good and works well! I really like the design theme. Very cute!
One other comment that I received from my mentor that I'll share with you is about file/folder organization. Where you have the styling in the same document as the code, he recommended having basically individual folders for each page (or page-like thing) and then bundling the files for that page together. E.g. You could have a StartPage folder and in it, you have two separate documents - one that is your StartPage.js and one is StartPage.styles.js. That way you can see your styling separately, but easily. Just a thought!
@@ -13,7 +14,7 @@ | |||
work correctly both with client-side routing and a non-root public URL. | |||
Learn how to configure a non-root public URL by running `npm run build`. | |||
--> | |||
<title>Technigo React App</title> | |||
<title>The Rabbit Hole Labyrinth</title> | |||
</head> | |||
|
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 think I saw Annika post on Slack about og images. I found these two resources to be helpful in generating og tags.
https://metatags.io/
https://postimages.org/
@@ -0,0 +1,135 @@ | |||
import styled from 'styled-components'; | |||
import Background from '../Assets/Mushroom.jpeg' |
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.
Why did you import the image here rather than just include the url on line 10?
display: flex; | ||
flex-direction: column; | ||
justify-content: space-evenly; | ||
height: 100vh; |
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.
If you made the height of the StartWrapper less than 100vh (even just 95vh or 90vh), would it still overlap with the footer?
|
||
const MainPage = () => { | ||
const isLoading = useSelector((store) => store.loading.isLoading) | ||
console.log(isLoading, 'isLoading') |
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.
Just an FYI - this console.log is still active.
align-items: center; | ||
text-align: center; | ||
justify-content: flex-start; | ||
border: 2px solid red; |
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.
Did you mean to leave a red border on the card container?
box-shadow: 0 4px 30px rgba(0, 0, 0, 0.1); | ||
backdrop-filter: blur(3.9px); | ||
-webkit-backdrop-filter: blur(3.9px); | ||
border: 1px solid rgba(213, 146, 131, 0.3); |
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.
When looking at inspect, it only looks like the yellow border is showing up. Did you mean to have two borders?
padding-left: 20px; | ||
padding-right: 20px; | ||
padding-bottom: 30px; | ||
/* overflow: auto; */ |
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.
Here for the padding, (and in HeaderTitle for margin) - why did you choose to write multiple lines for padding instead of combining the values to have one line?
e.g.
padding: 0 20px 30px 20px;
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.
Well done this week! Your code is easy to read and I love your theme and animation, also nice touch that the input for the username is being implemented at the end of the question to make it more personal 🤩
import React from 'react'; | ||
import { useSelector } from 'react-redux' | ||
import Lottie from 'lottie-react'; | ||
import { CardContainer, LocationText, SingleChoice } from './GlobalStyles' | ||
import animationData from '../lotties/running-rabbit.json'; | ||
|
||
export const FinalPage = () => { | ||
const defaultOptions = { | ||
loop: true, | ||
autoplay: true, | ||
animationData, | ||
renderer: 'svg', | ||
rendererSettings: { | ||
preserveAspectRatio: 'xMidYMid slice' | ||
} | ||
}; | ||
|
||
const game = useSelector((store) => store.game.gameStep) | ||
const username = useSelector((store) => store.game.username) | ||
|
||
return ( | ||
|
||
<CardContainer bgColor="rgb(226 212 26 / 0.4)"> | ||
<LocationText>{game.description}</LocationText> | ||
<SingleChoice>Well done, {username}, you have found your way out of the rabbit hole!</SingleChoice> | ||
<Lottie | ||
animationData={defaultOptions.animationData} | ||
loop={defaultOptions.loop} | ||
autoplay={defaultOptions.autoplay} | ||
renderer={defaultOptions.renderer} | ||
rendererSettings={defaultOptions.rendererSettings} | ||
style={{ width: '300px', height: '300px' }} /> | ||
</CardContainer> | ||
) | ||
} |
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.
Nice that you used Lottie and GlobalStyles!
import React from 'react' | ||
import { useSelector, useDispatch } from 'react-redux' | ||
import { getGameStep } from 'reducers/game' | ||
import { CardContainer, StyledButton, LocationText, SingleChoice } from './GlobalStyles' | ||
|
||
const GameBoard = () => { | ||
const game = useSelector((store) => store.game.gameStep) | ||
const actions = useSelector((store) => store.game.gameStep.actions); | ||
const username = useSelector((store) => store.game.username); | ||
const dispatch = useDispatch() | ||
|
||
return ( | ||
<CardContainer> | ||
<LocationText>{game.description}</LocationText> | ||
<p>Your location is currently: {game.coordinates}</p> | ||
<p>Which way will you travel, {username}?</p> | ||
{actions && actions.map((action) => ( | ||
<SingleChoice key={actions.direction}> | ||
<LocationText>{game.type}</LocationText> | ||
<p>{action.description}</p> | ||
<StyledButton | ||
type="submit" | ||
onClick={() => dispatch(getGameStep( | ||
action.type, | ||
action.direction | ||
))}> | ||
Go {action.direction} | ||
</StyledButton> | ||
</SingleChoice> | ||
))} | ||
</CardContainer> | ||
); | ||
} | ||
|
||
export default GameBoard |
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 structure, easy to read the code!
export const getGameStep = (type, direction) => { | ||
return (dispatch, getState) => { | ||
dispatch(loading.actions.setLoading(true)) | ||
console.log(loading, 'loading getGameStep') | ||
|
||
const options = { | ||
method: 'POST', | ||
headers: { 'Content-Type': 'application/json' }, | ||
body: JSON.stringify({ | ||
username: getState().game.username, | ||
type, | ||
direction | ||
}) | ||
} |
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.
the "type" is always the same throughout the labyrinth so it would be enough here with just "direction"
No description provided.