Community Feedback

• 2

  Hi Rene, nice job on the new challenge. Your design respond well.

  I see that you're using the the invalid JS event, cool. I'll have to learn more about that but I can provide another solution to add to your arsenal. Attach a submit event listener to your form (example below). Then you can get the value from the email textbox to check if value exists and valid. To check if email is valid format look into regular expressions for email.

  const form = document.querySelector("name-of-form");
  
  function validateForm(event) {
    event.preventDefault();
  
   // code here
  }
  
  form.addEventListener('submit', validateForm);

  I think your tablet layout can kick in sooner. There's a lot of white space surrounding the mobile layout that can be utilized. Also the dot image maybe looks out of place on tablet view

  For the podcast icons, because they're icons that someone would click to go somewhere (call to action). Surround each image with an anchor tag.

  Great job overall and hope it helps 👍👍

  P

  artimys1,655Posted 2 days ago

  Reply

  • 0

    @artimys Hey, thanks for your input.

    Just FYI, as it's maybe interesting for you too: First I tried to solve it more or less exactly like you suggested. Attach a submit event listener and validate the form as soon as it gets triggered.

    Well, the only problem is that the submit event never get's triggered, because the HTML5 validation kicks in first when I set the attribute type="email" That's how I found out about the invalid event.

    And yeah, you are right. I should definitely spend some more time with my mobile layout :) I will work on that likely tomorrow.

    P

    rsauerwein30Posted a day ago

    Reply
  • 0

    @rsauerwein Ah good point on the type="email". There's a way to disable it like so <form action="" novalidate> but it feels wrong.

    I ended up reading the ConstraintValidation API here. Pretty powerful stuff

    P

    artimys1,655Posted a day ago

    Reply

  Add a comment
• 2

  Hi Rene, I've got a few suggestions in addition to the above that I hope you find helpful. I'm only viewing on mobile atm so can't review how it looks on other screen sizes I'm afraid.

  Layout

  • There's no background image visible like in the designs
  • the form button is hitting the bottom of the screen for me. It needs some margin bottom or give a container/wrapper element some padding so it has space around it.

  HTML

  • The alt attribute on the logo image needs to say something more meaningful. At the moment there's no Name of the product on the page so assistive tech would just hear it called 'logo'
  • I don't think those podcast service images would actually be links so don't think they need anchors around them. But tqey are a list of services, so should be in a list element
  • Your form element needs a label
  • it's more usual to put any validation messages next to the input they relate to, rather than under the whole form.

  JS

  • I don't actually think this is necessary at all. As you're using the required attribute on the input anyway, you can use a pattern attribute on the input (containing a regex) and css to acheive exactly the same thing (more in fact) thanks to the :valid and :invalid pseudo states.

  As a little extra, if you wanted a prettier focus state on your form elements, you could use drop shadows or pseudo elements on wrapper elements (because you can't add a pseudo element to an input) using borders or backgrounds to acheive matching rounded outlines.

  Overall these are all small things and easily fixable, so I'd encourage you to take one more pass at this so you can use as a portfolio piece. Well done so far! 🙌

  

  grace-snow1,580Posted a day ago

  Reply

  • 0

    @grace-snow Hi Grace, thanks for your suggestions. I will definitely work on that in the upcoming 1-2 days and give you another update.

    For now I have one question - you mentioned: There's no background image visible like in the designs

    That's interesting. Seems like I ran for my first time into some compatibility issues. Could you please clarify that? What part of background exactly is missing? The singing guy? I can't reproduce that with my iPhone.

    What I actually did in my code: generic.css background-color: var(--dark-blue);

    mobile.css

    /* Background */
     background-image: url("../assets/mobile/image-host.jpg");
     background-position: center;
     background-repeat: no-repeat;
     background-blend-mode: multiply;

    But actually it's a new concept for me to combine different backgrounds. What I actually tried to achieve for the mobile version was to mix --dark-blue and image-host.jpg together to match as good as possible with the challenge designs - hopefully you understand what I'm trying to tell you..

    However, I just realized that on the iPhone that effect also doesn't seem to work as image-host.jpg seems to be 100% in foreground. So yeah, I have for sure to work more on my mobile version (with Chrome on my desktop PC it looks better..)

    P

    rsauerwein30Posted a day ago

    Reply

  Add a comment

Give some feedback to rsauerwein about their solution