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

Sidebar: Add list view tab for Navigation block et al. #45483

Merged
merged 2 commits into from
Nov 18, 2022

Conversation

aaronrobertshaw
Copy link
Contributor

Related:

What?

Adds a new "List View" tab to the sidebar and if the block inspector tabs experiment is enabled, renders the Navigation block's menu controls to that tab.

Why?

How?

  • Adds new InspectorControls group slot for the new tab
  • Adds new List View tab
  • Updates the Navigation block to render its menu controls into the new slot/tab

Important Notes

  • As there are only a couple of blocks out of 80 that would utilize this tab. It doesn't make a lot of sense to display an empty tab for all the others to maintain strict consistency on displayed tabs.
  • Despite the less-than-ideal impact on a11y here due to the small occurrence of different tabs, I hope this approach might still be accepted. cc/ @alexstine in case you have any ideas other than rendering a useless tab for about 98% of blocks.
  • If the list view is only being displayed for the Navigation block and maybe one or two others, and it would contain important controls for those blocks, I've kept it as the first tab for those blocks as per the original design mockups.

Testing Instructions

  1. Ensure you do not have the Block Inspector Tabs experiment enabled
  2. Edit a post with a Nav block and select it
  3. The menu related controls should still display and function as normal
  4. Enable the Block Inspector Tabs experiment
  5. Switch back to the editor and re-select the Nav block
  6. Confirm the Nav block now renders the menu-related controls into the new List View tab
  7. Add some other blocks and ensure they do not display the List View tab
  8. Bonus points if you tweak an existing block to render some controls into the List View tab and confirm they don't not display due to the whitelist enforced for that tab.

Screenshots or screencast

Screenshot 2022-11-02 at 7 38 09 pm

@aaronrobertshaw aaronrobertshaw added [Type] Enhancement A suggestion for improvement. [Feature] Inspector Controls The interface showing block settings and the controls available for each block [Block] Navigation Affects the Navigation Block [Package] Block editor /packages/block-editor labels Nov 2, 2022
@aaronrobertshaw aaronrobertshaw self-assigned this Nov 2, 2022
@codesandbox
Copy link

codesandbox bot commented Nov 2, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

Comment on lines +35 to +36
? __( 'This block has no list options.' )
: __( 'The selected blocks have no list options.' ) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not real clear on which blocks will be rendering what controls to this tab so the wording here is pretty poor. Any suggestions?

@github-actions
Copy link

github-actions bot commented Nov 2, 2022

Size Change: +200 B (0%)

Total Size: 1.3 MB

Filename Size Change
build/block-editor/index.min.js 176 kB +178 B (0%)
build/block-library/index.min.js 194 kB +22 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 982 B
build/annotations/index.min.js 2.76 kB
build/api-fetch/index.min.js 2.26 kB
build/autop/index.min.js 2.14 kB
build/blob/index.min.js 475 B
build/block-directory/index.min.js 7.09 kB
build/block-directory/style-rtl.css 990 B
build/block-directory/style.css 991 B
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
build/block-editor/style-rtl.css 15.9 kB
build/block-editor/style.css 15.9 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 90 B
build/block-library/blocks/archives/style.css 90 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 122 B
build/block-library/blocks/audio/style.css 122 B
build/block-library/blocks/audio/theme-rtl.css 126 B
build/block-library/blocks/audio/theme.css 126 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 84 B
build/block-library/blocks/avatar/style.css 84 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 482 B
build/block-library/blocks/button/editor.css 482 B
build/block-library/blocks/button/style-rtl.css 532 B
build/block-library/blocks/button/style.css 532 B
build/block-library/blocks/buttons/editor-rtl.css 337 B
build/block-library/blocks/buttons/editor.css 337 B
build/block-library/blocks/buttons/style-rtl.css 332 B
build/block-library/blocks/buttons/style.css 332 B
build/block-library/blocks/calendar/style-rtl.css 239 B
build/block-library/blocks/calendar/style.css 239 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 100 B
build/block-library/blocks/categories/style.css 100 B
build/block-library/blocks/code/editor-rtl.css 53 B
build/block-library/blocks/code/editor.css 53 B
build/block-library/blocks/code/style-rtl.css 121 B
build/block-library/blocks/code/style.css 121 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comment-template/style-rtl.css 199 B
build/block-library/blocks/comment-template/style.css 198 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 840 B
build/block-library/blocks/comments/editor.css 839 B
build/block-library/blocks/comments/style-rtl.css 637 B
build/block-library/blocks/comments/style.css 636 B
build/block-library/blocks/cover/editor-rtl.css 612 B
build/block-library/blocks/cover/editor.css 613 B
build/block-library/blocks/cover/style-rtl.css 1.57 kB
build/block-library/blocks/cover/style.css 1.55 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 126 B
build/block-library/blocks/embed/theme.css 126 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 253 B
build/block-library/blocks/file/style.css 254 B
build/block-library/blocks/file/view.min.js 346 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 948 B
build/block-library/blocks/gallery/editor.css 950 B
build/block-library/blocks/gallery/style-rtl.css 1.53 kB
build/block-library/blocks/gallery/style.css 1.53 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 B
build/block-library/blocks/group/editor-rtl.css 654 B
build/block-library/blocks/group/editor.css 654 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 327 B
build/block-library/blocks/html/editor.css 329 B
build/block-library/blocks/image/editor-rtl.css 880 B
build/block-library/blocks/image/editor.css 880 B
build/block-library/blocks/image/style-rtl.css 627 B
build/block-library/blocks/image/style.css 630 B
build/block-library/blocks/image/theme-rtl.css 126 B
build/block-library/blocks/image/theme.css 126 B
build/block-library/blocks/latest-comments/style-rtl.css 298 B
build/block-library/blocks/latest-comments/style.css 298 B
build/block-library/blocks/latest-posts/editor-rtl.css 213 B
build/block-library/blocks/latest-posts/editor.css 212 B
build/block-library/blocks/latest-posts/style-rtl.css 478 B
build/block-library/blocks/latest-posts/style.css 478 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 507 B
build/block-library/blocks/media-text/style.css 505 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 712 B
build/block-library/blocks/navigation-link/editor.css 711 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 296 B
build/block-library/blocks/navigation-submenu/editor.css 295 B
build/block-library/blocks/navigation/editor-rtl.css 2.15 kB
build/block-library/blocks/navigation/editor.css 2.16 kB
build/block-library/blocks/navigation/style-rtl.css 2.21 kB
build/block-library/blocks/navigation/style.css 2.2 kB
build/block-library/blocks/navigation/view-modal.min.js 2.81 kB
build/block-library/blocks/navigation/view.min.js 443 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 363 B
build/block-library/blocks/page-list/editor.css 363 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 174 B
build/block-library/blocks/paragraph/editor.css 174 B
build/block-library/blocks/paragraph/style-rtl.css 279 B
build/block-library/blocks/paragraph/style.css 281 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 501 B
build/block-library/blocks/post-comments-form/style.css 501 B
build/block-library/blocks/post-date/style-rtl.css 61 B
build/block-library/blocks/post-date/style.css 61 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 586 B
build/block-library/blocks/post-featured-image/editor.css 584 B
build/block-library/blocks/post-featured-image/style-rtl.css 315 B
build/block-library/blocks/post-featured-image/style.css 315 B
build/block-library/blocks/post-navigation-link/style-rtl.css 153 B
build/block-library/blocks/post-navigation-link/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 282 B
build/block-library/blocks/post-template/style.css 282 B
build/block-library/blocks/post-terms/style-rtl.css 96 B
build/block-library/blocks/post-terms/style.css 96 B
build/block-library/blocks/post-title/style-rtl.css 100 B
build/block-library/blocks/post-title/style.css 100 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 135 B
build/block-library/blocks/pullquote/editor.css 135 B
build/block-library/blocks/pullquote/style-rtl.css 326 B
build/block-library/blocks/pullquote/style.css 325 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 288 B
build/block-library/blocks/query-pagination/style.css 284 B
build/block-library/blocks/query-title/style-rtl.css 63 B
build/block-library/blocks/query-title/style.css 63 B
build/block-library/blocks/query/editor-rtl.css 440 B
build/block-library/blocks/query/editor.css 440 B
build/block-library/blocks/quote/style-rtl.css 213 B
build/block-library/blocks/quote/style.css 213 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 409 B
build/block-library/blocks/search/style.css 406 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 234 B
build/block-library/blocks/separator/style.css 234 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 464 B
build/block-library/blocks/shortcode/editor.css 464 B
build/block-library/blocks/site-logo/editor-rtl.css 490 B
build/block-library/blocks/site-logo/editor.css 490 B
build/block-library/blocks/site-logo/style-rtl.css 203 B
build/block-library/blocks/site-logo/style.css 203 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 116 B
build/block-library/blocks/site-title/editor.css 116 B
build/block-library/blocks/site-title/style-rtl.css 57 B
build/block-library/blocks/site-title/style.css 57 B
build/block-library/blocks/social-link/editor-rtl.css 184 B
build/block-library/blocks/social-link/editor.css 184 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.4 kB
build/block-library/blocks/social-links/style.css 1.39 kB
build/block-library/blocks/spacer/editor-rtl.css 322 B
build/block-library/blocks/spacer/editor.css 322 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 505 B
build/block-library/blocks/table/editor.css 505 B
build/block-library/blocks/table/style-rtl.css 631 B
build/block-library/blocks/table/style.css 631 B
build/block-library/blocks/table/theme-rtl.css 172 B
build/block-library/blocks/table/theme.css 172 B
build/block-library/blocks/tag-cloud/style-rtl.css 251 B
build/block-library/blocks/tag-cloud/style.css 253 B
build/block-library/blocks/template-part/editor-rtl.css 235 B
build/block-library/blocks/template-part/editor.css 235 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 691 B
build/block-library/blocks/video/editor.css 694 B
build/block-library/blocks/video/style-rtl.css 174 B
build/block-library/blocks/video/style.css 174 B
build/block-library/blocks/video/theme-rtl.css 126 B
build/block-library/blocks/video/theme.css 126 B
build/block-library/classic-rtl.css 162 B
build/block-library/classic.css 162 B
build/block-library/common-rtl.css 1.02 kB
build/block-library/common.css 1.02 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 11.5 kB
build/block-library/editor.css 11.4 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/style-rtl.css 12.4 kB
build/block-library/style.css 12.4 kB
build/block-library/theme-rtl.css 704 B
build/block-library/theme.css 708 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.83 kB
build/blocks/index.min.js 49.9 kB
build/components/index.min.js 203 kB
build/components/style-rtl.css 11.5 kB
build/components/style.css 11.5 kB
build/compose/index.min.js 12.2 kB
build/core-data/index.min.js 15.5 kB
build/customize-widgets/index.min.js 11.3 kB
build/customize-widgets/style-rtl.css 1.38 kB
build/customize-widgets/style.css 1.38 kB
build/data-controls/index.min.js 653 B
build/data/index.min.js 8.08 kB
build/date/index.min.js 32.1 kB
build/deprecated/index.min.js 507 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.7 kB
build/edit-navigation/index.min.js 16.2 kB
build/edit-navigation/style-rtl.css 4.06 kB
build/edit-navigation/style.css 4.06 kB
build/edit-post/classic-rtl.css 546 B
build/edit-post/classic.css 547 B
build/edit-post/index.min.js 34.4 kB
build/edit-post/style-rtl.css 7.39 kB
build/edit-post/style.css 7.38 kB
build/edit-site/index.min.js 61 kB
build/edit-site/style-rtl.css 8.4 kB
build/edit-site/style.css 8.37 kB
build/edit-widgets/index.min.js 16.7 kB
build/edit-widgets/style-rtl.css 4.41 kB
build/edit-widgets/style.css 4.41 kB
build/editor/index.min.js 43.7 kB
build/editor/style-rtl.css 3.6 kB
build/editor/style.css 3.59 kB
build/element/index.min.js 4.68 kB
build/escape-html/index.min.js 537 B
build/experiments/index.min.js 868 B
build/format-library/index.min.js 6.95 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.64 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.77 kB
build/is-shallow-equal/index.min.js 527 B
build/keyboard-shortcuts/index.min.js 1.78 kB
build/keycodes/index.min.js 1.83 kB
build/list-reusable-blocks/index.min.js 2.13 kB
build/list-reusable-blocks/style-rtl.css 835 B
build/list-reusable-blocks/style.css 835 B
build/media-utils/index.min.js 2.93 kB
build/notices/index.min.js 963 B
build/nux/index.min.js 2.06 kB
build/nux/style-rtl.css 732 B
build/nux/style.css 728 B
build/plugins/index.min.js 1.94 kB
build/preferences-persistence/index.min.js 2.22 kB
build/preferences/index.min.js 1.33 kB
build/primitives/index.min.js 944 B
build/priority-queue/index.min.js 1.58 kB
build/react-i18n/index.min.js 696 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.74 kB
build/reusable-blocks/index.min.js 2.21 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 10.7 kB
build/server-side-render/index.min.js 1.77 kB
build/shortcode/index.min.js 1.53 kB
build/style-engine/index.min.js 1.48 kB
build/token-list/index.min.js 644 B
build/url/index.min.js 3.61 kB
build/vendors/inert-polyfill.min.js 2.48 kB
build/vendors/react-dom.min.js 38.5 kB
build/vendors/react.min.js 4.34 kB
build/viewport/index.min.js 1.08 kB
build/warning/index.min.js 268 B
build/widgets/index.min.js 7.21 kB
build/widgets/style-rtl.css 1.18 kB
build/widgets/style.css 1.19 kB
build/wordcount/index.min.js 1.06 kB

compressed-size-action

@alexstine
Copy link
Contributor

@aaronrobertshaw If this is meant to be block specific, why can we just not add a button in the block formatting toolbar or even inside the block placeholder as the image block handles this? I am just not understanding how to communicate to screen reader users that some blocks you edit in the block content and some blocks you edit from the sidebar. I cannot over stress how important consistency is. It is not A11Y compliant to have different patterns for different blocks. I am honestly kind of shocked this hasn't received more visual feedback as well considering it would be a confusing experience for everyone to have different blocks working differently. My point. The community needs to come together and figure out what the sidebar is going to be used for and then leave it well alone.

Thanks.

@aaronrobertshaw aaronrobertshaw force-pushed the try/appearance-settings-tabs-in-sidebar branch from 60863b2 to 8530a42 Compare November 9, 2022 06:34
@scruffian
Copy link
Contributor

Hi @alexstine

Thanks for your reply. I have some follow up questions:

  1. "It is not A11Y compliant to have different patterns for different blocks". Is your concern that different blocks have different tabs in the inspector, or is it that some block content is editable in the canvas and some block content is editable in the sidebar?

  2. "I am just not understanding how to communicate to screen reader users that some blocks you edit in the block content and some blocks you edit from the sidebar". Is your concern that we should not be allowing users to edit content in the sidebar at all, or that we need to communicate to screen readers that they have that option?

  3. "it would be a confusing experience for everyone to have different blocks working differently.". The proposal here is to allow users to edit the content of the navigation using the sidebar control in addition to the on-canvas controls. This would be an additional affordance, not a replacement for on-canvas editing. In your opinion, is this a problem?

