Components: Add ItemGroup and Item #30097
Conversation
sarayourfriend
requested review from
ajitbohra and
chrisvanpatten
as code owners
sarayourfriend
added
[Feature] Component System
|
Size Change: +236 kB (+15%) Total Size: 1.86 MB
ℹ️ View Unchanged
|
|
It was such a blast 🍐 'ing with you on this @sarayourfriend :D. Going through the PR now. I think it was awesome that we got an initial implementation of nested + collapsible lists working in the story:
For now, I think we should remove it from the story (until we can refine + create a better working implementation) |
|
Thoughts on design: The original new Menu component was designed to work as a (sub) component of Dropdown. We want to carry that spirit within this new/updated implementation of That Menu component was intentionally designed (aesthetically) to look like the current Gutenberg dropdown:
In that case, we may need to make some style adjustments. |
|
I did a quick test, and I was able to get these new It's too much effort to get it working 100% in the Story. I did enough to ensure the integration, features, and mechanics work 👍 |
| @@ -4,6 +4,7 @@ | |||
| import { createContext, useContext } from '@wordpress/element'; | |||
|
|
|||
| export const ItemGroupContext = createContext( { size: 'medium' } as { | |||
| spacedAround: boolean; | |||
There was a problem hiding this comment.
Shall we give this a default value in the context, just for consistency?
There was a problem hiding this comment.
We could! I wasn't sure how to go about this, since it's kind of inferred from bordered / separated 🤔
|
I've pushed up some styling updates to prepare the component to replace
Note: The current I've also updated the "Non action" story to showcase some alternate UIs we can build using this component:
|
✋ Question about semantics +
|
|
Hey! Great work! I don't think we should render Also, the following markups are not valid: // button is not read as a button if it has role listitem
<div role="list">
<button role="listitem" />
<button role="listitem" />
</div>// ul requires children to be li's
<ul>
<button />
<button />
</ul>This makes the <ItemGroup action>
<Item />
<Item />
</ItemGroup>That would render this: <div>
<button />
<button />
</div>That's the best markup I can think of for a group of buttons. But I wonder if it wouldn't be better as a separate component. I would expect it to be |
|
@diegohaz Thank you for your thoughts!! (Humour me for a second, haha ❤️) If you have the time, I'd love to know how you would approach the construction of these use cases - specifically from a semantic/a11y perspective and a React component API perspective. To clarify, I'm mostly trying to understand the best rendered HTML markup. List of actions (or links) HTML <div>
<nav>
<ul>
<li><a>Link</a></li>
<li><a>Link</a></li>
</ul>
</nav>
<hr />
<ul>
<li><button>Action</button></li>
<li><button>Action</button></li>
</ul>
</div>React component: <div>
<nav>
<List>
<ListItem as={NavLink} to="/url">Link</ListItem>
<ListItem as={NavLink} to="/url">Link</ListItem>
</List>
</nav>
<Divider />
<List>
<ListItem action onClick={...}>Link</ListItem>
<ListItem action onClick={...}>Link</ListItem>
</List>
</div>List of non-actionable items HTML <ul>
<li>...</li>
<li>...</li>
<li>...</li>
</ul>React component: <List>
<ListItem>...</ListItem>
<ListItem>...</ListItem>
<ListItem>...</ListItem>
</List>Complex list of actionable items, with inner actions HTML <ul>
<li>
<input type="checkbox" />
...
<button>...</button>
</li>
...
</ul>React component: <List>
<ListItem action onClick={...}>
<Checkbox />
...
<Button>...</Button>
</ListItem>
...
</List>Thank you!! |
|
I tried giving this a test and the G2 components test page is so inaccessible. There are so many unlabeled buttons and links, it's hard enough finding the component to test. Once I finally found it, once again, the iframe with Mac's Voiceover doesn't play well. I wrote all this to say there's really no way to pass this accessibility wise until a more suitable testing environment is created. If your testing environment is inaccessible from the start, it's tough to tell what the new feature is. When I clicked the dropdown it said something about a bunch of random letters web dialog opened. Again, not sure if this is just a side-effect with the iframe or something is going wrong with the dialog that opens. Thanks. |
|
@alexstine Haii! Thank you for trying to help!
The new component is just a general (consistent) way to render a list of items. At this moment, we're not ready for a real accessibility run through, as we're still figuring out the correct structure (convos above with @diegohaz ). We are planning on using it for a new Dropdown component. However, the one from Storybook was just an initial test to see if it would even work. Perhaps we should switch the status of this PR to draft - since we're still actively working on it. Your comment has raised an important point. (cc'ing @sarayourfriend) |
If they're all navigation links, but it makes sense to have some distinction between the groups of links, we can have multiple nav elements: <div>
<nav aria-label="Primary folders">
<ul>
<li><a>Inbox</a></li>
<li><a>Drafts</a></li>
</ul>
</nav>
<nav aria-label="Secondary folders">
<ul>
<li><a>Trash</a></li>
<li><a>Spam</a></li>
</ul>
</nav>
</div>But, if Trash and Spam are buttons, just wrap them within a div: <div>
<nav>
<ul>
<li><a>Inbox</a></li>
<li><a>Drafts</a></li>
</ul>
</nav>
<div>
<button>Move to Trash</button>
<button>Move to Spam</button>
</div>
</div>
This looks like a table. <table aria-label="Items">
<tbody>
<tr>
<td>
<input type="checkbox" aria-labelledby="title-1" />
</td>
<td>
<a href="#" id="title-1">Line item 1</a>
</td>
<td>
<button>...</button>
</td>
</tr>
<tr>
<td>
<input type="checkbox" aria-labelledby="title-2" />
</td>
<td>
<a href="#" id="title-2">Line item 2</a>
</td>
<td>
<button>...</button>
</td>
</tr>
</tbody>
</table>Or, in the case we want keyboard users to be able to navigate through the table with arrow keys, a |
|
@diegohaz Wow!! That's really helpful!!! I never thought of that layout like a |
Yes! That would look like this: <div role="grid">
<div role="row">
<div role="gridcell">
<input type="checkbox" aria-labelledby="title-1" />
</div>
<div role="gridcell">
<a href="#" id="title-1">Line item 1</a>
</div>
<div role="gridcell">
<button>...</button>
</div>
</div>
</div>If we use |
|
@diegohaz Hmm! Maybe this particular UI I had in mind doesn't fit a |
sarayourfriend
force-pushed
the
add/hooks-list-component
branch
from
aa44a5b to
e57fede
Compare
There was a problem hiding this comment.
I made some adjustments: fixed the Button import in the story and renamed the dropdownMenu story to just dropdown so it's not confused with an actual menu element, which would require different focus management (roving tabindex).
Since this is a very abstract component, I'm still not sure how the use cases could impact its API, but I think we can move forward with it and keep it experimental while we understand better the use cases and make the necessary adjustments.
Description
Adds new
ItemGroupandItemcomponentsObviates the need for #29645.
How has this been tested?
Run Storybook (
npm run storybook:dev) and go to the ItemGroup story (/iframe.html?id=g2-components-experimental-itemgroup--default&viewMode=story).Screenshots
Types of changes
New feature
Checklist: