Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Intro component with signup form

Misiu• 50

@misiucodes

Desktop design screenshot for the Intro component with sign-up form coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


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?
  • Javascript: could I have done anything to factor my code differently so it's cleaner and less repetitive?
  • for the form, is there anyway to maintain the height of the form after the error messages appear?

Community feedback

Ken• 935

@kenreibman

Posted

I forgot to add on... for your JavaScript, 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 .classList.add('show-error')

changing the 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

0

Misiu• 50

@misiucodes

Posted

@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!!

1
Ken• 935

@kenreibman

Posted

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 heading, p, etc. since people looking over your code may not know exactly what p you 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 ~1000px for 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 max-width in rem units for your main container, wrapper in 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 aria-hidden= "true"
  • Also using correct HTML5 semantics, you should always wrap your content in a main tag. Your case you could just change your wrapper div into a main class="wrapper".

I hope this helps!

Marked as helpful

0
P
Grace• 27,630

@grace-snow

Posted

Hi

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

Good luck

1

Misiu• 50

@misiucodes

Posted

@grace-snow Thank you for taking the time to provide feedback!

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord