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

Edit Post: Update advanced settings phrases #6162

Merged
merged 3 commits into from Apr 16, 2018

Conversation

Projects
None yet
2 participants
@gziolo
Member

gziolo commented Apr 13, 2018

Description

Fixes #4239 and #6153.

I noticed that in #6153 there should be one instance of document settings. This PR fixes it, but also updates a few occurrences in the docs.

@afercia - is that all? Did I miss anything?

@gziolo gziolo self-assigned this Apr 13, 2018

@gziolo gziolo requested a review from afercia Apr 13, 2018

@gziolo gziolo added this to the 2.7 milestone Apr 13, 2018

@afercia

This comment has been minimized.

Contributor

afercia commented Apr 13, 2018

@gziolo thanks, seems to me all hte occurrences are covered
but...

It's important for the sidebar region to always have the same name. Elements with a role="region" and an aria-label attributes work as "landmark regions", that is: screen readers offer shortcuts to quickly jump through them.

These regions have been implemented to split the whole editor in 3 areas, so users can quickly move to the top toolbar, the content area, the sidebar. For this reason, the sidebar should always have the same name. Changing the label depending on which content is displayed in the sidebar would be confusing. For example, see how VoiceOver exposes landmark regions:

document settings
block settings

This is the main reason why previously everything, including the button in the menu, was named "advanced settings". That way, users were able to look for an "advanced settings" landmark at any time, without too much overhead.

Now that the menu item refers to "block", things are a bit different. Maybe the simpler option is to use just "Editor settings" for both the document and block sidebar.

@gziolo

This comment has been minimized.

Member

gziolo commented Apr 13, 2018

Now that the menu item refers to "block", things are a bit different. Maybe the simpler option is to use just "Editor settings" for both the document and block sidebar.

I can do that, sounds good. What about plugins sidebar? They use the same region but have a different name. Any thoughts?

@afercia

This comment has been minimized.

Contributor

afercia commented Apr 13, 2018

What about plugins sidebar? They use the same region but have a different name. Any thoughts?

Yeah we'll have to think about that, no great ides so far. Possible options:

  • a very generic name for the 3rd region
  • a 4th region
@gziolo

This comment has been minimized.

Member

gziolo commented Apr 14, 2018

Should we merge it as it and tackle plugins sidebar separately?

@afercia

This comment has been minimized.

Contributor

afercia commented Apr 16, 2018

Yep I'd say so.

@gziolo gziolo merged commit 0d2cce9 into master Apr 16, 2018

2 checks passed

codecov/project 44.37% (-0.08%) compared to b29b92d
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gziolo gziolo deleted the update/advanced-settings branch Apr 16, 2018

@gziolo

This comment has been minimized.

Member

gziolo commented Apr 16, 2018

Yeah we'll have to think about that, no great ides so far. Possible options:

Let's open another PR when we decide on the approach 🙇

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