@jessegood
Posted
Hi arey
I just submitted a solution for the same problem. Here is some feedback I hope it helps!
-
The
<div>element is a block element and is not allowed inside<label>elements. I assume you meant to use the numbers as labels? In that case you could just remove the<div>from them and they would become the label. -
You had two media queries, one for
400pxand360px. I wasn't sure why you split them up as they seem very close. -
I see that you built the Thank You part in JavaScript. One easier way might be to use
display: noneand then just set it to display when you want to show it (and set the other part todisplay: none). -
Instead of adding event listeners on each element you want. Another idea would be to just add an event listener on the
documentand then just only react to click events when a user clicks a radio-input or the button. You could also removetype="submit"from your button so it does not act like a submit button.
Also, doesn't affect your case, but as a JavaScript best practice, always use the === (the triple equals).
Marked as helpful
@arey-dev
Posted
Hello! @jessegood, Thank you for giving a feedback to my solution 😊 I'm glad you mentioned all of that. Here's why I did such things:
- The
divinsidelabelwas a really horrible mistake. I wanted to make a round container but I couldn't set the width onlabel. And that's why I putdivinside it, to have a container that I can make round. But then again, I could've just set thedisplay: inline-blockon labels 😅 - I had to set a breakpoint at
360pxbecause I wanted the style to fit down to300px, for really small screen sizes. - setting
display: noneis for sure the easier and, I think, the right approach. - I didn't think of that 😅 but I did delegate events on radio buttons. I'll come up with a solution that uses document-wide event listener.
Again, thank you! This feedback helped me a lot. I'll make sure to make changes in my code, and also take a look at your solution.
