Just HTML, SCSS and Vanilla JavaScript

Community feedback

• 

  P

  Dave•5,295

  @dwhensonPosted over 3 years ago

  hey @AgataLiberska amazing work! 👍 This looks total great and pretty much pixel perfect too - I can never get that right.

  Nice approach with the absolutely position div. I'm no expert, but I think that is more semantically correct than an aside here, which should really contain content "only indirectly related to the document's main content", which isn't really the case if you are using a background image.

  I can't really add much value to your work but here are some, very small, suggestions you might like to consider:

  1. Design-wise - as I said it looks great to me, my only small suggestion would be to add a max-width on the input so that it doesn't stretch too wide on smaller screens?

  2. My only other suggestion would be to add the novalidate attribute through the JavaScript rather than in the HTML. That way if the JS fails for any reason you can still take advantage of the in-built browser validation.

  Both these are really small points!! Really like what you've done here. Cheers! 🙌

  2

  ---

  

  Agata Liberska•3,995

  @AgataLiberskaPosted over 3 years ago

  @dwhenson Thank you! That's exactly what I thought - that aside would need some content in it, and I didn't want a div outside of main so I moved it back in.

  Thank you for your suggestions too, I'll get on them straight away :)

  0
• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted over 3 years ago

  Hey there, Agata Liberska! 👋

  Just wanted to drop in to say good job on this challenge! 👏 Your solution scales up/down very nicely (something that I had a little trouble with when I worked on this challenge)! 😆

  One idea I had about your challenge with the layout is that it's very easy to overlay elements with CSS grid, so you could have put the image inside the same container as the rest of the main content of the page and then wrapped that container and the header in a grid container. Then, you could make the main content full screen and simply place the header in the top-left corner of the screen and overlay it over the main content. IDK it that's a best practice, but it's a thought. 😅

  Keep coding (and happy coding, too)! 😁

  1

  ---

  

  Agata Liberska•3,995

  @AgataLiberskaPosted over 3 years ago

  @ApplePieGiraffe I think I'm going to have to look into CSS grid more, didn't know that overlaying elements was an option :D

  0
• 

  Sven•295

  @Sven72Posted over 3 years ago

  Hi @AgataLiberska, maybe ARIA-Label is what you are looking for. (In case you don't want to use randomly div's due to accessibility reasons.) If so, I found this explanation to ARIA, which is helpful: https://stackoverflow.com/questions/22039910/what-is-aria-label-and-how-should-i-use-it And especially this MDN article about img:role ttps://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Role_Img Cheers, Sven

  1

  ---

  

  Agata Liberska•3,995

  @AgataLiberskaPosted over 3 years ago

  @Sven72 I was actually considering using role=banner but I already had a header element, didn't want to reorganise everything and wasn't sure if I can have both, since header has the same meaning I think...

  0