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

Polish menus and focus styles #6812

Merged
merged 4 commits into from
May 23, 2018
Merged

Polish menus and focus styles #6812

merged 4 commits into from
May 23, 2018

Conversation

jasmussen
Copy link
Contributor

This fixes a regression in menu focus styles that was introduced as part of some focus fixes to the movers way back.

It also fixes #6734, as it changes how the shortcuts are spelled out.

screen shot 2018-05-18 at 09 37 06

screen shot 2018-05-18 at 09 37 12

@jasmussen jasmussen added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label May 18, 2018
@jasmussen jasmussen self-assigned this May 18, 2018
@jasmussen jasmussen requested review from noisysocks, ellatrix and a team May 18, 2018 07:41
@gziolo
Copy link
Member

gziolo commented May 18, 2018

Nice, More Menu is getting fixed. I'm not sure why those keyboard shortcuts were updated in the first place, so I will leave review to others :)

@jasmussen
Copy link
Contributor Author

I think I also fixed the Block More Menu btw.

Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

👍 on design review

@jasmussen
Copy link
Contributor Author

Rebased, and fixed an issue as a result of the buttons refactor. I think this is ready to go.

@jasmussen jasmussen requested a review from a team May 22, 2018 08:30
[ COMMAND ]: '⌘',
[ SHIFT ]: isMac ? '⇧shift' : 'Shift',
[ SHIFT ]: isMac ? 'Shift' : 'Shift',
Copy link
Member

Choose a reason for hiding this comment

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

This could be simplified to [ SHIFT ]: 'Shift',.

[ ALT ]: isMac ? '⌥option' : 'Alt',
[ CTRL ]: isMac ? '⌃control' : 'Ctrl',
[ ALT ]: isMac ? 'Option' : 'Alt',
[ CTRL ]: isMac ? 'Ctrl' : 'Ctrl',
Copy link
Member

Choose a reason for hiding this comment

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

This could be simplified to [ CTRL ]: 'Ctrl',.

Joen Asmussen and others added 3 commits May 23, 2018 08:48
This fixes a regression in menu focus styles that was introduced as part of some focus fixes to the movers way back.

It also fixes #6734, as it changes how the shortcuts are spelled out.
@jasmussen
Copy link
Contributor Author

Addressed feedback, rebased, and merging if the checks pass. Thanks!

@jasmussen jasmussen merged commit 5f3c7f8 into master May 23, 2018
@jasmussen jasmussen deleted the fix/menu-focus branch May 23, 2018 07:28
@mtias mtias added this to the 3.0 milestone Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More Menu shows overly verbose shortcut
5 participants