Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve accessibility #49

Open
marcvangend opened this issue Mar 23, 2020 · 6 comments
Open

Improve accessibility #49

marcvangend opened this issue Mar 23, 2020 · 6 comments
Labels
faq Add to readme feature Feature request

Comments

@marcvangend
Copy link

marcvangend commented Mar 23, 2020

Thanks for sharing your code. I tried it - installing was easy and it works as advertised.

I do want to mention though that the resulting HTML structure has a significant impact on accessibility. Wrapping the grid items into column <div>s means that the tab order of the page is changed. Given a 4x3 grid, people who rely on keyboard navigation now have to step through the items in a 1-5-9-2-6-10-3-7-11-4-8-12 order. This is extremely confusing, especially for blind users.

Wrapping the columns in <div>s also means that it is not possible to apply the masonry component to a HTML list (<ul> or <ol>). These tags are often used to build semantic and accessible markup.

IMHO as web developers we have an obligation to build websites for everybody. If there are ways to improve the accessibility of this component, that would be awesome. If there aren't, please mention the impact on accessibility in the readme.

@paulcollett
Copy link
Owner

This is a very valid point. I'd wonder how far tabIndex would get us in this case, and if there are any other attributes/values we can apply?

Happy to help out here with some additional help

@paulcollett paulcollett added feature Feature request help wanted labels May 10, 2020
@marcvangend
Copy link
Author

I have tried to come up with an easy CSS-only way to do it, but I'm afraid I don't have the answer. This CSS Tricks article lists a number of solutions, but no magic bullet.

That said I do know that tabIndex is not the answer. In fact tabIndex is bad for a11y, because the browser will prioritize any element with a tabIndex greater than 0. Quoting MDN:

Avoid using tabindex values greater than 0. Doing so makes it difficult for people who rely on assistive technology to navigate and operate page content. Instead, write the document with the elements in a logical sequence.

@ppsammartino
Copy link

ppsammartino commented Sep 21, 2020

Hi! I actually handled the tab/shift+tab behavior like this:
This works on a stateless component, but adjusting the declarations to use state and 'this' will suffice.
First, I instance an empty array (before the return statement)

let imgRefs = []

'images' is the name of my array of elements to be consumed

<Masonry ...>
    {
        images.map((img, i) =>
            <a href=...
                 key={ i }
                 ref={(r) => {imgRefs[`img-${i}`] = r }}
                     onKeyDown={ e => handleImgKeydown(e, i) }
            >
                <img ... />
            </a>
        )
    }
</Masonry>

This will create refs for every element inside your masonry grid and store it on that imgRefs array.

Next, you can take a look at my 'handleImgKeydown' function, design only to do something when Tab is pressed
You can make it shorter, but for clarity purposes I've spared no line

    const handleImgKeydown = (e, i) => {
    if (e.key === 'Tab'){
      // Determine the sign of the operation.
      // If shift key is pressed, it goes to the previous element
      let interval = 1
      // Check if shift is pressed too
      if(e.shiftKey){
        interval = -1
      }
      const newIndex = i + interval
      // Handle edge cases. 'Images' is the array of elements with the attributes to fill the masonry. Change it to your own.
      if (newIndex >= 0 && newIndex <= images.length){
        // This prevents the tab to jump to the default next element
        e.preventDefault()
        imgRefs[`img-${newIndex}`].focus()
      }
    }
  }

I hope that was helpful!

@sheparddw
Copy link

sheparddw commented Oct 10, 2020

@marcvangend
What I did was add the following prop to the Masonry component:
role="list"
Then within whatever child I place within the Masonry component, I use role="listitem".

This tells the screenreader to treat the container as <ul> and the children as <li> even though the markup is just divs and even though there is a container column, the screenreader ignores this. This fixes the lack of ul/li markup issue for the screenreader for me. As for order, I have not looked at that as in my use case the order of items does not matter.

So
<Masonry role="list"><MyChildComponent role="listitem" /></Masonry>

Maybe this should be added to the Readme?

@marcvangend
Copy link
Author

Thanks for the role="listitem" tip, good to know.

As for the JS to fix keyboard navigation... I appreciate the effort but personally I'm not a big fan of interfering with key presses. (What if someone changed their keyboard shortcuts because their left hand is injured?) Also this doesn't seem to solve screen reader issues.

PS. You might want to know that native CSS masonry is on the horizon.

@paulcollett
Copy link
Owner

This is great @sheparddw, thanks for the find!

I'll be keeping an eye on the native CSS solution, which ideally we can roll into the plugin for supporting browsers - thanks for the link @marcvangend

@paulcollett paulcollett added faq Add to readme and removed help wanted labels Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
faq Add to readme feature Feature request
Projects
None yet
Development

No branches or pull requests

4 participants