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

Make accessible toolbar stable and deprecate old usage #23316

Merged
merged 28 commits into from
Aug 11, 2020

Conversation

diegohaz
Copy link
Member

@diegohaz diegohaz commented Jun 19, 2020

Closes #18619

Summary of changes in this PR:

  • Renamed __experimentalToolbarItem component to ToolbarItem.
  • Renamed Toolbar's __experimentalAccessibilityLabel prop to label.
  • Deprecated Toolbar when used without the label prop (will still fallback to ToolbarGroup for backward compatibility).
  • Wrote developer documentation for Toolbar, ToolbarGroup, ToolbarButton and ToolbarItem components.

How to test?

Make sure block toolbar and the top toolbar are working.

@diegohaz diegohaz added [Type] Enhancement A suggestion for improvement. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [a11y] Keyboard & Focus labels Jun 19, 2020
@diegohaz diegohaz self-assigned this Jun 19, 2020
@diegohaz diegohaz added the [Status] In Progress Tracking issues with work in progress label Jun 19, 2020
@diegohaz diegohaz mentioned this pull request Jun 19, 2020
7 tasks
@github-actions
Copy link

github-actions bot commented Jun 19, 2020

Size Change: +579 B (0%)

Total Size: 1.16 MB

Filename Size Change
build/block-editor/index.js 125 kB +222 B (0%)
build/block-editor/style-rtl.css 10.6 kB +34 B (0%)
build/block-editor/style.css 10.6 kB +36 B (0%)
build/block-library/index.js 132 kB -167 B (0%)
build/block-library/style-rtl.css 7.76 kB +1 B
build/block-library/style.css 7.77 kB +1 B
build/blocks/index.js 48.4 kB +98 B (0%)
build/components/index.js 200 kB +43 B (0%)
build/components/style-rtl.css 15.7 kB +2 B (0%)
build/components/style.css 15.7 kB +2 B (0%)
build/edit-post/index.js 304 kB +268 B (0%)
build/edit-post/style-rtl.css 5.62 kB +5 B (0%)
build/edit-post/style.css 5.61 kB +4 B (0%)
build/editor/index.js 45.3 kB +29 B (0%)
build/element/index.js 4.65 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.44 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.97 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-library/editor-rtl.css 7.59 kB 0 B
build/block-library/editor.css 7.59 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/compose/index.js 9.68 kB 0 B
build/core-data/index.js 11.8 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.45 kB 0 B
build/date/index.js 5.38 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.23 kB 0 B
build/edit-navigation/index.js 10.9 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-site/index.js 17 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.js 9.38 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.33 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.71 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@youknowriad youknowriad added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jul 2, 2020
@gziolo
Copy link
Member

gziolo commented Jul 2, 2020

Cross-linking my comment #23615 (comment) in the tracking issue for experimental APIs.

@ellatrix ellatrix mentioned this pull request Jul 3, 2020
12 tasks
@diegohaz
Copy link
Member Author

diegohaz commented Aug 6, 2020

This is ready for review! :)

@diegohaz diegohaz requested a review from gziolo August 6, 2020 21:47
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

This is looking great for me.

I'd like to ensure that I understand the block author story better here (as I think it's important to include in the dev note). How does a block author add an accessible toolbar/buttons to its BlockControls?

If I'm understanding properly, there's no assumptation that a parent Toolbar exists, so we have to add a Toolbar with its children, right? And if we want to add a single button, we just use Button directly, right?

@diegohaz
Copy link
Member Author

@youknowriad Not exactly. I updated the docs here so they're (hopefully) more clear regarding the usage of BlockControls.

Basically you have to use either ToolbarItem or ToolbarButton components in BlockControls. Optionally wrapping them with ToolbarGroup if you want them in a separate group in the block toolbar.

documentation screenshot

documentation screenshot

If you use Button or any other tabbable element that is not ToolbarItem or ToolbarButton inside BlockControls, you'll see this warning that will point you to the docs above:

deprecated( 'Using custom components as toolbar controls', {
alternative: 'ToolbarItem or ToolbarButton components',
link:
'https://developer.wordpress.org/block-editor/components/toolbar-button/#inside-blockcontrols',
} );

@diegohaz diegohaz merged commit af768dc into master Aug 11, 2020
@diegohaz diegohaz deleted the try/accessible-toolbar branch August 11, 2020 09:50
@github-actions github-actions bot added this to the Gutenberg 8.8 milestone Aug 11, 2020
@diegohaz diegohaz removed the [Status] In Progress Tracking issues with work in progress label Sep 7, 2020
@cguntur cguntur mentioned this pull request Oct 14, 2020
17 tasks
@tellthemachines tellthemachines removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Nov 18, 2020
@nerrad
Copy link
Contributor

nerrad commented Dec 7, 2020

@diegohaz this new API and deprecation wasn't added to the CHANGELOG.md for the @wordpress/components package. Will you please add details about this to the changelog in the appropriate version section it was released with? Thanks!

cc @gziolo for visibility.

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). [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Components: Improve Toolbar
5 participants