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

Nav sidebar: Add active style to sidebar nav items and home button to ensure text is white while clicked #44418

Merged
merged 1 commit into from
Jul 27, 2020

Conversation

andrewserong
Copy link
Member

A tiny PR to ensure that the text in the buttons in the nav sidebar are white while clicked.

Changes proposed in this Pull Request

  • Add &:not( [aria-disabled='true'] ):active selector to the .wpcom-block-editor-nav-item and .wpcom-block-editor-nav-sidebar-nav-sidebar__home-button classes. The aria-disabled specificity is required so that we override the components-button:not([aria-disabled="true"]):active selector in Gutenberg.

Screenshot (before)

Screenshot while clicking an item in the nav sidebar list:

image

Screenshot (after)

Screenshot while clicking an item in the nav sidebar list:

image

Testing instructions

  • Checkout branch
  • cd apps/full-site-editing && yarn build && npx wp-env start
  • localhost:4013/wp-admin
  • Go to edit a post or a page
  • Click the W button to open the nav sidebar
  • Click and hold on any of the listed pages or the back button and the text should be white

@matticbot
Copy link
Contributor

@andrewserong andrewserong requested a review from p-jackson July 24, 2020 02:26
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 24, 2020
@andrewserong andrewserong requested a review from a team July 24, 2020 02:26
@andrewserong andrewserong self-assigned this Jul 24, 2020
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

Caution: This PR affects files in the FSE Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D46846-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing in the FSE Plugin for more info: PCYsg-ly5-p2

Copy link
Member

@p-jackson p-jackson left a comment

Choose a reason for hiding this comment

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

This is great thanks @andrewserong! Tested the "Add New Page" button too and it all LGTM.

@p-jackson p-jackson merged commit 4709956 into master Jul 27, 2020
@p-jackson p-jackson deleted the update/nav-sidebar-active-state branch July 27, 2020 02:54
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants