Survey project - Vera Sjunnesson #447
Conversation
mvfrid
left a comment
mvfrid
left a comment
There was a problem hiding this comment.
Really love the design! Such a nice mix with the artsy background images and the bold pink font with a drop shadow.
Love love love the progress bar! So nice to have it "integrated" like this at the top of the page.
I tried out your app in my mobile and it looked great. Good job on making it responsive! The only thing I could find was that for the smallest possible screen size (320px wide), the input field does not adapt fully & the placeholder text disappears.
Keyboard navigation worked great. I also listened to your app in the screen reader and could follow it well. If you want to, it would be nice to include something to also tell the visually impaired user about the survey progress. Like adding an aria-label only visible for screen readers, saying "Survey progress xx%". If you have time and feel like it, that would really elevate this to the next step.
Good job on the answer validation! A very nice solution.
As always, you write great and readable code that is easy to follow. If you want to improve it even more, then maybe look into the CSS file and try to add some comments or divisions that make it easier to follow.
I know you said that you felt like your code was a mess, but I think its not at all. You did a really great job this week and should be proud of your work!
code/src/App.js
Outdated
| <div className={classList( | ||
| 'background-image', | ||
| counter === 0 && 'image1', | ||
| counter === 1 && 'image2', | ||
| counter === 2 && 'image3', | ||
| counter === 3 && 'image4', | ||
| counter === 4 && 'image5', | ||
| counter === 5 && 'image6', | ||
| counter === 6 && 'image7' | ||
| )}> |
There was a problem hiding this comment.
Really clever way of handling the change in background images! Very good to connect it to the counter, which you already use for navigating through the flow of the code.
| <div className="progress-bar"> | ||
| <div style={{ width: `${(counter / totalCount) * 100}%` }} /> | ||
| </div> | ||
| ) |
code/src/App.js
Outdated
| aria-label="Background images of cinematic locations" | ||
| className={classList( | ||
| 'background-image', | ||
| counter === 0 && 'image1', | ||
| counter === 1 && 'image2', | ||
| counter === 2 && 'image3', | ||
| counter === 3 && 'image4', | ||
| counter === 4 && 'image5', | ||
| counter === 5 && 'image6', | ||
| counter === 6 && 'image7' | ||
| )}> |
There was a problem hiding this comment.
Really nice solution with the background images!
code/src/components/Location.js
Outdated
| <label htmlFor="continent"> | ||
| <select | ||
| id="continent" | ||
| aria-label="Drop down menu to select a continent" |
There was a problem hiding this comment.
Very nice and clear to follow with the screen reader!
code/src/components/Summary.js
Outdated
| </div> | ||
| <br /> | ||
| <p className="next"> | ||
| <span>Okay, lets see what we can do for you!👇</span> |
There was a problem hiding this comment.
Consider adding an aria-label to the emoji, telling the screen reader what it is. It sounded a bit confusing with "backhand index finger pointing down".
code/src/components/Summary.js
Outdated
| <button | ||
| type="submit" | ||
| onClick={nextStepResult} | ||
| aria-label="Submit button"> |
There was a problem hiding this comment.
It repeated this sentence several times
code/src/components/Name.js
Outdated
| <input | ||
| className="name-input" | ||
| type="text" | ||
| placeholder={!error ? 'Type your name here...' : '* Please enter your name'} |
There was a problem hiding this comment.
Nice solution using a state and ternary expressions for this!
| <span> | ||
| Are you basically open to anything? | ||
| </span> |
There was a problem hiding this comment.
In desktop, this adds a scroll bar both horisontally and vertically. On mobile it looks great. This does not bother me so much, it still looks good, but thought I would add a comment about it anyway :)
2 participants
View it live: https://survey-project-vera-sjunnesson.netlify.app/