-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Expand ItemGroup
#64445
Comments
Nice, this feels mostly exhaustive and a good system. Only a few pieces feel like they are missing, and some of those are question-marks. First off, there's the stacked color-swatch: This feels like a pattern worth keeping. But how much can it stack? Currently in Global Styles > Colors, there's this drilldown: I think that drilldown works fairly well as indication of where you'll go. But does it work in this swatches-only state? Can the pattern be accommodated in these guidelines? Or should this example usage be changed, and if so, to which variant here? |
That same river bugs me too, but making the swatches smaller creates a different kind of river, in the swatches themselves now lining up weirdly. I think it's probably fine to keep the text river. An alternative is to start the text further in, though that might look awkward in its own way. Perhaps grouping the color stacks so they are all at the bottom would make it feel more coherent? |
Yeah a guideline to group by decoration type by default could help 👍 |
Pinging @WordPress/gutenberg-components for any feedback on the designs, as time allows. |
👋 Thank you for starting the conversation on this! I'll list a few thoughts that came up while going through the issue:
|
Based on the conversation in #66413, just going to add that we should consider making |
@ciampo do you think it would be feasible to include hierarchy/nesting in this component? In #65828 there's a suggestion to use |
Noting that the Edit palette with 'multi color-swatches' is being proposed to be removed in #66169:
|
As reported in #67425 it's important to remind everyone that, so far, In fact, by default it renders a div with
Even when Unfortunately, as reported in #67425, at the moment there are many cases in the codebase where
Those usages are semantically wrong. I would appreciate that components enforce rendering correct, semantic, markup but I see that in practice they are often used only for visual purposes. I'd strongly encourage any proposed chang e in this issue to first consider what kind of semantic markup this component should render. If it's a list, then it should be just a plain list with no special keyboard interaction. If it's going to be an ARIA composite widget with only one tab stop and arrow keys navigation, then it should use one of the ARIA design patterns with the expected semantics and interaction but I doubt that in that case the items can contain other interactive elements as proposed in this issue. |
I agree that semantics are central to building any UI, and should be one of the first aspects to consider when designing a component's APIs. I'm open to changing the default semantics of the component — from the designs above, I think we may consider changing it to a composite widget with |
I'm not sure the role=grid would be appropriate and I'm not sure this component should ever provide arrows navigation. A grid is... a grid with vertical and horizontal arrows navigation, while this is a list with vertically stacked items. Also, looking at the actual usage of this component in the codebase (see a few examples reported on #67425)) it appears in many cases ItemGroup has been used just to draw a border around other components. Which is unfortunate. This component was originally meant to be a simple list, and to only be used with |
In recent times
ItemGroup
instances have been extended ad hoc to accommodate required features, for example:–
button to remove the item–
button to remove itemsThe more this components is arbitrarily customised the more tech debt we create. It would be good to define and implement a modern spec based on the use cases outlined above.
The text was updated successfully, but these errors were encountered: