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

Fix casing for context menu entries on MacOS #26163

Closed
fallaciousreasoning opened this issue Oct 21, 2022 · 8 comments · Fixed by brave/brave-core#15588
Closed

Fix casing for context menu entries on MacOS #26163

fallaciousreasoning opened this issue Oct 21, 2022 · 8 comments · Fixed by brave/brave-core#15588

Comments

@fallaciousreasoning
Copy link

Description

In #25532 we had a discussion on the correct casing on macOS. The conclusion was that context menus should be Title Case.

We should update the
Hide Brave Rewards icon text to be Hide Brave Rewards Icon
Hide Brave Wallet icon text to be Hide Brave Wallet Icon

Anything else @stephendonner @sangwoo108 @aguscruiz

@stephendonner
Copy link

Anything else @stephendonner @sangwoo108 @aguscruiz

I believe that covers them; thanks, @fallaciousreasoning 👍

@stephendonner stephendonner added OS/macOS and removed OS/Android Fixes related to Android browser functionality labels Oct 21, 2022
@fallaciousreasoning
Copy link
Author

Also, @sangwoo108 made a good point - we could just show hide on these context menus. What do you think?

@stephendonner
Copy link

Also, @sangwoo108 made a good point - we could just show hide on these context menus. What do you think?

e.g. Hide for both Windows/Linux and macOS - that works, though I'm not familiar with any best practices re: labeling of UI elements for screenreaders.

@rebron
Copy link
Collaborator

rebron commented Oct 21, 2022

Wallet icon also needs updating.
Currently Hide Brave Wallet icon
To Hide Brave Wallet Icon

@rebron rebron added the priority/P4 Planned work. We expect to get to it "soon". label Oct 21, 2022
@sangwoo108
Copy link

that works, though I'm not familiar with any best practices

https://developer.apple.com/design/human-interface-guidelines/components/system-experiences/the-menu-bar/

Prefer short, one-word menu titles. Various factors

This line is what I referred to.


re: labeling of UI elements for screenreaders.

Wow, this is a very good point. I've played with Voice Over on Mac, and find that it could be confusing. Voice over lets me know that I'm in the menu, but I can't know for which this menu is. But in order to trigger a menu item for a button, the button should be focused and Voice Over let us know what button is focused now. So the flow is like

  • Focus button

image

  • Open menu

image

image

  • Navigate to the item

image


Maybe context menus for bookmarks could be an analog for this. They have random item title but doesn't show it on the context menu. I think it could be okay because they're literally "contextual" so users could be aware of what they're interacting with.


Then I noticed that could be why Chromium folks put an extension name on the top of extension menu while it's disabled. When using Voice Over and navigating items with keyboard, the name of an extension will be read out.

image

But this strategy could be too much for our purpose.

@fallaciousreasoning
Copy link
Author

@rebron what do you think? At the moment, I'm leaning towards just changing the casing, as I don't feel like I know enough about accessibility to make a call on Hide

@MadhaviSeelam
Copy link

Verification PASSED using

Brave | 1.47.126 Chromium: 108.0.5359.99 (Official Build) beta (64-bit)
-- | --
Revision | 410951fc34bb4b2cbf182231f9f779efaafaf682-refs/branch-heads/5359_71@{#9}
OS | Windows 11 Version 21H2 (Build 22000.1335)

Confirmed casing in the context menu on non-macOS (Windows) is as expected.

  • Hide Brave Rewards icon
  • Hide side panel icon
  • Hide Brave Wallet icon
Brave Rewards side panel Brave Wallet
image image image

@stephendonner
Copy link

Verified PASSED using

Brave 1.47.136 Chromium: 108.0.5359.128 (Official Build) beta (x86_64)
Revision 1cd27afdb8e5d057070c0961e04c490d2aca1aa0-refs/branch-heads/5359@{#1185}
OS macOS Version 11.7.2 (Build 20G1020)

Confirmed the strings are:

  • Hide Brave Rewards Icon
  • Hide Side Panel Icon
  • Hide Brave Wallet Icon
  • Hide Brave VPN icon
Brave Rewards side panel Brave Wallet Brave VPN
Screen Shot 2022-12-23 at 11 42 52 AM Screen Shot 2022-12-23 at 11 43 10 AM Screen Shot 2022-12-23 at 11 43 17 AM Screen Shot 2022-12-23 at 11 45 47 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants