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

ProgressBar: use text color to ensure enough contrast against background #55285

Merged
merged 3 commits into from
Oct 12, 2023

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Oct 11, 2023

What?

As discussed in #54202 (comment) and following comments, this PR tweaks the colors used for the ProgressBar component (both track and indicator) to be based on the text color.

Why?

Using the text color should ensure that the progress bar always has enough contrast when rendered against the background color.

How?

Testing Instructions

In Storybook:

  • open the ProgressBar story
  • Check that the progress bar is using the current text color and looks good
  • Using the tools in Storybook's top toolbar, change the Theme and observe how ProgressBar changes its color based on the theme
  • Using browser web tool, simulate a high contrast mode (this can be done in Chrome by simulating the forced-colors: active mode)

In the side editor:

  • load the site editor, make sure that the progress bar is always well visible
  • in the global styles, tweak the background color
  • reload the site editor: the background color of the loading screen should follow the background color chosen in the previous step. The progress bar should still have enough contrast

Screenshots or screencast

Storybook:

progress-bar-storybook.mp4

Site editor:

progress-bar-site-editor.mp4

@ciampo ciampo self-assigned this Oct 11, 2023
@ciampo ciampo added the [Package] Components /packages/components label Oct 11, 2023
@ciampo ciampo added the [Type] Enhancement A suggestion for improvement. label Oct 11, 2023
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Looks and works great 🚀

I only have a question regarding the fallback track/indicator colors.

Thanks for addressing it @ciampo!

/* Text color at 90% opacity */
background-color: color-mix(
in srgb,
var( --wp-components-color-foreground, ${ COLORS.gray[ 900 ] } ),
Copy link
Member

Choose a reason for hiding this comment

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

Do we mean to default both indicator and track color to the same shade of gray? I'd expect the track and indicator fallback colors to be different from each other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR is implementing this suggestion from Joen. I think that it makes sense to use the same shade of gray and play with opacity, because it ensures high contrast against the background.

Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Nice solution @ciampo! This is testing great for me, too, and seems to play nicely with lots of different light and dark combinations:

image image image

LGTM! ✨

@ciampo ciampo merged commit 05693fb into trunk Oct 12, 2023
50 checks passed
@ciampo ciampo deleted the feat/progress-bar-better-contrast branch October 12, 2023 08:01
@github-actions github-actions bot added this to the Gutenberg 16.9 milestone Oct 12, 2023
@andrewserong andrewserong added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 12, 2023
@andrewserong
Copy link
Contributor

andrewserong commented Oct 12, 2023

Just adding the backport to WP Beta/RC label since the issue in #54202 was flagged for 6.4, so it might be worth seeing if this fix can land for 6.4, too 🙂

Edit: I've also swapped out the Enhancement label for Bug as this PR resolves a visibility bug for the progress bar in 6.4. Feel free to change the labels around if folks would prefer!

@andrewserong andrewserong added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels Oct 12, 2023
@t-hamano
Copy link
Contributor

Sorry for being late in checking. Just wanted to report that the progress bar is now displayed very clearly even in Windows high contrast mode 🙌

a59fc11730438aa212dc31ad256653d6.mp4

@mtias
Copy link
Member

mtias commented Oct 13, 2023

Thanks for taking care of this one and improving it without losing the original spirit.

mikachan pushed a commit that referenced this pull request Oct 13, 2023
…und (#55285)

* Use text color at different opacities for track and indicator

* Add high contrast mode styles

* CHANGELOG
# Conflicts:
#	packages/components/CHANGELOG.md
@mikachan mikachan removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 13, 2023
@mikachan
Copy link
Member

I just cherry-picked this PR to the 6.4-rc1-2 branch to get it included in the next release: 13d5b22

SiobhyB pushed a commit that referenced this pull request Oct 16, 2023
* Add selector as id to layout style overrides. (#55291)

* Fix flickering when focusing on global style variations (#55267)

* ProgressBar: use text color to ensure enough contrast against background (#55285)

* Use text color at different opacities for track and indicator

* Add high contrast mode styles

* CHANGELOG
# Conflicts:
#	packages/components/CHANGELOG.md

* Remove empty attrs. (#54496)

* Remove empty attrs.

* Fix linter errors

---------

Co-authored-by: Sarah Norris <sarah@sekai.co.uk>

* Add IS_GUTENBERG_PLUGIN flag to LastRevision (#55253)

* useBlockPreview: Try outputting EditorStyles to ensure local style overrides are rendered (#55288)

* useBlockPreview: Try alternative fix for displaying local style overrides

* Avoid duplicate styles, fix rendering issues in Safari

* Add more explanatory comments

* Remove additional check for styles within the block preview, as it is not needed since EditorStyles handles its own style overrides retrieval

* Bug: PHP notice when an image with lightbox is deleted (#55370)

* Fix PHP notice when an image with lightbox is deleted

* Fix PHP notice when an image with lightbox is deleted

---------

Co-authored-by: tellthemachines <tellthemachines@users.noreply.github.com>
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
Co-authored-by: Kishan Jasani <kishanjasani007@yahoo.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
Status: Done 🎉
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants