Skip to content
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

Tabs: tweak sizing and overflow behavior of TabList #64371

Merged
merged 68 commits into from
Sep 30, 2024

Conversation

DaniGuardiola
Copy link
Contributor

@DaniGuardiola DaniGuardiola commented Aug 8, 2024

What?

This PR modifies the behavior of Tabs.TabList in regards to sizing and overflow to address some needs.

Why?

We have needs around proper management of overflowing tabs (#64093) and tab lists that expand to fill their container (#61974 (comment)).

How?

  • It makes the TabList have a default width of fit-content (only for horizontal orientation). This means that tabs will only take up the space needed for their content, and the tab list (container) will adjust its width to the space needed for the sum of its child tabs.
    • This is set with :where for reduced specificity, in order to make overrides easier.
  • It sets overflow-x to auto in the TabList, making it overflow horizontally only when necessary.
  • It makes tabs expand (flex-grow: 1) to fill up additional available space when the container (TabList) has it. Note that due to TabList having width: fit-content by default, this won't happen unless the user intentionally modifies the width, for example by setting width: 100% to fill up its container's size.
  • It centers the contents of a tab horizontally (justify-content: center, only for horizontal orientation).

The TL;DR is:

  • Tabs will scroll when necessary.
  • Tabs hug contents by default.
  • If a "fill" style is wanted, just set the TabList width you want and tabs will adapt gracefully.

There is an additional detail that is important now that the tab list scrolls: when a tab is selected, it is programmatically scrolled into view.

Finally, I've gone through every single instance of Tabs and applied any necessary tweaks to retain the same styles. Everything should look exactly the same after these changes.

Testing Instructions

Open the storybook locally (npm run storybook:dev) and go to the "Tabs > Size And Overflow Playground" story. Play with it. Also, check that none of the instances of this component in Gutenberg are affected (see below).


Where to find all Tabs instances for testing:

  1. This one requires a small tweak in the code, replace canChooseAColor && canChooseAGradient with true in gutenberg/packages/block-editor/src/components/colors-gradients/control.js, then open post editor > select some text > go to "Block" in right sidebar > click "Background" (it'll be the "second" tablist below the first one)
  2. Open post editor > select some text > go to "Block" in right sidebar > click "Background"
  3. Open post editor > open block inserter > check inside "Patterns" and "Media" (vertical tabs)
  4. Open post editor > add a "navigation block" and select it > go to "Block" in the right sidebar (tabs with icon labels)
  5. Open post editor > open block inserter and document overview
  6. (With a "block" theme set like Twenty Twenty-Four) Appearance > Editor > Styles > Edit styles (pencil icon button) > Select a palette
  7. (With a "block" theme set like Twenty Twenty-Four) Appearance > Editor > Styles > Edit styles (pencil icon button) > Typography > Manage fonts
  8. (With a "block" theme set like Twenty Twenty-Four) Appearance > Editor > Styles > Edit styles (pencil icon button) > Style Book (eye icon button)
  9. (With a "non-block" theme set like Twenty Fifteen) Appearance > Widgets (tabs i right sidebar)
  10. Open post editor > open right sidebar
  11. Open post editor > select some text > open "more" dropdown in the floating menu > Highlight
  12. Open post editor > open options dropdown menu (vertical dots icon button) > Preferences (vertical tabs)

Testing Instructions for Keyboard

In the same story, you can tab into the tab list and use arrows to move.

Screenshots or screencast

Kapture.2024-08-08.at.14.04.04.mp4

@jasmussen
Copy link
Contributor

Have a look also at the custom-scrollbars-on-hover mixin, it may be a good fit here.

@DaniGuardiola DaniGuardiola self-assigned this Aug 8, 2024
@DaniGuardiola DaniGuardiola changed the title Tabs: tweak sizing and overflow behavior of TabList. Tabs: tweak sizing and overflow behavior of TabList Aug 8, 2024
@DaniGuardiola DaniGuardiola added the [Type] Feature New feature to highlight in changelogs. label Aug 8, 2024
@DaniGuardiola
Copy link
Contributor Author

DaniGuardiola commented Aug 8, 2024

I checked every instance, but I couldn't find these two in the product, I'd appreciate help finding them so I can test them:

@DaniGuardiola
Copy link
Contributor Author

@jasmussen that's SASS-only, which we don't have access to from this component. This would be easier to use if implemented in vanilla CSS, e.g. a utility class that uses CSS custom properties (A.K.A. CSS variables) as parameters.

@DaniGuardiola DaniGuardiola added the Needs Design Feedback Needs general design feedback. label Aug 8, 2024
@DaniGuardiola DaniGuardiola marked this pull request as ready for review August 8, 2024 13:37
Copy link

github-actions bot commented Aug 8, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: DaniGuardiola <daniguardiola@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@DaniGuardiola
Copy link
Contributor Author

@ciampo the original approach was actually that (to track the selected element). I switched to the active one to cover for selectOnMove, however after iterating a bit I found out that we can indeed get away by tracking the selected element, since the active element in all relevant cases always receives focus, and that already causes the container to scroll the element into view. I added the scroll margin back in the styles so that the behavior is similar in both cases.

In my testing I couldn't see anything misbehaving. I tested with different Mac scrollbar settings, different means of interaction, and different combinations of prop values (e.g. selectOnMove). Let me know if the fix also works for you 🤞

@DaniGuardiola
Copy link
Contributor Author

@tyxla fyi I made the inserts > patterns/media tabs changes more minimal in e80f76a since they altered the aspect too much. The vertical tabs styles are being overhauled in #65387 anyway so this makes for less changes in this PR.

Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

I did a round of testing on Storybook and in the editor and couldn't find any issues — hopefully I didn't miss anything.

Let's solve the conflicts, merge, and iterate!

Thank you for the patience while working on this PR, @DaniGuardiola 🙏

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in a comment, let's not forget to follow-up with a refactor of the style book layout that ideally doesn't rely on absolute positioning 👌

@DaniGuardiola DaniGuardiola enabled auto-merge (squash) September 30, 2024 18:51
@DaniGuardiola DaniGuardiola merged commit 37bceff into trunk Sep 30, 2024
64 checks passed
@DaniGuardiola DaniGuardiola deleted the feat/tablist-sizing-and-overflow-improvements branch September 30, 2024 19:25
@github-actions github-actions bot added this to the Gutenberg 19.4 milestone Sep 30, 2024
getdave pushed a commit that referenced this pull request Oct 1, 2024
* Tweak sizing and overflow behavior of TabList.

* Add size and overflow playground story.

* Add "scroll into view" behavior to selected tabs.

* fit-content only on horizontal orientation

* Reduce specificity of `fit-content` to make it easier to override.

* centered label only when orientation is horizontal

* Remove unused file.

* Fix inspector controls tabs.

* Fix font library modal tabs.

* Fix style-book tabs.

* fix typo

* Add changelog entry.

* fix emotion being weird ugh

* Prevent unwanted focusable container in Firefox.

* Add fade effect.

* Fix IntersectionObserver logic.

* Feature detect IntersectionObserver to prevent tests from failing.

* Add a bit of tolerance for scroll state detection.

* Fix vertical indicator.

* Better handling of vertical overflow.

* Add a bit of scroll margin for better "scroll into view" experience.

* Horizontal fade should only happen on horizontal direction.

* Adjust for offset parent scroll state in `getElementOffsetRect`.

* Better "scroll into view" positioning heuristics ("nearest").

* Invert use of before and after to remove z-index and fix related issues.

* Make vertical indicator light blue as discussed.

* Undo most overrides in pattern/media vertical tabs.

* Clean up outdated styles previously needed for label wrapping.

* Revert vertical indicator changes and some indicator patterns/media tabs styles

* Revert vertical indicator bug fix

* Add changelog entry

* Remove outdated style.

* Address feedback

* Fix scroll bug

* Improve automatic tab scrolling behavior.

* Tweaks to prevent unit test failure and minor cleanup.

* Undo unnecessary changes.

* Improved story

* Fix scroll jumping bug.

* Scroll to active tab instead of selected (support `selectOnMove=false`).

* Fix minor visual glitch with overflow fade out indicators.

* Misc tweaks

* Fix.

* Fix changelog

* Fix changelog but it's actually true

* Fix changelog

* Make Story Book tabs nicer.

* Temp fix for scrollbar issue in Style Book tabs.

* Fix scroll bug and clean up a little.

* Simplify and clean up a bit more.

* Fix merge issues.

* Fix merge issues again.

* Make inserter patterns/media changes more minimal

* Fix outdated comment

* Fix another typo in comment.

* Minor cleanup.

* Fix bad automatic merge.

* ugh, fix again

Co-authored-by: DaniGuardiola <daniguardiola@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>
huubl pushed a commit to huubl/gutenberg that referenced this pull request Oct 2, 2024
* Tweak sizing and overflow behavior of TabList.

* Add size and overflow playground story.

* Add "scroll into view" behavior to selected tabs.

* fit-content only on horizontal orientation

* Reduce specificity of `fit-content` to make it easier to override.

* centered label only when orientation is horizontal

* Remove unused file.

* Fix inspector controls tabs.

* Fix font library modal tabs.

* Fix style-book tabs.

* fix typo

* Add changelog entry.

* fix emotion being weird ugh

* Prevent unwanted focusable container in Firefox.

* Add fade effect.

* Fix IntersectionObserver logic.

* Feature detect IntersectionObserver to prevent tests from failing.

* Add a bit of tolerance for scroll state detection.

* Fix vertical indicator.

* Better handling of vertical overflow.

* Add a bit of scroll margin for better "scroll into view" experience.

* Horizontal fade should only happen on horizontal direction.

* Adjust for offset parent scroll state in `getElementOffsetRect`.

* Better "scroll into view" positioning heuristics ("nearest").

* Invert use of before and after to remove z-index and fix related issues.

* Make vertical indicator light blue as discussed.

* Undo most overrides in pattern/media vertical tabs.

* Clean up outdated styles previously needed for label wrapping.

* Revert vertical indicator changes and some indicator patterns/media tabs styles

* Revert vertical indicator bug fix

* Add changelog entry

* Remove outdated style.

* Address feedback

* Fix scroll bug

* Improve automatic tab scrolling behavior.

* Tweaks to prevent unit test failure and minor cleanup.

* Undo unnecessary changes.

* Improved story

* Fix scroll jumping bug.

* Scroll to active tab instead of selected (support `selectOnMove=false`).

* Fix minor visual glitch with overflow fade out indicators.

* Misc tweaks

* Fix.

* Fix changelog

* Fix changelog but it's actually true

* Fix changelog

* Make Story Book tabs nicer.

* Temp fix for scrollbar issue in Style Book tabs.

* Fix scroll bug and clean up a little.

* Simplify and clean up a bit more.

* Fix merge issues.

* Fix merge issues again.

* Make inserter patterns/media changes more minimal

* Fix outdated comment

* Fix another typo in comment.

* Minor cleanup.

* Fix bad automatic merge.

* ugh, fix again

Co-authored-by: DaniGuardiola <daniguardiola@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>
huubl added a commit to huubl/gutenberg that referenced this pull request Oct 2, 2024
@stokesman
Copy link
Contributor

Dropping a comment here because it seems #65837 is due to these changes. Not saying this PR did anything wrong and perhaps it’s just a follow up needed to fix a detail of Tabs usage in the Inserter.

draganescu pushed a commit that referenced this pull request Oct 2, 2024
* Tweak sizing and overflow behavior of TabList.

* Add size and overflow playground story.

* Add "scroll into view" behavior to selected tabs.

* fit-content only on horizontal orientation

* Reduce specificity of `fit-content` to make it easier to override.

* centered label only when orientation is horizontal

* Remove unused file.

* Fix inspector controls tabs.

* Fix font library modal tabs.

* Fix style-book tabs.

* fix typo

* Add changelog entry.

* fix emotion being weird ugh

* Prevent unwanted focusable container in Firefox.

* Add fade effect.

* Fix IntersectionObserver logic.

* Feature detect IntersectionObserver to prevent tests from failing.

* Add a bit of tolerance for scroll state detection.

* Fix vertical indicator.

* Better handling of vertical overflow.

* Add a bit of scroll margin for better "scroll into view" experience.

* Horizontal fade should only happen on horizontal direction.

* Adjust for offset parent scroll state in `getElementOffsetRect`.

* Better "scroll into view" positioning heuristics ("nearest").

* Invert use of before and after to remove z-index and fix related issues.

* Make vertical indicator light blue as discussed.

* Undo most overrides in pattern/media vertical tabs.

* Clean up outdated styles previously needed for label wrapping.

* Revert vertical indicator changes and some indicator patterns/media tabs styles

* Revert vertical indicator bug fix

* Add changelog entry

* Remove outdated style.

* Address feedback

* Fix scroll bug

* Improve automatic tab scrolling behavior.

* Tweaks to prevent unit test failure and minor cleanup.

* Undo unnecessary changes.

* Improved story

* Fix scroll jumping bug.

* Scroll to active tab instead of selected (support `selectOnMove=false`).

* Fix minor visual glitch with overflow fade out indicators.

* Misc tweaks

* Fix.

* Fix changelog

* Fix changelog but it's actually true

* Fix changelog

* Make Story Book tabs nicer.

* Temp fix for scrollbar issue in Style Book tabs.

* Fix scroll bug and clean up a little.

* Simplify and clean up a bit more.

* Fix merge issues.

* Fix merge issues again.

* Make inserter patterns/media changes more minimal

* Fix outdated comment

* Fix another typo in comment.

* Minor cleanup.

* Fix bad automatic merge.

* ugh, fix again

Co-authored-by: DaniGuardiola <daniguardiola@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>
@DaniGuardiola
Copy link
Contributor Author

DaniGuardiola commented Oct 2, 2024

@stokesman thanks for bringing this to my attention, I'm investigating.

Edit: this was fixed in #65387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Type] Feature New feature to highlight in changelogs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants