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

Add new category form improvements #2588

Merged
merged 7 commits into from Sep 1, 2017

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Aug 29, 2017

This PR tries to improve a bit the accessibility of the mini-form used to add new categories. Further improvements should be addressed in separate issues, see #2582 and #2581.

  • adds visible label elements to the input field and select
  • add IDs using withInstanceId
  • removes the input placeholder
  • removes the "+" plus sign from the toggle: we don't want screen readers announce "plus" and it's inconsistent with other button-links used in the sidebar, e.g. "set featured image"
  • fixes the focus styles
  • adds missing type="text" to the input field (this also fixes the focus style)
  • keeps the form open after a category has been added: otherwise, when adding multiple categories, users would be forced to re-open the form each time
  • adds aria-expanded to the toggle
  • tries to improve translations and labels

Fixes #2572

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Aug 29, 2017
@codecov
Copy link

codecov bot commented Aug 29, 2017

Codecov Report

Merging #2588 into master will increase coverage by 0.96%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2588      +/-   ##
==========================================
+ Coverage   31.68%   32.64%   +0.96%     
==========================================
  Files         175      177       +2     
  Lines        5312     5881     +569     
  Branches      915     1093     +178     
==========================================
+ Hits         1683     1920     +237     
- Misses       3081     3304     +223     
- Partials      548      657     +109
Impacted Files Coverage Δ
...ebar/post-taxonomies/hierarchical-term-selector.js 0% <0%> (ø) ⬆️
blocks/editable/index.js 9.85% <0%> (-1.17%) ⬇️
blocks/url-input/index.js 1.28% <0%> (-0.42%) ⬇️
blocks/api/parser.js 97.93% <0%> (-0.25%) ⬇️
editor/selectors.js 96.22% <0%> (-0.18%) ⬇️
editor/modes/visual-editor/block-list.js 0% <0%> (ø) ⬆️
editor/modes/visual-editor/block.js 0% <0%> (ø) ⬆️
editor/sidebar/header.js 0% <0%> (ø) ⬆️
editor/modes/visual-editor/index.js 0% <0%> (ø) ⬆️
editor/writing-flow/index.js 0% <0%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14574e7...f769c91. Read the comment docs.

@@ -114,7 +115,7 @@ class HierarchicalTermSelector extends Component {
adding: false,
formName: '',
formParent: '',
showForm: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we could drop it entirely here

@afercia afercia changed the title Update/add new category form improvements Add new category form improvements Aug 29, 2017
@afercia
Copy link
Contributor Author

afercia commented Aug 29, 2017

Visual comparison of the form, from left to right: before, after, after with focused elements:

screen shot 2017-08-29 at 15 29 46

Previously closed form after add, now leaves _as is_, so no need to explicitly assigned
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

I made suggested changes per https://github.com/WordPress/gutenberg/pull/2588/files#r135794709 in f769c91. Looks good 👍

@afercia
Copy link
Contributor Author

afercia commented Sep 1, 2017

Thanks! Will split a few pending improvements in a new issue.

@afercia afercia merged commit a2756aa into master Sep 1, 2017
@afercia afercia deleted the update/add-new-category-form-improvements branch September 1, 2017 07:12
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.

Add new category controls improvements
3 participants