Skip to content
Submitted about 3 years ago

Room Page Master Responsive

Adil Akothiat•205
@Adil-Akothiat
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Room homepage
  • HTML
  • CSS
  • JS
3intermediate

Design comparison

SolutionDesign

Solution retrospective

I made it using sass any feedback is appreciated!! :)

Community feedback

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

    Hey, awesome job on this one. Layout looks really good in desktop, it is responsive and the mobile layout is good as well.

    Some suggestions besides Marlon's great feedback again^:

    • You don't need the first div that captures the whole layout. You could just use the body tag on this one. Also a preferred structure would be:
    <body>
  <header />
  <main />
  <footer />
</body>

    This way, all content will be inside a landmark elements.

    • Just to say it first, making carousel to be accessible is really hard, just pointing it out :>
    • The website logo shouldn't be inside a nav element, since it is not really a navigational link of your website. Only those 4 elements after it is needed.
    • The alt value for the img of the website logo should have been alt="room", since the website itself is named by that, better using it as a value for the website-logo's alt attribute.
    • Always use only 1 h1 per webpage. It would be great if the h1 on this would be a sr-only text. This means the text will only be visible to screen-reader users.
    • shop now should be a link using a tag.
    • The button to change the carousel should have aria-label defining what the button does. For example, the button for the next carousel item, it should be:
    <button aria-label="next carousel item"> </button>

    This way, there is an information on what this button will do.

    MOBILE

    • The hamburger menu should have been using button element and not just div. Using just div makes it un-accessible for keyboard and screen-reader users. Always use interactive html element for an interactive component.
    • The button that you will use in the hamburger should have aria-label="navigation menu dropdown" so that users will know what this button does. The button should also have aria-expanded="false" as default value, and you will toggle it to true by javascript, if the user toggles the button, then you just cycles it.

    Aside from those, really great job on this one.

    Marked as helpful
    1
  • Marlon Passos•940
    @MarlonPassos-gitPosted about 3 years ago

    hello, I didn't do this challenge so I'm not aware of the difficulty, but in my head it seems that it would be easier to do if you had used CSS GRID, I don't know but it seems.

    • where it says "SHOP NOW" should be an anchor element that would take the user to another page, so I would put it inside an <a> tag

    • It may seem strange, but it is recommended that you put in your images some size in the html, as this prevents the page does not move when loading

    • "ABOUT OUR FURNITURE" could be a <h2>, it doesn't make sense for you to use an h3 on the page if you didn't use h2, never skip

    • It would be nice if we could switch images using the arrow keys on the keyboard.

    • I see a few HTML issues in the report, perhaps it's worth taking a look at them!

    otherwise I liked it, it was very similar to the original version

    Marked as helpful
    1
  • Adil Akothiat•205
    @Adil-AkothiatPosted about 3 years ago

    => Thanks all for this great ideas, I will work on it and improve the live site in the future.

    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