Our reporter did not find any issues in this project! 🎉
Intro component with signup form
Misiu’s questions for the community
Would love feedback on:
- CSS styling: could I have done anything different to make the CSS file cleaner?
- CSS styling: is this optimized for mobile and different screen sizes?
- for the form, is there anyway to maintain the height of the form after the error messages appear?
instead of changing the innerHTML, I would create a class that reveals the error text like,
.show-error with display properties,
and add the
show-error class to the element with
innerHTML is not the most correct way to do it, as well as setting the
opacity to 1 and 0 does not hide it for people with screen readers so they will still get an error message regardless of if you can see it on the screen or not.
You can take a look at my code here as well.
Marked as helpful
@kenreibman Ken, this was AMAZING feedback! Thank you so much for taking the time to provide a detailed response on things I could change to make my code better. I learned so much just from this feedback and from looking at your code. Sending so much love from a newbie developer!!
Hey @misiucodes! Nice job! Since you asked a few questions in your description, I'll give you some feedback.
could I have done anything different to make the CSS file cleaner?
- People may disagree, but I like to name all of my HTML elements, including
p, etc. since people looking over your code may not know exactly what
pyou are referring to, when there are multiple paragraph elements in the document.
- is this optimized for mobile and different screen sizes?*
- Your media query breakpoint is way too late. I would switch from the desktop to tablet/mobile view at
~1000pxfor this project. A tablet user would have a hard time seeing their input as the form field becomes too narrow in width. That leads me to the next one,
- Set a
remunits for your main container,
wrapperin your case. It helps with responsiveness, and you can always set a new max-width for your container in a media query.
Also look at your reports and try to fix some of your accessibility issues as well.
- The icon-error just needs an
- Also using correct HTML5 semantics, you should always wrap your content in a
maintag. Your case you could just change your
wrapperdiv into a
I hope this helps!
Marked as helpful
You have a few html issues with this that need addressing
- text should never be in divs alone, use a meaningful element like a paragraph
- errors need linking to their input with aria-describedby
- errors need an aria-live attribute
- all inputs need labelling
I don't think you need as many structural divs on this either (remember you can put a class on form for example)
Make sure you Indent your code consistently. It's very easy for bugs to Creep in with poor formatting
@grace-snow Thank you for taking the time to provide feedback!
Join our Slack community
Join over 180,000 people taking the challenges, talking about their code, helping each other, and chatting about all things front-end!