Newsletter signup with form validation using vanilla JavaScript

Community feedback

• 

  Andrey•2,800

  @dar-juPosted 4 months ago

  Hey, Julia!

  I looked at your code: there are no errors in html and css, bam is fine, js works correctly. Great job!

  Your regular expression skips invalid email addresses, for example, the address test@. This should not happen. There is a fairly proven and reliable expression, you can just save it for a similar case: /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/

  When you hover over the button, all the content starts moving, I found the lines that affect this, delete lines 130 and 131: margin-top: -0.5rem; margin-bottom: 0rem; And it will look better visually.

  It is better to remove the pointer cursor on the input field, because it is not a button.

  I saw your request about flex. If you remove the following lines from the js: 34 - signupPage.style.display = "none"; 36 - successPage.style.display = "flex"; 42 - signupPage.style.display = "flex"; 44 - successPage.style.display = "none"; Then your code will also work correctly, the classes works. I checked.

  Everything else is great!

  Marked as helpful

  0
• 

  P

  Yulia•720

  @YuliaLantzbergPosted 4 months ago

  Hi Andrey. I really appreciate your detailed review and efforts you took to go through my code in attention. It's very helpful. You are right about regular expression. I've added and saved the sample you gave. Thank you so much. Indeed, the entire content was moving with hover on the button. I wanted actually that only button will move. I didn't take in attention that margin can trigger the entire scope. Now I replaced margins with translateY and it makes work perfectly, moving the button without disturbing other elements. About the flex, unfortunately, it still doesn't work for me if to remove the mentioned lines. I am talking about the mobile view only. On other views it works perfectly if to remove them.

  And again, thank you so much for your feedback. Really appreciate

  0

  ---

  

  Andrey•2,800

  @dar-juPosted 4 months ago

  @YuliaLantzberg, ah, now I understand where the problem is

  Look at the CSS part of the code:

  .success.active {
    height: 100vh;
    display: flex;
    flex-direction: column;
    justify-content: space-around;
    padding: var(--standard-padding);
    display: none;
  }
  

  Here, flex is first, and then the block is removed. Remove the line display: none; and it should work on the mobile version

  Marked as helpful

  0

  

  P

  Yulia•720

  @YuliaLantzbergPosted 4 months ago

  @dar-ju OMG, you are right. So stupid bug, how did I miss it

  0