fix(datalist accessibility): h2 used as child element#1870
fix(datalist accessibility): h2 used as child element#1870christiemolloy merged 7 commits intopatternfly:masterfrom jenny-s51:iss1114
Conversation
|
Deploy preview for pf-next ready! Built with commit 0f75170 |
|
Great job with this one! The example looks good! I just have a few suggestions for tweaking the documentation and example label. I'll make inline comments for those... |
| {dataListSimpleExample} | ||
| </Example> | ||
| <Example | ||
| heading="Data list simple h2: screen reader accessible" |
There was a problem hiding this comment.
I suggest naming this example something like "Data list simple with headings", and then leave it to the documentation to explain why headings are beneficial.
| ### Usage | ||
| | Class | Applied to | Outcome | | ||
| | ------ | -- | -- | | ||
| | `.pf-c-data-list__cell` > * | `h2` | Provides better separation of list items by defining the primary content section as a heading. Easy way for screen reader users to navigate to specific sections on the page. Heading level should be based on the context in which the DataList component is being used. E.g., if the heading for the section that contains the DataList is a level 3, then `h4` elements should be used in the DataList items. **Required** | |
There was a problem hiding this comment.
These tables are primarily used to describing what classes or attributes are can or should be used in the component. But this example is just illustrating how the heading element can be incorporated into the datalist html structure.
I would remove the table and just have a paragraph that explains why headings are beneficial. And then the end of the explanation can include a statement like "Refer to the [example label] for an example of how a heading can be used in this component.
Also remove the "Required". This is used to indicate classes or attributes that result in the component not being usable or rendered as expected.
There was a problem hiding this comment.
I take back my suggestion to include this statement, "Refer to the [example label] for an example of how a heading can be used in this component."
Sometimes our component documentation is included at the end of the page, but in this case it's not and is adjacent to the example. So we can leave that statement out.
|
@jgiardino should be all set I think? let me know if there's anything else :) |
jgiardino
left a comment
jgiardino
left a comment
There was a problem hiding this comment.
Your updates look good! Thanks for making these changes!
| export const Docs = docs; | ||
|
|
||
| export default (props) => { | ||
| export default props => { |
There was a problem hiding this comment.
@jenny-s51 not sure where this change came from, but i dont think its part of this PR so you can remove this change?
| @@ -0,0 +1,2 @@ | |||
| ### Usage | |||
| When a list item includes more than one block of content, it can be difficult for some screen reader users to discern where one list item ends and the next one begins. A simple way to provide better separation of list items is to define the primary content section as a heading. Headings are useful for indicating a new section of contents, but also provide an easy way for screen reader users to navigate to specific sections on the page. The heading level should be based on the context in which the DataList component is being used. For example, if the heading for the section that contains the DataList is a level 3, then `h4` elements should be used in the DataList list items. No newline at end of file | |||
There was a problem hiding this comment.
@jgiardino in When a list item includes more than one block of content do you mean when a list includes more than one list item?
There was a problem hiding this comment.
For example, this <li> includes two separate <div>s. When a <li> includes more than one block of content, it might not be obvious which blocks are grouped together inside a list item. Some screen readers like VO announce the beginning/end of a list item, but for some screen readers, this is not announced. Using headings is an easy way to organize the information in a way that's more obvious in some screen readers.
<li class="pf-c-data-list__item" aria-labelledby="simple-item1">
<div class="pf-c-data-list__item-row">
<div class="pf-c-data-list__item-content">
<div class="pf-c-data-list__cell">
<span id="simple-item1">Primary content</span>
</div>
<div class="pf-c-data-list__cell">
Secondary content
</div>
</div>
</div>
</li>
| export const Docs = docs; | ||
|
|
||
| export default (props) => { | ||
| export default props => { |
|
🎉 This PR is included in version 2.12.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
closes #1114 -- adds changes mentioned in part 1.