Skip to content
Submitted about 3 years ago

Flexbox / Grid + Simple input validation via Regex / vanilla JS

Patricia Hurst•80
@patricia-hurst
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Base Apparel coming soon page
  • HTML
  • CSS
  • JS
1newbie

Design comparison

SolutionDesign

Solution retrospective

I'd love feedback as I am still learning (especially JavaScript)!

Community feedback

  • Raymart Pamplona•16,100
    @pikapikamartPosted about 3 years ago

    Hey, awesome work on this one. Desktop layout looks fine, it is somehow responsive but the image is not taking full height when the screen-width changes. Mobile layout looks great though.

    Kirin already gave feedback on this, just going to add some suggestions as well:

    • It would be great to have a base styling with these properties
    html {
  box-sizing: border-box;
  font-size: 100%;
}
*,
*::before,
*::after {
  box-sizing: inherit;
}

    This way, handling an element's size will be lot easier because of the box-sizing: border-box. You can search up some more of how this works.

    • On your body tag change the background-image: url(/images/bg-pattern-desktop.svg); to background-image: url(./images/bg-pattern-desktop.svg); using / or root path in github does not work.
    • Always have a main element to wrap the main content of your page. On this one, the .wrapper should be using the main instead of div.
    • Website-logo img should be using the website's name as the alt like alt="base apparel". Remember that a website's logo is meaningful so always make sure it uses the proper alt value.
    • h1 should be text-align: left and you don't need to use those span inside it to make each text wrap onto another row, you should just use max-width on the h1 and let the text wraps.
    • Your input tag lacks an associated label tag on it. Since there are visible-label, the label would be a screen-reader only label, meaning it would make user of like sr-only class. The text-content should describe what the input needs like the value on the placeholder.
    • error-icon is not showing, check for / path usage.
    • Now, the form error works but only limited for sighted users. A great way to make the input more accessible should be done like this:
    if ( input is wrong )
  input.setAttribute("aria-invalid", "true");
  input.setAttribute("aria-describedBy", id of the error-message);
else 
  input.removeAttribute("aria-invalid");
  input.removeAttribute("aria-describedBy");

    The error-message element should have an id attribute which is referenced by the aria-describedBy attribute on the input element. By doing that, your user will know that the input is wrong because of aria-invalid and they will know what kind of error they made because of the aria-describedBy

    Aside from those, great work again on this one.

    1
  • Ahmed•105
    @Kirin02Posted about 3 years ago

    Hey, i'd say try to make the submit button hover animation a bit more interesting i would say, the position of the image is kinda odd i'd say espcially since it got sharp edges i would say move it to the extreme right or limit the height or something if you use inspect tool you'll find that it overflowing from the <body> Also It highly unrecommended to Use !important other than that i like the mobile display it looks really good but i think you can improve the desktop one a bit more

    0

Please log in to post a comment

Log in with GitHub

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord