-
Notifications
You must be signed in to change notification settings - Fork 39
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
Etna&Martins Zustand Maze #8
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Martin Jönsson <Martin-Joensson@users.noreply.github.com>
Co-authored-by: Martin Jönsson <Martin-Joensson@users.noreply.github.com>
fixed direction button; chang lottie loading animation; add some styling Co-authored-by: Martin Jönsson <Martin-Joensson@users.noreply.github.com>
Fixed buttons. Only display buttons that can be used.
Co-authored-by: Martin Jönsson <Martin-Joensson@users.noreply.github.com>
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.
Great job with the game! Nice pictures and easy to navigate in the UI 👍 We also loved that you implemented the sound effect! You did some of the stretch goals too like displaying the user name and the directions. We know that you were very busy last week, so good job! 🌕
/Alma and Wen
setIsStarted: (newValue) => set({ isStarted: newValue }), | ||
|
||
startGame: (username, uniqueName) => { | ||
uniqueName = username + Date.now(); |
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.
Smart way to get a unique name! We did the same 😁
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.
Clever!
uniqueName: uniqueName, | ||
labData: data, | ||
coordinates: data.coordinates, | ||
isLoading: false, |
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.
It could maybe be better to put "isLoading" in a finally block instead of here, because then it will always be executed regardless of potential errors :)
import AudioOn from "../../public/volume_up.svg"; | ||
import AudioOff from "../../public/volume_off.svg"; | ||
|
||
const audio = new Audio(AudioFile); |
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 touch to use audio in your game!
useEffect(() => { | ||
if (audioOn) { | ||
audio.play(); | ||
audio.volume = 0.05; |
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.
Smart way to control the volume 🔊
import { useGameStore } from "../stores/useGameStore"; | ||
import { useRef } from "react"; | ||
|
||
export const GameButton = ({ buttonName }) => { |
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.
You could consider giving this component a clearer name, it's not evident for us what "gamebutton" would do :) Maybe something like Startbutton would be better.
|
||
return isLoading ? ( | ||
<Lottie | ||
animationData={footstepsAnimation} |
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 love the animation! Very cute footsteps! 👣
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.
Same!
{/* <GameButton buttonName="East" disabled="disabled"/> | ||
<GameButton buttonName="South" /> | ||
<GameButton buttonName="West" /> | ||
<GameButton buttonName="North" /> */} |
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.
You can remove this comment!
|
||
|
||
export const PlayScreen = () => { | ||
const backgroundImage = useRef(); |
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 foud a way to use useRef 👍
<div className="start-container"> | ||
<h1>The Maze</h1> | ||
<p> | ||
Enter the labyrinth on your own risk... <br /> Can you find a way out |
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.
Apparently <br>
is outdated or has no semantic meaning so it could cause accessibility issues. It's better to use 2 separate p elements 😃
#east { | ||
grid-area: east; | ||
} | ||
|
||
#south { | ||
grid-area: south; | ||
} | ||
|
||
#west { | ||
grid-area: west; | ||
} | ||
|
||
#north { | ||
grid-area: north; | ||
} |
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 haven't used grid in this way before, looks cool and smart way to solve the position of direction buttons!
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 job with the labyrinth! Your project is structured and the UX/UI is thought through. Keep up the good work 🚀
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.
Simple and clean App 👍
|
||
return isLoading ? ( | ||
<Lottie | ||
animationData={footstepsAnimation} |
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.
Same!
import { useState } from "react"; | ||
|
||
export const TextBox = () => { | ||
const { labData } = useGameStore(); |
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.
Haha took me a minute to realise lab was short for labyrinth in this case 🙈
return ( | ||
<> | ||
<button className="textbox-button control-button" onClick={handleShow}> | ||
{showDirections ? "Show Description" : "Show Directions"} |
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.
Love this! Although it would be nice if it didn't show on the last screen since there aren't any directions then
username: "The wanderer", | ||
uniqueName: "thewanderer", |
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'd rather have a placeholder instead of an initial state for the username
setIsStarted: (newValue) => set({ isStarted: newValue }), | ||
|
||
startGame: (username, uniqueName) => { | ||
uniqueName = username + Date.now(); |
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.
Clever!
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 structured store and apart from labData
all variable names are very clear 😄
Netlify link
https://the-zustand-maze.netlify.app/
Collaborators
[Martin-Joensson]