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

Heading level dropdown: remove obtrusive tooltips in favor of visible text #56035

Merged
merged 3 commits into from
Nov 16, 2023

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Nov 10, 2023

Fixes #50402
See #55927
See #46003

What?

Removes the dropdown menu heading level tooltips in favor of visible text.

Why?

Since the dropdown menu has now a vertical orientation, the tooltips of the menu buttons hide the other buttons on hover and focus. This is a functional and usability / accessibility problem that needs to be fixed.

I'd also like to remind that icon-only buttons have inherent usability and accessibility problems. Their usage should be limited to only the cases where there isn't enough space to use visible text. This is not the case, as there's a lot of space to show visible text.

How?

Makes the dropdown option buttons use both an icon and visible text.
Having both the icon and visible text seems to be the most appropriate solution here. The icon represents a visual hint that may help some users, while the visible text is universally accessible to everyone.

Other examples of dropdown menus that use both an icon and visible text are already present in the block toolbar, for example the block switcher menu and the More options menu.

This PR also adds a stylesheet to the component, which is only used to increase a little the horizontal gap between the heading icons and the visible text. This is not uncommon, as there are already components that override the button component default gap value, for example the Link control and the block switcher.

Testing Instructions

  • Edit a post and add a heading block.
  • Click the 'Change level' button in the toolbar.
  • Observe the dropdown menu shows both an icon and visible text.
  • Observe there is no tooltip on hover and focus.
  • Go to the Site editor.
  • Select the Site title and click the 'Change level' button in the toolbar.
  • Observe the dropdown menu shows both an icon and visible text.
  • Observe the dropdown includes also the Paragraph menu item.
  • Observe there is no tooltip on hover and focus.

Testing Instructions for Keyboard

Screenshots or screencast

Before:

before

After:

after

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Block] Heading Affects the Headings Block [Package] Block editor /packages/block-editor [Block] Site Title Affects the Site Title Block labels Nov 10, 2023
Copy link

github-actions bot commented Nov 10, 2023

Size Change: -1.58 kB (0%)

Total Size: 1.7 MB

