fix(lists): added role to list elements within components #8702
Conversation
|
Preview: https://patternfly-react-pr-8702.surge.sh A11y report: https://patternfly-react-pr-8702-a11y.surge.sh |
thatblindgeye
force-pushed
the
iss8453_listRole
branch
from
211ae3d to
50813c1
Compare
thatblindgeye
force-pushed
the
iss8453_listRole
branch
from
50813c1 to
8a31d76
Compare
|
Do we care about this one on the wizard? This is the current wizard - it looks OK on wizard next.
|
mcoker
left a comment
mcoker
left a comment
There was a problem hiding this comment.
Left a comment about the existing wizard - the nested menus are missing a role. Though looking a little closer at the main nav list, even though we have visible list item numbers, those are pseudo elements, and we have list-style: none on the list to hide the default markers. I'm assuming that will need role="list", too?
thatblindgeye
force-pushed
the
iss8453_listRole
branch
from
8a31d76 to
53b793f
Compare
|
@mcoker nice catch! Added the role to the two remaining Wizard lists |
|
just needs a rebase to get the tests passing. Does this need a code mod? |
thatblindgeye
force-pushed
the
iss8453_listRole
branch
from
377e987 to
29e13ba
Compare
|
@nicolethoen I don't think it will. Any update shouldn't really affect any tests or anything like that |
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
6 participants
What: Closes #8453
Mainly added
role="list"where applicable (also updating snapshots), but also exposed/added aria-label prop to several components.I added
role="listitem"to ProgressStep as the semantic role was being removed bydisplay: contentsin the CSS. Adding this role isn't a perfect fix as it now announces list items as "groups" rather than list items due to the aria labeling we have onli.pf-c-progress-stepper__stepand.pf-c-progress-stepper__step-title. cc @mcoker looks like this was mentioned when the component was created: https://github.com/patternfly/patternfly/pull/4357/files#r703742528Additional issues: