Skip to content
OverviewSolutions
2
Comments
3
P
Ray
@raygdev

All comments

  • P
    Ray•70
    @raygdev

    Nice job on your work! I like your choice of the details and summary elements for the accordion.

    Some suggestions around semantics:

    1. The article element should go inside the main element.

    2. The header should be a sibling to main

    3. It would improve semantics if you wrapped each of the details elements in li elements with a parent ul.

    Bonus accessibility:

    You have semantic elements so using the tab keys and enter/space work out of the box. That is the bare minimum for accessibility. It would improve accessibility if you provided more keyboard controls by allowing the user to

    1. Use the up arrow key to navigate the list

    2. Use the down arrow key to navigate the list

    3. Use the home key to focus the first detail in the list

    4. Use the end key to focus the last detail in the list.

    Other than those small improvements, this is great overall!

    Marked as helpful
  • Hossein Heydari•560
    @Hossein-H-I
    What are you most proud of, and what would you do differently next time?

    I create my first landing page

    What specific areas of your project would you like help with?

    Any feedback is welcome.

    Agency landing page solution

    3
    P
    Ray•70
    @raygdev

    Overall the html looks good! I would like to make a suggestion about the multiple headers. There should only really be one header per page. For your navigation, it's good, but I would remove the other ones. They would be announced to screen readers and likely confuse or annoy them.

    Nice work on the navigation for mobile. It took me through some frustration trying to get that top right corner right! I would like to make some suggestions about your hamburger and some accessibility issues with your navigation:

    1. The hamburger image should be wrapped in a button. As it stands, AT users wouldn't know that it was an interactive element because it's announced as an image with it's alt text and not a button.

    2. Change the alt text in that image to something like "toggle menu" instead of just "menu". The alt text will become the accessible name (what is read to screen readers) of the button it is wrapped in.

    3. You need to have the button connected to the navigation that is not in view on mobile (your ul). This is done by:

    • giving the ul and id
    • providing an aria-controls that is equal to the ul id.
    • providing an aria-expanded that you can toggle true or false based on if the ul is visible or not.

    There are other things to consider as well but aria authoring practices guide is a good place to start

    Other things to note...

    For your footer:

    1. Your svgs should have an aria-hidde="true" on them. Otherwise they will be read to screen readers.

    2. Your links that wrap your svgs should have accessible names. You can do this two ways:

    • add a span before your svg with text like "Sunnyside facebook page" and the like for the rest of them and visually hide them
    • alternatively, add aria-label and the value of the text you would like to be the text for the link. For instance aria-label="Sunnyside Facebook page"

    Other than those things around accessibility, it looks great! Nice work!

  • Raymond023•290
    @Raymond023
    What specific areas of your project would you like help with?

    Tear my accessibility down and give me a good feedback... Cheers

    Responsive FAQ Accordion

    #accessibility#jquery
    3
    P
    Ray•70
    @raygdev

    I think this would be better semantics if:

    1. You remove the section elements all together.

    2. Instead of using a header element, remove it and wrap the entire FAQ in an article tag as the top-level element.

    3. For each question, wrap everything in a details element and a nested summary element with your question and a p element as a sibling to the summary (this will be hidden by default). This would be more semantic usage and gives you the benefit of reducing or eliminating your need for javascript. It will recieve focus, and listen for events (keyboard navigation, enter, space, etc) out of the box.

    4. I would leave the alt attributes as empty strings since they don't really serve any other purpose other than decoration.

    If you want to continue with the pattern that you have, I'd still remove the sections.

    1. Wrap each faq question and answer in a div as their parent element.

    2. still keep the image alt attributs as an empty string for the buttons.

    3. Remove the tab-index attributes from the heading elements (your buttons will recieve default tab order).

    4. still wrap everything in an article element.

    5. Have a single button nested in your heading element for each question. The question text should be children of the button, along with the images. You can toggle the display of the image based on the button state. This still gives you default button/browser behavior.

    6. Change the button aria-label To something more descriptive like toggle details.

    7. Have an aria-controls attribute that has the same name as the p element that is either visually hidden or display none

    8. Provide an id for each p element.

    9. Have an aria-hidden attribute thay you can toggle to true or false in javascript if the details area is open or closed.

    There are probably a few things I am missing, but this is, to the best of my knowledge, a good way of doing it.

    Personally I'd prefer the first option since it is more semantic and will likely eliminate the need for js. It is also automatically accessible rather than having to rely on aria-* attributes.

    Hopefully this helps!