Bookmark landing page with flexbox and vanilla js

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hi Zajac,

  Congratulation on completing another FM challenge, I have some suggestions regarding your solution:

  HTML

  • In the logo , the SVG does not contain any visible text that describes the graphic, we need to add the alternative text (invisible) by:

  1. <title>A short title of the SVG</title>
  2. Add the appropriate ID’s to the <title>.
  3. It must be the first child of its parent element On the <svg> tag, add aria-labelledby="uniqueTitleID”. The same for the footer’s logo.

  1- The button class="hamburger" needs to have an aria-label attribute or ansr-onlytext that describes the button purpose.` For example, you can have: aria-label='Mobile Navigation Trigger' or 'Open Menu.’

  2- Adding aria-expanded that, the user will be able to know that the button content is expanded or collapsed. At first, it has the “false” as a value then you use JavaScript to change the value.

  3- You should use aria-controls attribute on the toggle element, it should reference the id value of the ``<ul> ```element.

  • Avoid creating duplicate content (duplicate navigation). You can style the same navigation in mobile and desktop differently using media queries. Practice like this can result in a poor user experience, when a visitor finds substantially the same content repeated within a set of search results.

  • 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 an aria-hidden=”true” to remove that element from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative images for example bg-dots, social svgs, ..

  • The alternate text for the Google, Firefox and Opera are needed. As they are informative images, you can use the company’s name.

  • You might already be aware of this, but you can use the native HTML details and summary elements to quite easily make interactive accordions that are fairly accessible.

  • In btn-card, btn-small A button with no type attribute acts as type=”submit”, and will attempt to submit form data when clicked. Be explicit in your intentions and kind to future developers working with your code: provide a type. By specifying either button, submit or reset, the code’s purpose is clear and is easier to maintain

  • Forms with proper inputs and labels are much easier for people to use. To pair the label and input, one way is an explicit label’s for attribute value must match its input’s id value. Input fields without accompanying labels can lead to accessibility issues for those who rely on screen readers. If a screen reader comes across an input field without a label it will try to find some accompanying text to use as the label. (To hide the label visually but present for assistive technology, you may use sr-only class ).

  • Instead of using a generic div to wrap the social links , you put your links within an unordered list structure <ul> so that a screen reader will read out how many things are in the list to give visually impaired users the most information possible about the contents of the navigation.

  • In this instance social icons, they do not navigate the user anywhere (they are not clickable). You should use interactive element <a>, for example: <li><a href=”#”><svg aria-hidden=”true” focusable=”false”>...</svg></a></li>

  • Links must have discernible text. The social links wrapping the icons must have aria-label or sr-only text indicate where the link will take the user. Within them the svgs, they are purely decorative, you'll need to manually add an aria-hidden attribute and focusable=”false” to each of your svgs.

  [You may like to check my solution to the same challenge, it might help and if you have any recommendation please leave a comment .] (https://www.frontendmentor.io/solutions/bookmark-landing-page-mobile-first-lSSOu4P-bg)

  Aside these, Great job on this one . Hopefully this feedback helps.

  • Never use <div> alone to wrap a meaningful content. Just keep in mind that you should usually use semantic HTML in place of the div tag unless none of them (the semantic tags) really match the content to group together.* By adding semantic tags to your document, you provide additional information about the document, which aids in communication.

  • For the reason stated before, instead of using a generic div in <div class="cta__contact-us">, use <form>

  • The social links wrapping the icons must have aria-label or sr-only text indicate where the link will take the user. Then you set aria-hidden =”true” to the icons to be ignored by assistive technology.

  Aside these, you did great work. Hopefully this feedback helps.

  Marked as helpful

  1
• 

  Flávio Emanuel•0

  @flamssdevPosted over 2 years ago

  Hey Zajac, I'm just starting and I was having trouble creating an FAQ, you helped me a lot with your clean code, you made me not give up.

  1

  ---

  

  Zajac•150

  @Zajaczkowski23Posted over 2 years ago

  @flamssdev I'm glad that i could help you. :D

  0