Skip to content
OverviewSolutions
10
Comments
5
Stefani Josifovska
@stefani-josifovska

All comments

  • Stefani Josifovska•270
    @stefani-josifovskaPosted over 2 years ago

    Hi, good job on the code so far! Setting the mobile version is fairly easy, it's done with something called a media query (you can check how to use it on w3schools or mdn), and basically you define the changes in the css code between the desktop and the mobile version there. Also, once you've seen how it all works, I would highly recommend going with the mobile-first approach in the next challenges, i.e. doing the mobile version first and then adding a media query for the desktop version. I know the bootcamps usually do it the other way around, but I would really recommend you also try mobile-first - usually the code gets smaller this way, and it also helps you watch out for the details on the smaller screen and then build up for the bigger screens. :)

    Marked as helpful
    1
  • Keagan Dickinson•160
    @PhisherFTWSubmitted over 2 years ago

    This challenge was great, it forced me to use alot of the skills I am currently learning like Javascript DOM and more, I am very happy with how it turned out, although there is one small issue

    When I change the width of it to be below 368, the terms and services messages messes up and I know its because I used span in order to target the specific text so I could turn it red, but if you have a different way please let me know.

    Thanks

    Intro component with signup form master

    1
    Stefani Josifovska•270
    @stefani-josifovskaPosted over 2 years ago

    Hello Keagan, first of all great job! I only have one suggestion: maybe, if all the input fields are valid on submission, you could clear the input fields out of give any other kind of feedback to the user, so that he knows what is going on. Regarding your question, the display: flex on the "termsservices" class causes this problem, because it sees the text and the span as two elements and thus applies properties you don't really want. So maybe delete that property, and add a text-align: center instead, and your problem will be gone. :) Happy coding!

    Marked as helpful
    1
  • Stefani Josifovska•270
    @stefani-josifovskaPosted over 2 years ago

    Hi! I just opened your solution and first of all, I would like to point out that I love the structure, it is really clean. I only have one possible improvement, which is basically a solution for your desktop-, as well as mobile version at the same time, and it has to do with the positioning of the "container" element. Basically, you can also see on the segment on this page as well that your container is not perfectly centered vertically. I noticed that you have tried to do this by setting margins on the "container" div, which would normally work, but doesn't because you have also set margin-top: 10vh, so it's overwritten. Anyways, there are a few possible solutions, but the one that I have tried on your webpage and works perfectly is to work with the css of the "body" element. Namely, you have to first get rid of the automatic CSS that body has, by specifying margin: 0, and then you can set the body-height or minimum body height to 100vh (something in the spirit of your margin-top: 10vh approach), and then turn the display property of the body element to flex -> and you know what to do afterwards in order to center the div. :) I wouldn't say this is the best or the only approach, for that matter, but it's just one approach worth trying. *This would center the "container" div perfectly both in mobile and desktop version. However, you should maybe add top and bottom margins on the "container" div in mobile version, in order to make it look nicer. And maybe also add a background color to the body, to make it look nicer, as in the original design. Other than that, the solution is looking really nice! **And just another note regarding the css code: if you don't need to style a given element, there's no need to give it a class name and then defining an empty definition for that element. Just don't assign anything to it, in order to prevent redundant and unnecessary code. I hope I helped even a little bit. Good luck coding!

    0
  • Stefani Josifovska•270
    @stefani-josifovskaPosted over 2 years ago

    I really love the design and I think you've done an amazing job CSS-wise (note that I'm only seeing the mobile version at the moment)! Regarding the email validation, judging from the code, I would say that the problem lies in the fact that you have commented out the listener which listen for your event, so there really isn't any way to make the browser go through the js file one again, meaning once it's loaded and you start typing, the input won't match the email pattern, of course, and it throws the error message. Also, I see you've used the .preventDefault() method, but unfortunately there isn't an event to attach it to, so that's why it's not working. So I guess you could try calling validation(e) inside the listener definition, so right next to "submit", and maybe pulling the const definitions outside of the function. And I don't reallt understand the purpose of the email===" " condition inside the if-statement, but if you define this with a && operator, the if-statement would never be true, therefore you will forever be stuck with the error message. Sorry for the quite long comment but I hope I've managed to help a little bit :)

    Marked as helpful
    1
  • Stefani Josifovska•270
    @stefani-josifovskaPosted over 2 years ago

    First of all, I have to say that I love that everything is responsive and functional! However, I do have a remark about the input validation, because the way you have defined the regex works perfectly if the entered email doesn't contain any special symbols, like a dot, an underscore or a number, which most emails do. So, I would update the regex and I think it'll work perfectly! Also, a little remark on the form styling, which isn't something you did wrong, just something you probably haven't noticed while coding - once a valid email is entered, the red border color still stays, but I think it should go back to the original color. Other than that, of course everything could always use a little more styling and working on some details, but everything would be fully functional even without it. Anyways, good work!

    Marked as helpful
    1