@alexstine
Copy link
Contributor

@scruffian Sure, happy to go a little further. I know comments are not always as descriptive as necessary.

  1. Some content is editable in the canvas and some in the sidebar. It is reasonable to expect that switching blocks you would be presented with different content to edit. It is not reasonable to assume a sidebar is dynamic and could render different tabs based on the block. It only seems reasonable visually because you can see the sidebar change in real time. If you had to explore the content of every block in the canvas plus check the sidebar for every block, I would call this undesirable UX for screen reader users.
  2. I think the sidebar should not change based on the block, at least not to the extent of having different tabs per block. Consistency is everything.
  3. Why does the functionality need to be duplicated? This not only increases code overhead but introduces a new area to watch out for regressions in. The way I understood the editor was editing is supposed to take place in the canvas, the less used options are in the sidebar. Also, keep in mind, from a DOM and screen reader perspective, the sidebar is after the block content. Not lined up nicely. I think what gets people in trouble is there is always a focus on how things should function visually and that simply is not practical for screen readers as there is no sighted component.

The other thing to think about is communicating things to users. Gutenberg is already very verbose. This means that users are required to listen to instructions everywhere to understand how the editor works. I am in favor of this only if it is necessary and I think introducing these changes for the sidebar will require us to communicate more info for users who can't see and it is already information overload.

Imagine as a sighted user if you had to read tool tips all over the editor because the icons for the action you were trying to perform just did not make sense. The whole editor is still very much like this for screen reader users. Some parts are slowly getting better but other parts still suffer from enormous amounts of verbosity. Some of it is unavoidable such as communicating special shortcut key functionality or communicating the positioning of a block. All this information is stuff that the sighted take for granted because it is right there visually. All this stuff has to be consumed in different mediums for the non-sighted.

Thanks.

@scruffian
Copy link
Contributor

@alexstine, thanks for taking the time to reply. I really appreciate the detail in your response.

