[2nd] Sign-up Form w/ CSS grid, JS - Newbie

Community feedback

• 

  Amon•2,560

  @A-amonPosted over 3 years ago

  Hello! Looks great~ It's responsive too.

  I have a few suggestions for you 😀:

  • Why do you have wrap a line inside h1 with span tag?🤔 You can try using br tag to break the line of the same sentence. (Don't abuse it tho! Check out https://forum.freecodecamp.org/t/should-you-ever-use-the-br-tag/440252/2 for how to use it semantically.)

  • Try to not set styles directly in JS if possible. Add/remove or toggle class instead. E.g. Adding .error class to show error-related styling.

  • Instead of having an unused empty label, set the aria-label attribute on input element. (Doing this allows you to remove the label element)

  • The alt text for your error icon might not be appropriate 🤔, because imagine if the screen reader reads out "Exclamation" or something else to users. Maybe, "Warning", etc. would be better. Or if you wish to hide it, just leave the alt empty. Check this out: https://dev.to/nadiarasul/do-icons-need-alt-attributes-5g52

  Marked as helpful

  1

  ---

  

  Yuko Horita•645

  @Sloth247Posted over 3 years ago

  @A-amon Hey, I actually wanted to talk about another project. Thank you for pointing out the points;

  • <br> tags : I think I got some feedback from someone here not to use it to change the row - I know this could be easy if I use it... however, how can you change the font-weight without using span?
  • not styling directly in JS: Okay, understood
  • aria-label: I thought all input field should have labels for each?
  • alt text: good point, I will fix it from next project.

  I saw your solution for Pricing component with toggle by not using JS...! Do you think you can also do this project without using JS? And I tried your solution for the same project but it didn't work on my code😂

  0

  

  Amon•2,560

  @A-amonPosted over 3 years ago

  @Sloth247 Nope, it wouldn't be possible to change the font-weight but this project uses same font-weight throughout the heading (If I'm right 😂 I don't trust my sight lmao!) I don't think there's a ban on br tags or something 😂 For me, I think it really depends on how you use it. I think he/she refers to using br tags to replace usage of CSS positioning or margin which is definitely not right.

  Yep, every input should have label because it tells the screen reader users what the input is for. But aria-label attribute is also an alternative to it. Only difference would be, what's in aria-label won't be visually available.

  Mhmm, I think it could be possible (Don't take my word for this tho! 😂). I have few things in mind 🤔 Maybe soon. You can follow my github in case you wanna see if I manage to do it or not (This sounds so wrong 😂)

  Is it in your repo? I can try checking it out~

  0

  

  Yuko Horita•645

  @Sloth247Posted over 3 years ago

  @A-amon Oh ya, I'm sorry I was blind. Please see the comment from matt for my first project about <br> (Please ignore the outcome of the project, it was terrible).

  • aria-label : thanks for pointing out. I will look into the details.

  • Price toggle project: No, I have not saved it in repo but I will let you know when I do via slack message (only if you don't mind)😊

  0

  

  Amon•2,560

  @A-amonPosted over 3 years ago

  @Sloth247 No, you're not blind! 😂 Anyway, interesting. But maybe because of abuse of br tags, preferences, etc. (I don't really know anymore 😀)

  Ah- Yea, let me know on slack!

  0
• 

  T

  Chamu•13,130

  @ChamuMutezvaPosted over 3 years ago

  Nice work, the validation can be improved such that when correct data is entered whilst typing the error disappears. See this solution that impressed me this week solution

  • you do not need to resubmit your solution when you make changes to your code. As long as you update you repository then everything will be ok, you can generate another report if you want to see if some errors on this site has been cleared

  Marked as helpful

  0

  ---

  

  Yuko Horita•645

  @Sloth247Posted over 3 years ago

  @ChamuMutezva Thank you, his solution is very detailed 😲 I can say this project is definitely not for newbie to perfectly build it...!

  0

  

  T

  Chamu•13,130

  @ChamuMutezvaPosted over 3 years ago

  @Sloth247 , that's correct - the js involved is really a bit tricky for a newbie challenge.

  0

  

  Yuko Horita•645

  @Sloth247Posted over 3 years ago

  @ChamuMutezva Ya, and I didn't know the function to generate another report. Thanks for heads-up.

  0