Base Apparel Coming Soon page with Grid

Community feedback

• 

  solitary_coder•1,000

  @kabir-afkPosted about 1 year ago

  Hey kevin , good job on completing the challenge , on reviewing your queries I was able to work on some of them . . . here are the necessary corrections to be made :-

  • Reason for whitespace
    • so when you inspect your image-on-1024px vw you'll see that your img is centered inside the <picture> element , this is ok but it is also the reason why you see whitespace despite compartmentalizing your grid through grid-template-areas . . . to solve this issue you can do the following:-

  picture img{
  margin:auto;
  width:100%
  }
  

  Literally this single addition of line solves your problem and fixes your layout issues

  • Submit btn

    • Not that your approach is wrong but another alternative would have been to use flex on your form element and position it accordingly , this would have been more responsive ig

  • Background-image

    • Well can't really help with that 😅😅. . .but you can try setting your background-size to a custom value like 70vw or whatever suits you, won't be perfect but still better than cover ig

  • Other Issues

    • you forgot to add alt description in your images , not a big deal but quite a need when there is a problem fetching the image properly.
    • you forgot adding id to your input element which is bad in terms of accessibility , it literally presents itself as an issue in your console . . .now here you have a single input element , but once you start working with a lot of inputs inside a single page , it will become a big issue.

  • Regarding JS

  Thanks to you I got to know about the checkValidity() method, otherwise up until now I was using JS Regex and using .test method to validate my input values . . .there are still some issues that I would like to address . . . .

  Your code really seemed repetitive , especially the constant addition and removal of classes, like you could have used toggle for a shorter and better approach . . . and since you used innerHTML to change the text inside error-message you could have done styling inside it using

  errorMessage.innerHTML = !inputField.checkValidity() ?
  `<small style="color: red;">Please provide a valid email address</small>`
      : `<small style="color: green;"><b>Success!</b> And immediately shredded.</small>`;
  
      inputField.style.borderColor = (!inputField.checkValidity()) ? "red" : "green";
      errorIcon.classList.toggle("hidden", inputField.checkValidity());
  });
  inputField.value = "";
  

  This also avoids need of adding unnecessary classes inside your CSS. Make sure to add the required attribute in your html as well , for the checkValidity() method to work when inputField is empty, something like

  <input type="email" placeholder="Email Address" class="input-field" required>
  

  Look , the JS code I provided might not resonate with you very well, but I think its better to write a short , concise and maintainable code when you can . . .

  Hope I was able to help, happy coding!! 🥂🥂

  Marked as helpful

  0

  ---

  

  Kevin H.•150

  @kevinx9000Posted about 1 year ago

  @kabir-afk -- Thank you for all the helpful comments! I will definitely refer to this when reviewing my code.

  0

  

  solitary_coder•1,000

  @kabir-afkPosted about 1 year ago

  @kevinx9000 I reupdated the JS code since I noticed that there was some typo . . . glad I was able to help

  0