I agree that ideally the mechanism for editing block content should be the same for every block. In the case of the navigation block, the nature of the visual display of the block makes it very different to select and manipulate the items using a mouse. When navigation menus contain submenus, these items are visually hidden which makes it harder to edit for users who are relying on the visual display and a mouse. The idea behind adding the list view to the sidebar is to display all the items, even those that are visually hidden (like submenus), so that visual editing is easier.

From what I understand (and please correct me if I am wrong) it seems like you're saying adding additional controls to edit the block content in the sidebar doesn't add value for those using screen readers. This makes sense to me - if you're using a screen reader then visually hidden items will still be accessible, so the same concerns don't apply. What do you think of the idea of hiding the sidebar controls for screen readers? Since they already have a way to edit the content that works, maybe it's not necessary to give them another? I expect this is a bad idea, in which case I am very interested to understand more. Thanks for taking the time to reply, your help in this area is extremely useful to us.

@alexstine
Copy link
Contributor

@scruffian It will not be possible to hide this content to screen readers without making it inaccessible to the keyboard at the same time. You need 2 things to hide major sections like this.

  1. aria-hidden="true"
  2. tabindex="-1"

You cannot get by with one or the other because the first hides to screen readers but the second removes from the tabbable elements. If you only applied 1, screen readers would not understand why they keep hitting empty/hidden tab stops in the sidebar. If you applied both, sighted keyboard users would not be able to use it.

Is the problem that the block itself is too small of a canvas area? Ever thought about adding an expand content button or edit block in new window type functionality? That way if a user is working with a really complex block, they could click a button and have the content render in a dialog or something removing all the other distractions.

Thoughts?

I actually attempted something like this in a PR long ago but just could not work out the logic for how the sidebar would fit in to the flow of editing. Being totally honest, the sidebar for blocks seems kind of wrong to start but I'll leave that up for another discussion later down the road. I doubt anyone will be jumping to get rid of it.

Thanks.

@aaronrobertshaw
Copy link
Contributor Author

Great discussion @alexstine and @scruffian; I appreciate you both helping refine our approach here. 👍

I had a chat with @priethor today, where I proposed exploring another potential option that I'd like to get your thoughts on. It might not be perfect either, although I'm hoping we can get closer to a compromise that would allow this experiment to move forward.

This new approach would include the following:

  • All tabs (list view, settings, and appearance) being rendered in the DOM for all blocks
  • We hide any tabs that are empty only visually.
  • If all but one of the tabs are empty, we'd visually hide the entire tab list
  • Empty tabs would still contain text advising they contain no controls for the given block type

Some immediate downsides I can see with this approach are:

  • Setting which tab has the initial focus or is activated would be problematic if visually hiding tabs
  • There's still more information, adding to the overload Alex raised

I'm sure there'll be others as well.

Some upsides might be:

  • We will keep which tabs are in the DOM consistent across blocks
  • We can reintroduce from earlier iterations some of the originally requested functionality
  • At least for some users, we can keep the sidebar cleaner

I'd welcome any better solutions, or ideas on how we could address the downsides above.

@alexstine
Copy link
Contributor

@aaronrobertshaw

We hide any tabs that are empty only visually.

Yeah, I can see this being a big problem for sighted keyboard users. How do we hide something visually and keep it accessible with the keyboard for non-screen reader users?

I just do not think I will ever be sold on using the sidebar for this. Too much moving around and editing in different places.

@joedolson Can you add some feedback on this? Maybe there is something I am overlooking?

@alexstine alexstine added the Needs Accessibility Feedback Need input from accessibility label Nov 14, 2022
@mtias
Copy link
Member

mtias commented Nov 14, 2022

We will keep which tabs are in the DOM consistent across blocks

Why would this be an upside? The list view in the inspector is only relevant for blocks that manage their children, which is a small subset of blocks and optional for certain patterns. This notion has already been introduced for contentOnly locking, we should consider this an iteration on that.

Also this affordance should not take away from being able to interact in the canvas. And a direct toolbar link to open this view should also not be discarded — it's what the "list view" in the navigation block toolbar should likely do, given before it opened a modal, now it opens a panel in the inspector.

@aaronrobertshaw
Copy link
Contributor Author

Why would this be an upside?

My understanding of the a11y concerns raised to date was that consistency trumped most things when it came to compromises. I thought then that keeping the tabs consistent in the DOM for screen readers was desirable. I've gone around in circles on this too many times and disregarded the impact visually hiding tabs would have on sighted keyboard users.

