Skip to content
OverviewSolutions
1
Comments
2
CelianB
@CelianB

All comments

  • CelianB•40
    @CelianBPosted over 1 year ago

    Hi, here is my comments

    • Not responsive
    • Wrong font
    • you function "validateEmail" can be shorcuted by return (/^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/.test(emailVal))
    • you should define your variable email in validateInput function
    • you could use variable for colors
    • Create the entire div with js in success.html is a bad practice, just create a span with an Id and write in it with .innerText
    • Consistency with semi-colons, use Prettier
    0
  • CelianB•40
    @CelianBPosted over 1 year ago

    Hi, good job ! Some things, I can tell :

    • you class name can be more meanfull (mb-1,mb-2)
    • you could use variables for colors
    • your css and js could be placed in separated file
    • "Style" directly in html is I think a bad pratice
    • Using innerHTML is a bad practice (even if here there is not risk), I can lead to some XSS attack. In your situation, it force you to write content in JS and it is bad for separation of concern. Here you could write the text in html, place a span with an Id and write into the span (with innerText)
    • In general I think you have too much nested div with 2 div nested with "display:flex"

    Thanks for reading, feel free to do the same on my code, I just publish it

    Marked as helpful
    0