Mobile-first Ping Landing Page

Community feedback

• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted over 3 years ago

  Hey there, Shivam! 👋

  Just wanted to say nice job on another challenge! 👏

  I think if you'll follow brasspetals' helpful advice you'll be good to go (it was some of the same stuff I was going to suggest)! 😀

  Keep coding (and happy coding, too)! 😁

  1

  ---

  

  Shivam•520

  @shivjoshi1996Posted over 3 years ago

  @ApplePieGiraffe Thank you! Appreciate you reviewing my solution. Happy coding to you too!!

  0
• 

  Anna Leigh•5,135

  @brasspetalsPosted over 3 years ago

  Hi, Shivam! 👋

  Congrats on completing another challenge! It looks good and responds perfectly. Your CSS and HTML both look good to me. My only small suggestions would be to include the social media div in the footer and to wrap the social media icons in anchor tags since they are technically links. Oh! One more thing - I discourage using inline styling. Add a class to your h1 span to apply the font-weight instead.

  To clear up the errors in your report, add an alt attribute to the logo img, and a label for your email input. I believe the last errors are being caused by an extra quotation mark on line 28 of your index.html.

  As for your question, instead of adding the .form-error class, you could try using input[type="email"]:invalid selector for styling instead.

  Great job on this challenge - happy coding!

  1

  ---

  

  Shivam•520

  @shivjoshi1996Posted over 3 years ago

  @brasspetals Thank you so much for the feedback! I have made the changes and they work well. A couple of follow up questions, if you don't mind:

  1. Due to input=[type="email"]:invalid, The email input form now shows as red on page load (I understand this is because I have the "required" attribute in the HTML). Is there a better way to handle this sort of thing, so that the form doesn't immediately show up red, but does if the user enters nothing in?

  2. The Facebook icon now looks a little wonky compared to the other two. I tried to use .social-media i:nth-child(1), as I had before, but it now changes all of the icons. I couldn't figure out how to make them all look the same, do you have any suggestions? I assume it is a quick-fix that I am just not aware of.

  Again, thanks for all of your help, appreciate it a lot!!

  1

  

  Anna Leigh•5,135

  @brasspetalsPosted over 3 years ago

  @shivjoshi1996 Ack, I see required is getting in the way - sorry I didn’t catch that before. 🤦‍♀️ Here’s a link to a method using a bit of JS to work around the issue. It’s basically what you were doing before, but hopefully this time you won't have to use !important for the styles to be applied.

  The icon problem is happening because the icons are all different widths. One method would be to give the icons a set height and width (both the same value, so that you get a circle and not an oval), and then use flex and justify-content to center them. Adding the following to .social-media i seemed to work for me:

  width: 34px;
  height: 34px;
  display: flex;
  justify-content: center;
  

  You’ll probably have to add text-decoration: none to .social-media a after doing this to get rid of the underline.

  Hopefully these both work - let me know!

  1