@mattstuddert
Posted
Hey Mehdi, nice work on this challenge! Here are some tips after taking a look at your code:
- You're using
padding
to separate the content from the edges of the screen. I'd recommend using a containing element instead. A common practice is to have a.container
class responsible for constraining the content to a specific width. This class can then be reused through the page whenever content needs to be contained. Although the actual values might change, the styles would look something like this:
.container {
margin: 0 auto;
max-width: 1100px;
width: 90%;
}
- Avoid setting click listeners on non-interactive elements, like the
div
element. These can't be accessed by anyone not using a mouse/trackpad to navigate the content, which is a bad practice. Instead, add click listeners to interactive elements likea
orbutton
. This will ensure the element is focusable and accessible to people not using a mouse/trackpad. - For a lot of the content, you're uppercasing the content in the HTML itself. Try to avoid doing that as some screen reader software will read this content letter-by-letter, making that content inaccessible to screen reader users. Instead, write it normally in your HTML and then use
text-transform: uppercase;
in your CSS to visually uppercase the text to match the design. - I'd recommend trying to clear the errors in the accessibility and HTML validation reports. If you're not sure what to do, try Googling the error message. Once you think you've resolved the issue, you can generate a new report to see if it clears the issue.
- You're not using any headings elements at the moment. Headings are important for creating a proper content hierarchy. This is especially helpful for screen reader users, as screen readers will use headings to structure the content. I'd recommend adding headings to add this hierarchy.
I hope this helps! Keep it up and let me know if you have any questions 🙂