The list view in the inspector is only relevant for blocks that manage their children, which is a small subset of blocks and optional for certain patterns. This notion has already been introduced for contentOnly locking, we should consider this an iteration on that.

This aligns with my original intent for this PR, although I have been attempting to find a compromise that would also address the a11y issues raised.

As this is behind a feature flag and is consistent with the existing contentOnly locking. I'd very much like to move ahead with the PR as is, i.e. only rendering the list view tab for that small subset of blocks. We can open it up then for wider testing, feedback, and ideas.

I just do not think I will ever be sold on using the sidebar for this. Too much moving around and editing in different places.

@alexstine, it appears we are back to square one here. There are a lot of benefits to being able to have this tab for the small subset of blocks that need it. I know, and appreciate, this isn't something you agree with, but I'd like to thank you for your continued input here 🙇

@mtias
Copy link
Member

mtias commented Nov 15, 2022

Consistency can be good or bad, depending on what is being made consistent. For example, it wouldn't make sense to show the Cover block focus point tool on a Paragraph because they are not contextually relevant. The proposed Settings and Styles split is something that should be consistent because it's relevant for almost every block and helps structure all the tools. The List View, however, is only relevant for a few blocks or a few situations (like locked patterns). Forcing that to be consistent will degrade the experience for the majority of the blocks that don't need it or don't have child blocks.

The point about the sidebar being disconnected from the canvas in terms of being the main interface to edit is definitely a good concern. I think the issue is how this is being presented that is leading to confusions on the goals, though. It's important to have a more data view for editing menus and navigating sections of child blocks, but it should not replace in canvas editing or traversing. Also, it should be easy to go from the canvas / toolbar to the list view editing for those that need that input mode. A while back we used to have a list view button in the toolbar of the navigation block that would open a modal to edit — the design intention is to render this element in a dedicated sidebar panel instead of the modal, but it follows the same idea.

ContentOnly patterns can also be edited in canvas, even if the sidebar provides an overview of the structure. The two things should be complementary.

@talldan talldan force-pushed the try/appearance-settings-tabs-in-sidebar branch from 8530a42 to 2990b61 Compare November 17, 2022 05:09
Base automatically changed from try/appearance-settings-tabs-in-sidebar to trunk November 17, 2022 07:51
@scruffian
Copy link
Contributor

I rebased this now the parent branch is merged.

Copy link
Contributor

@glendaviesnz glendaviesnz left a comment

Choose a reason for hiding this comment

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

Worked as advertised for me, only for Nav block, and only with experiment enabled.

@talldan
Copy link
Contributor

talldan commented Nov 18, 2022

The other idea I've had to around this issue is that the tabs become an optional preference. It would default to on, but turn it off and the inspector works the way it is in trunk.

It would essentially be taking the experiment feature toggle that's currently implemented and converting it into a user preference.

The negative aspects are that it would be a lot to maintain, and might hinder future iteration if at some point some tab specific UI is desired. 🤔

Switch editor sidebar icons to new drawer icons

Icon should reflect RTL as well.

Update TabPanel to allow icons on TabButtons

Add menu group to InspectorControl slot fills

Move nav menu controls into InspectorControls menu group

Introduce menu, settings & appearance tabs to sidebar

Refactor InspectorControlTabs

Re-orders the appearance and settings tabs. Also now omits the TabPanel altogether if only a single tab has contents.

Make block inspector tabs a Gutenberg experiment

A little tidy up

Clean up conditional tabs display

Remove nav specific menu tab for now

Remove Menu inspector controls group

Refactor inspector controls tabs to components

Remove conditional display of tabs

Render no settings or tools messages when no fills

Reduce new styles for equal width tabs

Add general slot for items applying to block as a whole

Move query block new post link to new slot

Revert "Move query block new post link to new slot"

This reverts commit 1279985.

Revert "Add general slot for items applying to block as a whole"

This reverts commit 186276f.

Tweak no style options message

Add changelog for TabPanel updates

Remove empty readme until experiment settles

Fix copy and paste error

Provide list view tab and slot for nav block
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Feature] Inspector Controls The interface showing block settings and the controls available for each block Needs Accessibility Feedback Need input from accessibility Needs User Documentation Needs new user documentation [Package] Block editor /packages/block-editor [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants