Try: Remove segmented control vertical separators #35497
Conversation
There was a problem hiding this comment.
I always like PRs that remove code! Also, as @mtias pointed out, we don't seem to have designs using separators.
My only worry is what @jasmussen also pointed out — that having fixed size buttons with different amounts of text will cause uneven spacing and confuse the user.
We should also be careful and ensure that the space between "toggle buttons" is always large enough to make it clear that it denotes a separation between buttons. Otherwise the user may be getting confused in case a button's label text has spaces.
I think that we may go ahead and apply these changes (especially given the fact that this component is still experimental). We can always add the separator back at a later point.
Should we consider giving each ToggleGroupControlOption a hover background color to make the separation clearer?
Finally, we should probably add a CHANGELOG entry for this change?
|
Thanks for the reviews. Looks like I have some snapshots to add.
I like that, I always like to start simple and add as elements become necessary.
In fact I think we should add an entry to the component readme that this is a bit of an "editorial responsibility". It's an opinionated opponent, and IMO should never be used if there's ever a chance for the labels, whether in English or another language, to wrap. |
|
I updated the snapshots, and I added a small comment in the README about usage. I'd love input on whether I did that right, and I'll be sure to follow up.
Where would this update go? |
My bad, I thought I linked it already. It would be |
|
Not at all, and thank you. It's a slow Monday for me — I looked for a changelog section in the README 🙈 I pushed a change, let me know if that looks good. |
packages/components/CHANGELOG.md
Outdated
| @@ -2,6 +2,10 @@ | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| ### Change | |||
There was a problem hiding this comment.
Not sure I've seen a Change section in the changelog before. Maybe we can call it Enhancements for now? 🤷
packages/components/CHANGELOG.md
Outdated
| @@ -2,6 +2,10 @@ | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| ### Change | |||
|
|
|||
| - Removed the separator shown between items. ([#35497](https://github.com/WordPress/gutenberg/pull/35497)). | |||
There was a problem hiding this comment.
We can probably be a bit more specific
| - Removed the separator shown between items. ([#35497](https://github.com/WordPress/gutenberg/pull/35497)). | |
| - Removed the separator shown between `ToggleGroupControl` items ([#35497](https://github.com/WordPress/gutenberg/pull/35497)). |
| @@ -6,6 +6,8 @@ This feature is still experimental. “Experimental” means this is an early im | |||
|
|
|||
| `ToggleGroupControl` is a form component that lets users choose options represented in horizontal segments. To render options for this control use `ToggleGroupControlOption` component. | |||
|
|
|||
| Only use this control when you know for sure the labels of items inside won't wrap. For items with longer labels, you can consider a Dropdown component instead. | |||
There was a problem hiding this comment.
The Dropdown component is probably not what you meant here — did you mean SelectControl and/or CustomSelectControl?
In any case, if we mention a component's name, it's better to wrap it in backticks for better formatting.
Otherwise, the alternative here could be to be more generic and just mention "you can consider a select/dropdown component instead".
WDYT?
ciampo
added
[Package] Components
Co-authored-by: @ciampo
|
Nice, thank you. I pushed some small changes to address your feedback. |
|
@ciampo if you have a second for a last sanity check, then I can land this one. Thanks again. |
Description
Inspired by #35395 (comment), this PR removes the vertical separators from the Segmented Control entirely, as in most examples of its usage I have found them to only add noise.
Before:
After:
Especially in the linked font size example where font sizes are labelled in 2 letter same-width abbreviations, this works especially well.
However there's an argument to be made that in the Featured Image example shown here, where label widths aren't uniform but buttons are still uniform-width, the uneven space between items is a downside.
Let me know your thoughts.
How has this been tested?
Insert the Featured Image block, add a width, then observe the segmented control.
Checklist:
*.native.jsfiles for terms that need renaming or removal).