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 - Camilla #205

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

Project Labyrinth - Camilla #205

wants to merge 10 commits into from

Conversation

camcron
Copy link

@camcron camcron commented Apr 30, 2023

No description provided.

Copy link

@mvfrid mvfrid left a comment

Choose a reason for hiding this comment

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

I really like the style of your game. Everything kind of gives me the feeling of being in an adventure mystery in a desert far far away. Love it! Each design choice ties everything nicely together. Love the images and the type effect. Great job!

The responsitivity is really nice as well. I checked it in a couple of different sizes and in my mobile and it looked great and had a fluid responsitivty which I think is nice!

Overall, the code is well structured and divided into good sized components. However, I think it would improve readability if you broke out some styled components from the JS code and added it to a separate file. Especially for the Game.js, which becomes quite long and the "core code" is found way down at the bottom.

Really nice job!

Comment on lines +9 to +15
Technologies used:
React,
Redux Toolkit,
JavaScript,
Styled Components,
TypeIt,
UUID (Universally Unique Identifier).
Copy link

Choose a reason for hiding this comment

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

Great readme! I really like that you list the technologies and features that you have used.

Comment on lines +7 to +10
<meta property="og:title" content="The labyrinth game">
<meta property="og:description" content="Can you find your way through the labyrinth?">
<meta property="og:image" content="%PUBLIC_URL%/assets/OG-img.png">
<meta property="og:image:alt" content="Labyrinth">
Copy link

Choose a reason for hiding this comment

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

Excellent use of OG tags and favicon! Love that you found a labyrinth icon :D

Comment on lines +17 to +20
<GlobalStyles />
<Provider store={store}>
<Screen />
</Provider>
Copy link

Choose a reason for hiding this comment

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

Nice keeping the App.js clean by only having the Styles, Store and a Screen component.

Comment on lines +30 to +31
const isStartScreen = useSelector((store) => store.game.isStartScreen);
const isEndScreen = useSelector((store) => store.game.isEndScreen);
Copy link

Choose a reason for hiding this comment

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

Good way of getting conditions to use in your conditionals!

Comment on lines +57 to +58
default: // Fall back img
return `url(${process.env.PUBLIC_URL}/assets/archway-step1.png)`;
Copy link

Choose a reason for hiding this comment

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

Great idea to use a fall back image if none of the conditions are met!

Comment on lines +27 to +28
backdrop-filter: blur(4.9px);
-webkit-backdrop-filter: blur(4.9px);
Copy link

Choose a reason for hiding this comment

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

This backdrop filter is super cool!

const dispatch = useDispatch();
const loadingState = useSelector((store) => store.game.isLoading);
const isGameOver = currentLocation.actions && currentLocation.actions.length === 0;
const [isExpanded, setIsExpanded] = useState(false);
Copy link

Choose a reason for hiding this comment

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

Nice use of useState and useEffect!

const isStartScreen = useSelector((store) => store.game.isStartScreen);

const handleUserNameSubmitted = () => {
dispatch(hideStartScreen());
Copy link

Choose a reason for hiding this comment

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

Nice job making this into a reducer!

Comment on lines +107 to +108
placeholder="Enter name..."
required
Copy link

Choose a reason for hiding this comment

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

Nice work making it intuitive. We get feedback when we try to enter an empty value.

},
reducers: {
setUserName: (store) => {
store.username = uuidv4();
Copy link

Choose a reason for hiding this comment

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

So it does not matter what name the user types in? It will be randomly selected? I think it works for this application, but could be a good thing to look into if we were to re-use the username. Could we add the unique ID as an addition to the username instead of completely replacing ut?

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.

2 participants