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

Make the block settings menu item dynamic #3982

Merged
merged 2 commits into from Dec 18, 2017

Conversation

Projects
None yet
3 participants
@afercia
Contributor

afercia commented Dec 13, 2017

This PR tries to improve the interaction with the "Settings" menu item in the blocks "ellipsis" menu as detailed in #3340 (comment)

TODO: add a speak message when the sidebar block tab opens.

It's a first try to test if the new interaction makes sense, can be improved later.

Basically the item label and action are "dynamic" meaning they change based on the sidebar tab state:

screen shot 2017-12-13 at 19 57 43

screen shot 2017-12-13 at 19 57 54

screen shot 2017-12-13 at 19 57 59

Also, renames the label (and tooltip) for the block ellipsis menu: I've opted for "More options", inspired by a similar control in Gmail > compose:

more options

Note: when a control uses aria-expanded, it's better to not change its label as, for example:
Open Settings Menu - collapsed
Close Settings Menu - expanded

would sound a bit weird. Note that the similar control in Gmail uses aria-expanded and doesn't change the label.

Also renames the sidebar ARIA region to Editor advanced settings.

Fixes #3340

@afercia afercia requested a review from jasmussen Dec 13, 2017

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Dec 14, 2017

Contributor

I really reallly dig this 🏅

👍 👍 from me for all of this.

Code looks good to me too, though it couldn't hurt to get a quick 👍 from @youknowriad.

Contributor

jasmussen commented Dec 14, 2017

I really reallly dig this 🏅

👍 👍 from me for all of this.

Code looks good to me too, though it couldn't hurt to get a quick 👍 from @youknowriad.

@afercia afercia requested a review from youknowriad Dec 14, 2017

@aduth

aduth approved these changes Dec 14, 2017

@afercia afercia merged commit 3fc4d58 into master Dec 18, 2017

2 of 3 checks passed

codecov/project 37.85% (-0.54%) compared to ee99ade
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@afercia afercia deleted the try/block-settings-menu-item-dynamic branch Dec 18, 2017

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