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

Toolbar: Adjust colors for dark mode support #69278

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

SainathPoojary
Copy link
Contributor

@SainathPoojary SainathPoojary commented Feb 21, 2025

related to #66454

What?

This PR updates the toolbar components ( ToolbarGroup and Toolbar ) to use theme-adaptive color variables instead of hardcoded values, ensuring correct styling in both light and dark modes.

Why?

Previously, the toolbar components used hardcoded colors like $white and $gray-900, leading to inconsistent styling, especially in dark mode. Issues included:
• Toolbars had a white background in dark mode, making elements hard to see.
• Borders didn’t adjust properly to theme colors.
• The ::before pseudo-element used $gray-900, which didn’t adapt well to dark mode.

How?

  • Replaced $white with $components-color-foreground-inverted for better adaptability.
  • Replaced $gray-900 with $components-color-foreground for consistency.
  • Applied changes to:
    • .components-toolbar-group
    • .components-toolbar
    • .components-accessible-toolbar
    • The ::before background overlay.

Testing Instructions

  1. Open Storybook and navigate to the Toolbar component.
  2. Toggle dark mode using the Storybook theme switcher.
  3. Verify that the toolbar, buttons, and icons remain visible and have proper contrast.
  4. Check that borders and background colors adapt correctly in both light and dark modes.

Screenshots or screencast

2025-02-21.20-08-03.mp4

@SainathPoojary SainathPoojary marked this pull request as ready for review February 24, 2025 08:03
Copy link

github-actions bot commented Feb 24, 2025

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: SainathPoojary <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: ciampo <[email protected]>
Co-authored-by: mirka <[email protected]>

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

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components labels Feb 24, 2025
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Considering that color theming was planned for the @wordpress/components, this seems like a good approach.

One thing that bothers me is that it will no longer match the color theme of the popover. I'm not sure if color theming was planned for the popover/dropdown as well:

image

cc @WordPress/gutenberg-components

@ciampo
Copy link
Contributor

ciampo commented Feb 26, 2025

Considering that #44116, this seems like a good approach.

A few considerations:

  • there is definitely an interest in theming UI components in Gutenberg
  • the current implementation is meant to be used inside the @wordrpress/components package. The long-term intent is to develop a more general purpose (with a wider scope) @wordpress/theme package that can be used through the whole Gutenberg repo
  • the long-term goal is that most (if not all) the UI will be themed

One thing that bothers me is that it will no longer match the color theme of the popover. I'm not sure if color theming was planned for the popover/dropdown as well:

As mentioned above, we are a bit in the middle of a transition to theming UI — as such, not every component is currently themed.

As a general rule of thumb, I'd say that popovers and dropdowns should also be themed — although, to make sure, it would be better to get some design feedback (@WordPress/gutenberg-design ), and I'm not sure about how much capacity the design team currently has.

@mirka
Copy link
Member

mirka commented Feb 28, 2025

I'm not sure if color theming was planned for the popover/dropdown

Talking from the technical side, I was planning to experiment with this (i.e. theming across React portals), but I never got to it. It's probably a very separate problem from the scope of this PR, so I wouldn't consider it a blocker for this to land.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants