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.

Sign up for GitHub

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

Jump to bottom

Tabs: keep full opacity of focus ring on disabled tabs #63754

Merged
merged 5 commits into from
Jul 23, 2024
Merged

Tabs: keep full opacity of focus ring on disabled tabs #63754

merged 5 commits into from
Jul 23, 2024

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Jul 19, 2024 •
edited
Loading

What?

Part of #52997

Inspired by #62480, this PR makes sure that the focus ring on disabled Tabs.Tab components is fully opaque, and that the text color of disabled tabs doesn't change on hover.

Why?

The focus ring on disabled tabs should have full contrast, and disabled tabs should not show "interactive" hints on hover since they're not interactive.

How?

  • By not setting a low opacity on disabled tabs, and instead changing the text color to achieve a similar result.
  • By enabling hover styles only for non-disabled tabs

Testing Instructions

  • Visit the "disabled" Tabs Storybook example
  • Using the arrow keys, move focus on the disabled tab
  • Make sure that the focus ring is fully opaqu

Screenshots or screencast

Before (trunk) After (trunk)
Screenshot 2024-07-19 at 17 04 05 Screenshot 2024-07-22 at 09 49 10

@ciampo ciampo requested a review from ajitbohra as a code owner July 19, 2024 15:06
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 19, 2024 •
edited
Loading

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: ciampo <mciampini@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

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

@ciampo ciampo self-assigned this Jul 19, 2024
@ciampo ciampo added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Jul 19, 2024
@ciampo ciampo requested a review from a team July 19, 2024 15:12
@ciampo ciampo mentioned this pull request Jul 19, 2024
Open
31 tasks
tyxla
tyxla reviewed Jul 19, 2024
View reviewed changes
packages/components/src/tabs/styles.ts Outdated

&[aria-disabled='true'] {
cursor: default;
opacity: 0.3;
color: ${ COLORS.theme.gray[ 700 ] };
Copy link
Member

Choose a reason for hiding this comment

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

Is it intentional that we're making the text so close to the non-disabled tab text color now? I'd likely prefer a lighter gray, but as you said, let's ask for some design feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup! Looking at #62480 (comment), there seems to be consensus on the lighter 600 shade. I will update accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, using a semantic variable that aliases to gray 600.

@tyxla tyxla requested a review from a team July 19, 2024 19:08
@tyxla tyxla added the Needs Design Feedback Needs general design feedback. label Jul 19, 2024
@ciampo ciampo force-pushed the fix/tabs-focus-ring-opacity branch from b84e20f to ce77856 Compare July 22, 2024 07:46
@ciampo ciampo requested a review from tyxla July 22, 2024 07:47
@ciampo ciampo removed the Needs Design Feedback Needs general design feedback. label Jul 22, 2024
@ciampo
Copy link
Contributor Author

ciampo commented Jul 22, 2024 •
edited
Loading

Removing the "Needs design feedback" label since the relevant feedback was already provided in #62480 (comment)

This PR is ready for a final review.

tyxla
tyxla approved these changes Jul 23, 2024
View reviewed changes
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.

LGTM 👍

@ciampo ciampo merged commit e9e27b9 into trunk Jul 23, 2024
65 checks passed
@ciampo ciampo deleted the fix/tabs-focus-ring-opacity branch July 23, 2024 10:21
@github-actions github-actions bot added this to the Gutenberg 18.9 milestone Jul 23, 2024
westonruter added a commit that referenced this pull request Jul 23, 2024
@westonruter
Merge branch 'trunk' into try/interactivity-lazy-hydration
63f7145 
* trunk: (2604 commits)
  Update "Versions in WordPress" page (#63869)
  SelectControl: Fix hover/focus color in wp-admin (#63855)
  Add margin-bottom lint rules for RangeControl (#63821)
  JSON Schema Docgen Rework (#63868)
  JSON Schema Reorganization and Fixes (#63591)
  DataForm: Add a simple story for the DataForm component (#63840)
  Quick Edit: Support bulk selection (#63841)
  Update dataviews docs (#63860)
  Bump the github-actions group across 1 directory with 4 updates (#63808)
  Add unit tests for the gutenberg_render_block_core_post_title() function.
  Make hover block outlines not present in Distraction Free (#63819)
  DataViews: Rename the header property of fields to label (#63843)
  Fix: Error while Calling edit-site getCurrentTemplateTemplateParts selector (#63818)
  Revert "Update HeightControl component to label inputs" (#63839)
  Zoom out: hide vertical toolbar when block is not full width (#63650)
  Latest comments: Add color block support (#63419)
  Core Data: Remove leftover 'todo' comment (#63842)
  Tabs: keep full opacity of focus ring on disabled tabs (#63754)
  Fix selected row styles in table layout (#63811)
  Align checkbox, radio, and toggle input design (#63490)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyxla tyxla tyxla approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

Assignees

@ciampo ciampo

Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
Gutenberg 18.9
Development

Successfully merging this pull request may close these issues.
2 participants
@ciampo @tyxla