Filename Size Change
build/block-editor/index.min.js 247 kB -1.03 kB (0%)
build/block-editor/style-rtl.css 15.7 kB -38 B (0%)
build/block-editor/style.css 15.7 kB -36 B (0%)
build/block-library/blocks/quote/style-rtl.css 237 B +15 B (+7%) 🔍
build/block-library/blocks/quote/style.css 237 B +15 B (+7%) 🔍
build/block-library/blocks/read-more/style-rtl.css 140 B +8 B (+6%) 🔍
build/block-library/blocks/read-more/style.css 140 B +8 B (+6%) 🔍
build/block-library/index.min.js 212 kB +7 B (0%)
build/block-library/style-rtl.css 14.5 kB +15 B (0%)
build/block-library/style.css 14.5 kB +15 B (0%)
build/blocks/index.min.js 51.5 kB -23 B (0%)
build/customize-widgets/index.min.js 12.1 kB +80 B (+1%)
build/customize-widgets/style-rtl.css 1.43 kB -74 B (-5%)
build/customize-widgets/style.css 1.43 kB -73 B (-5%)
build/edit-post/index.min.js 35.9 kB +269 B (+1%)
build/edit-post/style-rtl.css 7.75 kB -138 B (-2%)
build/edit-post/style.css 7.74 kB -137 B (-2%)
build/edit-site/index.min.js 209 kB +284 B (0%)
build/edit-site/style-rtl.css 14.2 kB -153 B (-1%)
build/edit-site/style.css 14.2 kB -159 B (-1%)
build/edit-widgets/index.min.js 17.2 kB +79 B (0%)
build/edit-widgets/style-rtl.css 4.65 kB -202 B (-4%)
build/edit-widgets/style.css 4.65 kB -195 B (-4%)
build/editor/index.min.js 46 kB +18 B (0%)
build/patterns/index.min.js 4.81 kB +38 B (+1%)
build/reusable-blocks/index.min.js 2.76 kB +35 B (+1%)
build/rich-text/index.min.js 10 kB -208 B (-2%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 964 B
build/annotations/index.min.js 2.71 kB
build/api-fetch/index.min.js 2.29 kB
build/autop/index.min.js 2.11 kB
build/blob/index.min.js 461 B
build/block-directory/index.min.js 7.25 kB
build/block-directory/style-rtl.css 1.04 kB
build/block-directory/style.css 1.04 kB
build/block-editor/content-rtl.css 4.28 kB
build/block-editor/content.css 4.27 kB
build/block-editor/default-editor-styles-rtl.css 403 B
build/block-editor/default-editor-styles.css 403 B
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 138 B
build/block-library/blocks/audio/theme.css 138 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 104 B
build/block-library/blocks/avatar/style.css 104 B
build/block-library/blocks/block/editor-rtl.css 305 B
build/block-library/blocks/block/editor.css 305 B
build/block-library/blocks/button/editor-rtl.css 587 B
build/block-library/blocks/button/editor.css 587 B
build/block-library/blocks/button/style-rtl.css 633 B
build/block-library/blocks/button/style.css 632 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 113 B
build/block-library/blocks/categories/editor.css 112 B
build/block-library/blocks/categories/style-rtl.css 124 B
build/block-library/blocks/categories/style.css 124 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 421 B
build/block-library/blocks/columns/style.css 421 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 647 B
build/block-library/blocks/cover/editor.css 650 B
build/block-library/blocks/cover/style-rtl.css 1.7 kB
build/block-library/blocks/cover/style.css 1.69 kB
build/block-library/blocks/details/editor-rtl.css 65 B
build/block-library/blocks/details/editor.css 65 B
build/block-library/blocks/details/style-rtl.css 98 B
build/block-library/blocks/details/style.css 98 B
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 138 B
build/block-library/blocks/embed/theme.css 138 B
build/block-library/blocks/file/editor-rtl.css 316 B
build/block-library/blocks/file/editor.css 316 B
build/block-library/blocks/file/style-rtl.css 280 B
build/block-library/blocks/file/style.css 281 B
build/block-library/blocks/file/view.min.js 320 B
build/block-library/blocks/footnotes/style-rtl.css 201 B
build/block-library/blocks/footnotes/style.css 199 B
build/block-library/blocks/form-input/editor-rtl.css 229 B
build/block-library/blocks/form-input/editor.css 228 B
build/block-library/blocks/form-input/style-rtl.css 343 B
build/block-library/blocks/form-input/style.css 343 B
build/block-library/blocks/form-submission-notification/editor-rtl.css 343 B
build/block-library/blocks/form-submission-notification/editor.css 342 B
build/block-library/blocks/form-submit-button/style-rtl.css 69 B
build/block-library/blocks/form-submit-button/style.css 69 B
build/block-library/blocks/form/view.min.js 452 B
build/block-library/blocks/freeform/editor-rtl.css 2.61 kB
build/block-library/blocks/freeform/editor.css 2.61 kB
build/block-library/blocks/gallery/editor-rtl.css 957 B
build/block-library/blocks/gallery/editor.css 962 B
build/block-library/blocks/gallery/style-rtl.css 1.55 kB
build/block-library/blocks/gallery/style.css 1.55 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 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 189 B
build/block-library/blocks/heading/style.css 189 B
build/block-library/blocks/html/editor-rtl.css 340 B
build/block-library/blocks/html/editor.css 341 B
build/block-library/blocks/image/editor-rtl.css 849 B
build/block-library/blocks/image/editor.css 847 B
build/block-library/blocks/image/style-rtl.css 1.6 kB
build/block-library/blocks/image/style.css 1.59 kB
build/block-library/blocks/image/theme-rtl.css 137 B
build/block-library/blocks/image/theme.css 137 B
build/block-library/blocks/image/view.min.js 2.01 kB
build/block-library/blocks/latest-comments/style-rtl.css 357 B
build/block-library/blocks/latest-comments/style.css 357 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 505 B
build/block-library/blocks/media-text/style.css 503 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 671 B
build/block-library/blocks/navigation-link/editor.css 672 B
build/block-library/blocks/navigation-link/style-rtl.css 103 B
build/block-library/blocks/navigation-link/style.css 103 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation/editor-rtl.css 2.26 kB
build/block-library/blocks/navigation/editor.css 2.26 kB
build/block-library/blocks/navigation/style-rtl.css 2.27 kB
build/block-library/blocks/navigation/style.css 2.26 kB
build/block-library/blocks/navigation/view.min.js 1.07 kB
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 401 B
build/block-library/blocks/page-list/editor.css 401 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 235 B
build/block-library/blocks/paragraph/editor.css 235 B
build/block-library/blocks/paragraph/style-rtl.css 335 B
build/block-library/blocks/paragraph/style.css 335 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 508 B
build/block-library/blocks/post-comments-form/style.css 508 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 71 B
build/block-library/blocks/post-excerpt/editor.css 71 B
build/block-library/blocks/post-excerpt/style-rtl.css 141 B
build/block-library/blocks/post-excerpt/style.css 141 B
build/block-library/blocks/post-featured-image/editor-rtl.css 666 B
build/block-library/blocks/post-featured-image/editor.css 662 B
build/block-library/blocks/post-featured-image/style-rtl.css 345 B
build/block-library/blocks/post-featured-image/style.css 345 B
build/block-library/blocks/post-navigation-link/style-rtl.css 215 B
build/block-library/blocks/post-navigation-link/style.css 214 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 409 B
build/block-library/blocks/post-template/style.css 408 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-time-to-read/style-rtl.css 69 B
build/block-library/blocks/post-time-to-read/style.css 69 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 125 B
build/block-library/blocks/preformatted/style.css 125 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 335 B
build/block-library/blocks/pullquote/style.css 335 B
build/block-library/blocks/pullquote/theme-rtl.css 168 B
build/block-library/blocks/pullquote/theme.css 168 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 486 B
build/block-library/blocks/query/editor.css 486 B
build/block-library/blocks/query/style-rtl.css 312 B
build/block-library/blocks/query/style.css 308 B
build/block-library/blocks/query/view.min.js 637 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/rss/editor-rtl.css 149 B
build/block-library/blocks/rss/editor.css 149 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 184 B
build/block-library/blocks/search/editor.css 184 B
build/block-library/blocks/search/style-rtl.css 613 B
build/block-library/blocks/search/style.css 613 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/search/view.min.js 471 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 329 B
build/block-library/blocks/shortcode/editor.css 329 B
build/block-library/blocks/site-logo/editor-rtl.css 760 B
build/block-library/blocks/site-logo/editor.css 760 B
build/block-library/blocks/site-logo/style-rtl.css 204 B
build/block-library/blocks/site-logo/style.css 204 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 682 B
build/block-library/blocks/social-links/editor.css 681 B
build/block-library/blocks/social-links/style-rtl.css 1.45 kB
build/block-library/blocks/social-links/style.css 1.45 kB
build/block-library/blocks/spacer/editor-rtl.css 359 B
build/block-library/blocks/spacer/editor.css 359 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 432 B
build/block-library/blocks/table/editor.css 432 B
build/block-library/blocks/table/style-rtl.css 646 B
build/block-library/blocks/table/style.css 645 B
build/block-library/blocks/table/theme-rtl.css 157 B
build/block-library/blocks/table/theme.css 157 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 403 B
build/block-library/blocks/template-part/editor.css 403 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/term-description/style-rtl.css 111 B
build/block-library/blocks/term-description/style.css 111 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 99 B
build/block-library/blocks/verse/style.css 99 B
build/block-library/blocks/video/editor-rtl.css 552 B
build/block-library/blocks/video/editor.css 555 B
build/block-library/blocks/video/style-rtl.css 191 B
build/block-library/blocks/video/style.css 191 B
build/block-library/blocks/video/theme-rtl.css 139 B
build/block-library/blocks/video/theme.css 139 B
build/block-library/classic-rtl.css 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.11 kB
build/block-library/common.css 1.11 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 12.5 kB
build/block-library/editor.css 12.4 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 472 B
build/block-library/reset.css 472 B
build/block-library/theme-rtl.css 700 B
build/block-library/theme.css 705 B
build/block-serialization-default-parser/index.min.js 1.13 kB
build/block-serialization-spec-parser/index.min.js 2.87 kB
build/commands/index.min.js 15.5 kB
build/commands/style-rtl.css 947 B
build/commands/style.css 942 B
build/components/index.min.js 256 kB
build/components/style-rtl.css 12 kB
build/components/style.css 12 kB
build/compose/index.min.js 12.7 kB
build/core-commands/index.min.js 2.72 kB
build/core-data/index.min.js 71.8 kB
build/data-controls/index.min.js 651 B
build/data/index.min.js 8.87 kB
build/date/index.min.js 17.9 kB
build/deprecated/index.min.js 462 B
build/dom-ready/index.min.js 336 B
build/dom/index.min.js 4.68 kB
build/edit-post/classic-rtl.css 571 B
build/edit-post/classic.css 571 B
build/editor/style-rtl.css 3.58 kB
build/editor/style.css 3.58 kB
build/element/index.min.js 4.87 kB
build/escape-html/index.min.js 548 B
build/format-library/index.min.js 7.82 kB
build/format-library/style-rtl.css 577 B
build/format-library/style.css 577 B
build/hooks/index.min.js 1.57 kB
build/html-entities/index.min.js 454 B
build/i18n/index.min.js 3.61 kB
build/interactivity/index.min.js 11.4 kB
build/is-shallow-equal/index.min.js 535 B
build/keyboard-shortcuts/index.min.js 1.76 kB
build/keycodes/index.min.js 1.9 kB
build/list-reusable-blocks/index.min.js 2.21 kB
build/list-reusable-blocks/style-rtl.css 865 B
build/list-reusable-blocks/style.css 865 B
build/media-utils/index.min.js 2.92 kB
build/notices/index.min.js 964 B
build/nux/index.min.js 2.01 kB
build/nux/style-rtl.css 775 B
build/nux/style.css 771 B
build/patterns/style-rtl.css 567 B
build/patterns/style.css 566 B
build/plugins/index.min.js 1.81 kB
build/preferences-persistence/index.min.js 1.85 kB
build/preferences/index.min.js 1.26 kB
build/primitives/index.min.js 994 B
build/priority-queue/index.min.js 1.52 kB
build/private-apis/index.min.js 988 B
build/react-i18n/index.min.js 631 B
build/react-refresh-entry/index.min.js 9.46 kB
build/react-refresh-runtime/index.min.js 6.78 kB
build/redux-routine/index.min.js 2.71 kB
build/reusable-blocks/style-rtl.css 265 B
build/reusable-blocks/style.css 265 B
build/router/index.min.js 1.79 kB
build/server-side-render/index.min.js 1.96 kB
build/shortcode/index.min.js 1.4 kB
build/style-engine/index.min.js 1.98 kB
build/token-list/index.min.js 587 B
build/url/index.min.js 3.83 kB
build/vendors/inert-polyfill.min.js 2.48 kB
build/vendors/react-dom.min.js 41.8 kB
build/vendors/react.min.js 4.02 kB
build/viewport/index.min.js 967 B
build/warning/index.min.js 259 B
build/widgets/index.min.js 7.18 kB
build/widgets/style-rtl.css 1.18 kB
build/widgets/style.css 1.18 kB
build/wordcount/index.min.js 1.03 kB

compressed-size-action

@afercia
Copy link
Contributor Author

afercia commented Nov 10, 2023

Quick screenshot to visually compare other dropdown menus in the block toolbar that use both icons and visible text:

comparison

@t-hamano t-hamano self-requested a review November 10, 2023 13:28
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @afercia!

This PR also adds a stylesheet to the component, which is only used to increase a little the horizontal gap between the heading icons and the visible text.

Personally, I think it would be better to update the style of the dropdown menu itself in a separate PR. For example, even with the align control, the space between the icon and the text looks a little cramped for me.

width
align-control

The gap between the icon and text is 4px, which was changed from 8px by #50277. 8px feels more natural to me in a UI where icons may be pressed, like this dropdown menu.

@jasmussen @jameskoster Please let me know if you have any opinions on this point 🙏

@afercia
Copy link
Contributor Author

afercia commented Nov 10, 2023

@t-hamano thanks. Yes that could be an option.
Notice that also the gap of the 'More' dropdown is inconsistent with the gap of the Block Switcher dropdown. CSS and also marlup are a little custom.
I'd tend to think these dropdowns should use the same components / markup pattern. The same default CSS. To do so, maybe the button component itself should have props to set different gap values between the icon and the visible text.

@jasmussen
Copy link
Contributor

Please let me know if you have any opinions on this point 🙏

Serendipitiously I think @jameskoster has been working on dropdown menus lately, and had thoughts on this exact thing.

@afercia
Copy link
Contributor Author

afercia commented Nov 13, 2023

The gap between the icon and text is 4px, which was changed from 8px by #50277. 8px feels more natural to me in a UI where icons may be pressed, like this dropdown menu.

I'd agree #50277 didn't take into consideration buttons with a pressed state. The pressed state just makes more visible what the actual SVG viewBox width is, which is different from the width of the SVG glyph.

The editor SVG icons have varying glyphs: some glyphs take only a part of the actual SVG viewBox width thus leaving some horizontal spacing within the viewBox. Other icons take almost all the available viewBox width. Hence, having a consistent padding with adjacent element is basically impossible.

While #50277 improved the look of icon + label for some use cases, I think it actually made it worse for other use cases and that change should probably be reconsidered.

To illustrate, here's some examples with icons glyphs that have different widths, thus producing a visual uneven spacing:

Screenshot 2023-11-13 at 16 16 28

That said, this PR is not about the padding. It's about the functionality. I adjusted the padding only because, for this menu, 4px looked ugly.

I think it would be better to update the style of the dropdown menu itself in a separate PR.

I will remove the CSS change, but the padding issue should be addressed in a separate PR taking into consideration all the use cases.

@t-hamano t-hamano self-requested a review November 14, 2023 12:27
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

LGTM! Since this PR is about improving functionality, I think we can merge this PR and address the issue of the gap between icon and text in another PR.

Desktop

desktop

Mobile

mobile

One thing I came up with is that if we make the entire icon and text "pressed" style as shown below, it might be okay to have a gap of 4px between the icon and the text.

pressed

@jameskoster If there is an issue number for improving the design of the dropdown, please let us know.

@jameskoster
Copy link
Contributor

The spec for the new flyout menu component(s) can be found in #55933.

One thing to note relating to this PR; that spec does not include a configuration where MenuItems are selectable and have a decorative icon. So at some point we'll need to make a decision about how to proceed here, either no icons, or icons only (with tooltips):

Screenshot 2023-11-14 at 15 44 47

@afercia
Copy link
Contributor Author

afercia commented Nov 16, 2023

if we make the entire icon and text "pressed" style as shown below, it might be okay to have a gap of 4px between the icon and the text.

Personally, I like the idea.

One thing to note relating to this PR; that spec does not include a configuration where MenuItems are selectable and have a decorative icon.

@jameskoster I'll comment on #55933 and ping you there

So at some point we'll need to make a decision about how to proceed here, either no icons, or icons only (with tooltips):

I'm not sure I understand why it should be either one or the other. Icon + visible text is fine to me, when the visual hint provided by the icon adds clarity.
Icon only is less then ideal for usability and accessibility and should be avoided. The usage of icon-only buttons should be liited to only the cases where the horizontal space is limited e.g. toolbars.

Also, I would like to point out that the button component, which is at the base od the dropdown implementation, does have props to show both the icon and text and it does have a pressed state style that includes the text, see the storybook: https://wordpress.github.io/gutenberg/?path=/story/components-button--icon&args=isPressed:!true;text:Some+text;aria-pressed:true

Screenshot 2023-11-16 at 14 21 46

@afercia
Copy link
Contributor Author

afercia commented Nov 16, 2023

Turns out the reason why the dark background doesn't apply to the text is that these menu items use a is-active CSS class instead of is-pressed.

@afercia
Copy link
Contributor Author

afercia commented Nov 16, 2023

Noting that the gap issue is no different from what already happens with other dropdown menus, fgr example the Align dropdown:

Screenshot 2023-11-16 at 14 32 54

As such, I will leave it as is for now. This should be addressed holistically,

@afercia afercia merged commit e3e9dd0 into trunk Nov 16, 2023
51 checks passed
@afercia afercia deleted the fix/heading-level-dropdown-obtrusive-tooltip branch November 16, 2023 13:37
@github-actions github-actions bot added this to the Gutenberg 17.1 milestone Nov 16, 2023
@afercia afercia mentioned this pull request Nov 16, 2023
63 tasks
@carolinan
Copy link
Contributor

carolinan commented Nov 17, 2023

either no icons, or icons only

@jameskoster Why are these the only two options? I think that valid arguments for having visible text has been brought up over and over. That icons are "globally recognised" may be an overstatement, but they can enhance and make it easier to select the correct options even when there is text.

@richtabor
Copy link
Member

richtabor commented Nov 22, 2023

Does "Heading 2" convey more information than the "H2" icon? Seems duplicative; so I can see how an icon-only instance of this particular case would be fine.

Anyhow, I'd like to further explore how level switching and transforms are connected in #55520.

@afercia
Copy link
Contributor Author

afercia commented Nov 24, 2023

so I can see how an icon-only instance of this particular case would be fine.

I kindly disagree. As mentioned several times, text is always preferable, Icon-only needs to be avoided.

@richtabor
Copy link
Member

I kindly disagree. As mentioned several times, text is always preferable, Icon-only needs to be avoided.

The nuance with this particular control is that the icon is representative of text. H1 is graphically depicted exactly as "H1". In this case, we may not even need icons, but just text.

Also, I'd say that's your preference. There are many instances throughout all sorts of experiences where universally accepted iconography promotes a much more understandable UI. For example, the bold, italic, and link controls in the block toolbar (and just about every other text editing UI).

@afercia
Copy link
Contributor Author

afercia commented Nov 27, 2023

The nuance with this particular control is that the icon is representative of text.

Regarding this specific case: But still it is not text. SInce it's not text, we need to give the button an accessible name via aria-label, which introduces one more problem. Basically what we have till WordPress 6.4:

<button ... aria-label="Heading 1">{H1 icon}</button>

Is not accessible because, visually, it shows H1 but the accessible name is Heading 1. It's a mismatch between the visible 'label' of a control and its name, which is a violation of WCAG Label in Name.

Regarding the general icon-only issue:

Also, I'd say that's your preference.

There's tons of research and data about icons not being universally understandable. To mention just one of them:

Nielsen Norman Group
Icon Usability
Summary: A user’s understanding of an icon is based on previous experience. Due to the absence of a standard usage for most icons, text labels are necessary to communicate the meaning and reduce ambiguity.
https://www.nngroup.com/articles/icon-usability/

Also, we need to expand a little from our Western-centric cultural background and include other cultural backgrounds where symbols may have completely different meanings. The checkmark symbol is probably the case (not the only one) that better illustrates these cultural differences. In Swedish schools, in Finland and in Japan, the checkmark means 'wrong'.

So no, I wouldn't say this is 'my preference'.

@richtabor
Copy link
Member

Is not accessible because, visually, it shows H1 but the accessible name is Heading 1. It's a mismatch between the visible 'label' of a control and its name, which is a violation of WCAG Label in Name.

If "H1" is an icon, how is this a violation? Is a trash can icon for "Delete" also a violation?

So no, I wouldn't say this is 'my preference'.

A icons-less experience will certainly be a less usable experience for everyone. There is a balance; repetitiveness and increased cognitive load is not it. I'd argue a "H1" icon on a heading block is clearly the Heading Level 1. There is no room for misinterpretation and unexpected results by selecting it.

Also, we need

That is your perspective; and totally fine, though I don't particularly share the need to change this case. Perhaps you'd like to explore alternatives to see how to best expose selections, though I'm thinking the checkmark is overwhelmingly the supported icon for such use cases.

@afercia
Copy link
Contributor Author

afercia commented Nov 28, 2023

If "H1" is an icon, how is this a violation? Is a trash can icon for "Delete" also a violation?

I'm not sure i know how to explain it more clearly. Also, the H1 icon and a 'Delete' icon are very different cases. The first one visually looks like text. The second one is graphics. DId you take the time to read the WCAG criteria? There is an Understanding Label in Name link there.

Also, we need to expand a little from our Western-centric ...

That is your perspective; and totally fine, though I don't particularly share the need to change this case. Perhaps you'd like to explore alternatives to see how to best expose selections, though I'm thinking the checkmark is overwhelmingly the supported icon for such use cases.

I woudn't say it's my perspective. The checkmark is just an example. I thought it was clear it was an example. The general principle applies to all icon. I pointed to an authoritative resource, the Nielsen Norman Group, which clearly explains why icon-nly is a problem, That is based on actual research, user testing, and data. There's plenty of research around icon-only and they all coem to rhe same conclusion. I'm not sure our time is best spent on cycling back over and over on the same points. From my side, I already provided my feedback and recommendations.

@richtabor
Copy link
Member

Did you take the time to read the WCAG criteria?

I am of course quite familiar, yes.

I'm not sure our time is best spent on cycling back over and over on the same points.... I already provided my feedback and recommendations.

I concur. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Heading Affects the Headings Block [Block] Site Title Affects the Site Title Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Heading level tooltip covers subsequent level
6 participants