@PhoenixDev22
Posted
Hello
Congratulation on finishing this challenge. Excellent work! I have few suggestions regarding your solution, if you don't mind:
HTML
- It’s not recommended to add event listener on non-interactive elements
class="menu-open"
. You can use a<button>
with type=”button” around theimg
.
- As it has no discernible name, put an aria-label on your trigger to describe its purpose. For example, you can have: aria-label='Mobile Navigation Trigger' or 'Open Menu.’
- Add and toggle an
aria-expanded
attribute to your toggle element letting the user know if the content the button controls is expanded or not. At first, it has the “false” as a value then you use JavaScript to change the value.
- You should use
aria-controls
attribute on the toggle element, it should reference theid
value of the<ul>
element.
- Don't capitalize in html, let css text transform take care of that. Remember screen readers won't be able to Read capitalized text as they will often read them letter by letter thinking they are acronyms.
- Profile images like that avatar are valuable content images, not decorative, so should have alt text eg " Jennie F"
- look up a bit more about how and when to write alt text on images. Learn the differences with decorative/meaningless images vs important content. For decorative images, you set an empty
alt
to it with anaria-hidden=”true”
to remove that element from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative images.
- You should add
aria-hidden=”true”
to the images that make them be ignored by screen readers to avoid redundancy and repetition.
Aside these, great job on this one. hopefully this feedback helps.
Marked as helpful
@Fahatmah
Posted
@PhoenixDev22 Thank you so much! I've been wanting to improve things in my markup but I don't know where to start or what needs to be improve until you suggested this things. I don't really mind it instead I am happy that someone gave me suggestions.
- For the toggle menu, I only used image because I thought button cannot be nested with an anchor tag that has an image due to the last time in my html issues in my report from a fem challenge
- I don't know what is
aria-expanded
is and how it should be coded in Javascript. Can you give me some example? - And
aria-controls
with the referencing ofid
oful
😅
I have a lot to learn. And I am willing to learn it all. 😊