-
Notifications
You must be signed in to change notification settings - Fork 322
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 - Vio and Theres #206
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Theres Brännberg Lendt <[email protected]>
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.
Hey Theres and Vio,
We really enjoyed the theme of your labyrinth project and that you put extra effort into expanding the storyline that we were given. You have really gone the extra mile with unique background images and sound effects for each coordinate. Pictures and compass come together to enhance the theme.
One possibility for improvement would be to make the on/off toggle for sound a bit more visible on desktop. That could make it easier for the user.
Your code is clean and easy to follow. Ahoy and well done!
/Emma & Josefin
} | ||
}, [currentLocation, isMuted]) | ||
|
||
let backgroundImage; |
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 use of switch case. It gets a bit confusing though with the Backgrounds.js seemingly intended at some point to do the same thing. Maybe remove it or change variable names to make it clearer what the difference is between them?
setClickCount(0) | ||
} | ||
|
||
// First click shows directions, second click chooses 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.
Good use of comments to clarify your code!
store.isLoading = action.payload | ||
}, | ||
setUser: (store, action) => { | ||
store.username = `${new Date().getTime()}+${action.payload}` // Added time to make the username unique |
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 make sure the user don’t run into problems with a shared username.
@@ -0,0 +1,31 @@ | |||
import styled from 'styled-components/macro' | |||
|
|||
export const Button = styled.button` |
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 choice to make your button into a reusable component!
options={{ speed: 50, cursor: false }}> | ||
<StartText> | ||
<p> | ||
You have a burning ache in your throat |
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.
Really nice that you have added your own beginning and ending, it adds some storytelling to the user's experience.
newAudio.play() | ||
} | ||
// Pause the previous audio by referring to the current audio element | ||
if (audioRef.current) { |
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 managed to do different sound effects depending on where you are in the maze.
open the bottle from your chest pocket and take a sip... Yohoho! | ||
</EndText> | ||
<LottieDiv> | ||
<Lottie style={{ width: '300px', margin: 'auto' }} animationData={ship} /> |
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 choice of lottie!
https://captains-adventure.netlify.app/