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

change: [M3-8468] - Improve action menu styling #10964

Conversation

harsh-akamai
Copy link
Contributor

@harsh-akamai harsh-akamai commented Sep 18, 2024

Description 📝

When a user opens the LinodeActionMenu by using their mouse, the first option remain highlighted, even after the user is hovering over a separate option. This can be confusing for our users and negatively impacts accessibility.

It has also been reported that the LinodeActionMenu could use additional visual enhancements during the active state. In particular, when visiting the Linodes landing page, the top of the menu can be difficult to identify for some users because it's aligned with the "Last Backup" column of the Linodes table. This is particularly more evident when a user is using dark mode settings.

Changes 🔄

  • Added an eventHandler to change the <MenuItem/> state to focus status onMouseEnter
  • Changed the active state color for <MenuItem while in dark mode

Target release date 🗓️

N/A

Preview 📷

  • On hovering over menu items
Before After
image image

|

  • Visual enhacement
Before highlight color change After highlight color change
image image

How to test 🧪

Prerequisites

  • Open the website in dark theme

Verification steps

  • Navigate to localhost:3000/linodes and click on the <KebabIcon/> for any one of the Linodes
  • Ensure that the first item('Power Off` in this case) doesn't remain highlighted when you are hovering over other options
  • Verify that the Last Backup Column of the Linodes table is able to be differentiated from the top of the option menu

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@harsh-akamai harsh-akamai self-assigned this Sep 18, 2024
@harsh-akamai harsh-akamai requested a review from a team as a code owner September 18, 2024 15:36
@harsh-akamai harsh-akamai requested review from dwiley-akamai and cpathipa and removed request for a team September 18, 2024 15:36
@harsh-akamai harsh-akamai added the Accessibility Contains accessibility improvements or changes label Sep 18, 2024
Copy link

github-actions bot commented Sep 18, 2024

Coverage Report:
Base Coverage: 86.98%
Current Coverage: 86.98%

Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

Verified focus/hover state and color differentiation improvements ✅

"@linode/manager": Changed
---

Improve ActionMenu styling ([#10964](https://github.com/linode/manager/pull/10964))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Improve ActionMenu styling ([#10964](https://github.com/linode/manager/pull/10964))
Improve ActionMenu styling and accessibility ([#10964](https://github.com/linode/manager/pull/10964))

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

I'm wondering if this change could be made in the theme rather than in the component itself.

@harsh-akamai
Copy link
Contributor Author

harsh-akamai commented Sep 19, 2024

@bnussman-akamai wouldn't it be better to make the change in design-language-system repo since the value of Dropdown.Background.Hover isn't used anywhere else?

https://github.com/linode/design-language-system/blob/main/dist/themes/dark/index.d.ts#L4312

Copy link
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

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

Confirming the action menu styles. No regressions were found in dark or light mode.

@bnussman-akamai
Copy link
Member

I'm not sure @harsh-akamai

I'm mostly just concerned with one-off styling of the MenuItem. I think in an ideal world, this issue would be fixed in the theme or design-language-system rather than by a stray sx.

If the design-language-system color doesn't make sense and needs updating, we can make the change there!

@coliu-akamai coliu-akamai added the Approved Multiple approvals and ready to merge! label Sep 24, 2024
@harsh-akamai
Copy link
Contributor Author

Changed the color for active state as the one I chose in the initial commit coincided with the disabled state color.

Initial change New change
image image

@harsh-akamai harsh-akamai added the Add'tl Approval Needed Waiting on another approval! label Sep 27, 2024
@harsh-akamai harsh-akamai removed the Approved Multiple approvals and ready to merge! label Sep 30, 2024
@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Sep 30, 2024
@harsh-akamai harsh-akamai merged commit d664f54 into linode:develop Oct 1, 2024
20 checks passed
@harsh-akamai harsh-akamai deleted the feature/M3-8468-imporve-action-menu-styling branch October 1, 2024 05:55
Copy link

cypress bot commented Oct 1, 2024

Cloud Manager E2E    Run #6598

Run Properties:  status check passed Passed #6598  •  git commit d664f543aa: change: [M3-8468] - Improve action menu styling (#10964)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6598
Run duration 25m 53s
Commit git commit d664f543aa: change: [M3-8468] - Improve action menu styling (#10964)
Committer Harsh Shankar Rao
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 409
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Contains accessibility improvements or changes Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants