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

Snackbar: re-position snackbar above modals #54153

Closed
wants to merge 5 commits into from

Conversation

mikachan
Copy link
Member

@mikachan mikachan commented Sep 4, 2023

What?

The snackbar notifications currently show underneath the modals, which becomes more of an issue when the modal takes up a large percentage of the screen (e.g. at smaller resolutions, or where large modals are used). This PR adjusts the position and z-index of the snackbar component to allow it to display over the top of the modal.

Why?

Closes #52609.

How?

This PR wraps the SnackbarList component in createPortal, in order to attach it to the body. This is similar to how the Modal component works. This means the z-index can then be changed to allow the snackbar notifications to display over the top of the modal. The positioning of the snackbar has also been slightly adjusted as its position is now relative to the body element.

Testing Instructions

The easiest way to test this is to test on a smaller viewport:

  1. Set the viewport to a small enough size so that the modal takes up the majority of the screen, e.g. 900 x 595.
  2. Update a setting that triggers a snackbar notification, such as changing the global text color.
  3. Open the Preferences modal whilst the snackbar notification is still open, and check to see if the notification displays over the top of the modal.
  4. Alternatively, comment out the timeout dismiss in the snackbar component (in /packages/components/src/snackbar/index.tsx, line 91-101), trigger a snackbar notification and then open a modal.

Screenshots or screencast

Before After
image image

@mikachan mikachan added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components [Package] Base styles /packages/base-styles labels Sep 4, 2023
@mikachan mikachan self-assigned this Sep 4, 2023
@github-actions
Copy link

github-actions bot commented Sep 4, 2023

Size Change: +30 B (0%)

Total Size: 1.51 MB

Filename Size Change
build/components/index.min.js 245 kB +11 B (0%)
build/components/style-rtl.css 11.8 kB +10 B (0%)
build/components/style.css 11.8 kB +9 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 955 B
build/annotations/index.min.js 2.69 kB
build/api-fetch/index.min.js 2.28 kB
build/autop/index.min.js 2.1 kB
build/blob/index.min.js 451 B
build/block-directory/index.min.js 7.01 kB
build/block-directory/style-rtl.css 1.02 kB
build/block-directory/style.css 1.02 kB
build/block-editor/content-rtl.css 4.25 kB
build/block-editor/content.css 4.24 kB
build/block-editor/default-editor-styles-rtl.css 381 B
build/block-editor/default-editor-styles.css 381 B
build/block-editor/index.min.js 214 kB
build/block-editor/style-rtl.css 15 kB
build/block-editor/style.css 15 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 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 584 B
build/block-library/blocks/button/editor.css 582 B
build/block-library/blocks/button/style-rtl.css 629 B
build/block-library/blocks/button/style.css 628 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.63 kB
build/block-library/blocks/cover/style.css 1.62 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 126 B
build/block-library/blocks/embed/theme.css 126 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-interactivity.min.js 317 B
build/block-library/blocks/file/view.min.js 375 B
build/block-library/blocks/footnotes/style-rtl.css 201 B
build/block-library/blocks/footnotes/style.css 199 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 947 B
build/block-library/blocks/gallery/editor.css 952 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 336 B
build/block-library/blocks/html/editor.css 337 B
build/block-library/blocks/image/editor-rtl.css 834 B
build/block-library/blocks/image/editor.css 833 B
build/block-library/blocks/image/style-rtl.css 1.42 kB
build/block-library/blocks/image/style.css 1.41 kB
build/block-library/blocks/image/theme-rtl.css 126 B
build/block-library/blocks/image/theme.css 126 B
build/block-library/blocks/image/view-interactivity.min.js 1.83 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 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.26 kB
build/block-library/blocks/navigation/editor.css 2.26 kB
build/block-library/blocks/navigation/style-rtl.css 2.23 kB
build/block-library/blocks/navigation/style.css 2.22 kB
build/block-library/blocks/navigation/view-interactivity.min.js 988 B
build/block-library/blocks/navigation/view-modal.min.js 2.85 kB
build/block-library/blocks/navigation/view.min.js 469 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 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 588 B
build/block-library/blocks/post-featured-image/editor.css 586 B
build/block-library/blocks/post-featured-image/style-rtl.css 319 B
build/block-library/blocks/post-featured-image/style.css 319 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 314 B
build/block-library/blocks/post-template/style.css 314 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 302 B
build/block-library/blocks/query-pagination/style.css 299 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 450 B
build/block-library/blocks/query/editor.css 449 B
build/block-library/blocks/query/style-rtl.css 370 B
build/block-library/blocks/query/style.css 368 B
build/block-library/blocks/query/view.min.js 559 B
build/block-library/blocks/quote/style-rtl.css 222 B
build/block-library/blocks/quote/style.css 222 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 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 178 B
build/block-library/blocks/search/editor.css 178 B
build/block-library/blocks/search/style-rtl.css 607 B
build/block-library/blocks/search/style.css 607 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 631 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 323 B
build/block-library/blocks/shortcode/editor.css 323 B
build/block-library/blocks/site-logo/editor-rtl.css 754 B
build/block-library/blocks/site-logo/editor.css 754 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.44 kB
build/block-library/blocks/social-links/style.css 1.43 kB
build/block-library/blocks/spacer/editor-rtl.css 348 B
build/block-library/blocks/spacer/editor.css 348 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 639 B
build/block-library/blocks/table/style.css 639 B
build/block-library/blocks/table/theme-rtl.css 146 B
build/block-library/blocks/table/theme.css 146 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 185 B
build/block-library/blocks/video/style.css 185 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 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.1 kB
build/block-library/common.css 1.1 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.2 kB
build/block-library/editor.css 12.1 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/index.min.js 203 kB
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/style-rtl.css 13.8 kB
build/block-library/style.css 13.8 kB
build/block-library/theme-rtl.css 688 B
build/block-library/theme.css 693 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.87 kB
build/blocks/index.min.js 51.4 kB
build/commands/index.min.js 15.5 kB
build/commands/style-rtl.css 932 B
build/commands/style.css 929 B
build/compose/index.min.js 12.1 kB
build/core-commands/index.min.js 2.72 kB
build/core-data/index.min.js 16.8 kB
build/customize-widgets/index.min.js 12 kB
build/customize-widgets/style-rtl.css 1.46 kB
build/customize-widgets/style.css 1.45 kB
build/data-controls/index.min.js 640 B
build/data/index.min.js 8.38 kB
build/date/index.min.js 17.8 kB
build/deprecated/index.min.js 451 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.64 kB
build/edit-post/classic-rtl.css 544 B
build/edit-post/classic.css 545 B
build/edit-post/index.min.js 35.5 kB
build/edit-post/style-rtl.css 7.62 kB
build/edit-post/style.css 7.62 kB
build/edit-site/index.min.js 91 kB
build/edit-site/style-rtl.css 13.2 kB
build/edit-site/style.css 13.2 kB
build/edit-widgets/index.min.js 16.9 kB
build/edit-widgets/style-rtl.css 4.52 kB
build/edit-widgets/style.css 4.52 kB
build/editor/index.min.js 45.5 kB
build/editor/style-rtl.css 3.53 kB
build/editor/style.css 3.52 kB
build/element/index.min.js 4.82 kB
build/escape-html/index.min.js 537 B
build/format-library/index.min.js 7.73 kB
build/format-library/style-rtl.css 554 B
build/format-library/style.css 553 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.58 kB
build/interactivity/index.min.js 11.2 kB
build/is-shallow-equal/index.min.js 527 B
build/keyboard-shortcuts/index.min.js 1.64 kB
build/keycodes/index.min.js 1.87 kB
build/list-reusable-blocks/index.min.js 2.2 kB
build/list-reusable-blocks/style-rtl.css 836 B
build/list-reusable-blocks/style.css 836 B
build/media-utils/index.min.js 2.9 kB
build/notices/index.min.js 948 B
build/nux/index.min.js 1.99 kB
build/nux/style-rtl.css 735 B
build/nux/style.css 732 B
build/patterns/index.min.js 2.71 kB
build/patterns/style-rtl.css 240 B
build/patterns/style.css 240 B
build/plugins/index.min.js 1.79 kB
build/preferences-persistence/index.min.js 1.84 kB
build/preferences/index.min.js 1.24 kB
build/primitives/index.min.js 943 B
build/priority-queue/index.min.js 1.52 kB
build/private-apis/index.min.js 958 B
build/react-i18n/index.min.js 615 B
build/react-refresh-entry/index.min.js 9.47 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.7 kB
build/reusable-blocks/index.min.js 2.7 kB
build/reusable-blocks/style-rtl.css 243 B
build/reusable-blocks/style.css 243 B
build/rich-text/index.min.js 11 kB
build/router/index.min.js 1.78 kB
build/server-side-render/index.min.js 1.94 kB
build/shortcode/index.min.js 1.39 kB
build/style-engine/index.min.js 1.85 kB
build/sync/index.min.js 53.8 kB
build/token-list/index.min.js 582 B
build/url/index.min.js 3.73 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 958 B
build/warning/index.min.js 249 B
build/widgets/index.min.js 7.16 kB
build/widgets/style-rtl.css 1.15 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.02 kB

compressed-size-action

@joedolson
Copy link
Contributor

I don't think that this is a good change. If a user is opening a modal, then they're expecting that content to be what's in their viewport. If the snackbar conceals content or controls (e.g. for low vision users at high magnification or small devices), then a keyboard or screen reader user has to close the modal, then dismiss the snackbar, then re-open the modal in order to gain access to that control.

Snackbars shouldn't be used for notifications that are of such critical importance that a user must see them or deal with them, because they have a lot of accessibility concerns, particularly for low vision. And as low-importance notifications, they should also not be forced above the user's intended context.

@Marc-pi
Copy link

Marc-pi commented Sep 4, 2023

personaly i would rethink the position of the snackbar #47446, that is located right at the opposite side of the button that generates it, which is not UX impactfull at all ;-) (proximity concept)

@mikachan
Copy link
Member Author

mikachan commented Sep 5, 2023

@joedolson, thanks for leaving such detailed feedback! I agree that it's not a great experience when a user may want to dismiss a snackbar notification, as this action closes both the snackbar and the modal.

Snackbars shouldn't be used for notifications that are of such critical importance that a user must see them or deal with them, because they have a lot of accessibility concerns, particularly for low vision. And as low-importance notifications, they should also not be forced above the user's intended context.

I understand that the snackbar notifications are of low importance, however, I'm not sure it's a great experience when the user can only see a portion of the notification when it's obscured by the modal, especially in the case where a snackbar notification is shown while a modal is already opened (for example, in the new Font Library modal, shown in the video here).

I'm wondering if we could introduce a modal-friendly snackbar notification that, for example, always shows at the bottom of the modal, so it's displayed within the modal itself. If a modal is open, we could also disable the current snackbar, so only one shows at a time.

This issue may be specific to the new Font Library modal, so perhaps some additional design input is needed here. @jasmussen, what do you think of the current snackbar notifications that show when adding and deleting fonts in the new Font Library modal? They currently display like this:

Larger screen Smaller screen
image image

@Marc-pi, thanks for linking that issue! I'm not sure the position of the snackbar should be changed to be closer to the 'save' button, as other actions in the editor also trigger snackbar notifications, so changing the position wouldn't always guarantee that it would be closer to the action that triggered it. Also, in the case of large modals that take up the majority of the screen, the snackbar would still be behind the modal.

@madhusudhand
Copy link
Member

I'm wondering if we could introduce a modal-friendly snackbar notification that, for example, always shows at the bottom of the modal, so it's displayed within the modal itself. If a modal is open, we could also disable the current snackbar, so only one shows at a time.

Adding to this thought, Instead of embedding a snackbar into the modal, may be a modal can have its own messaging system with props and as you said a dedicated area in the footer (bottom left) section to show success or error messages.

@mikachan
Copy link
Member Author

mikachan commented Sep 5, 2023

may be a modal can have its own messaging system with props and as you said a dedicated area in the footer (bottom left) section to show success or error messages

Yes, this is exactly what I was thinking!

@matiasbenedetto
Copy link
Contributor

matiasbenedetto commented Sep 5, 2023

Snackbars shouldn't be used for notifications that are of such critical importance that a user must see them or deal with them, because they have a lot of accessibility concerns, particularly for low vision. And as low-importance notifications, they should also not be forced above the user's intended context.

@joedolson do you think this is a general problem in the current notification system in the editor, or do you detect this only on the changes introduced in this particular PR?

@alexstine
Copy link
Contributor

One of the concerns I also had as a keyboard user was not being able to dismiss the notification since focus is trapped in Modal component. It is also entirely possible the notification UI is no longer accessible after being wrapped in React portal, you must be careful with accessibility in portals as DOM positioning can be tricky.

Is there a way I can test this in the post editor? No site editor, please.

Thanks.

@alexstine alexstine added the Needs Accessibility Feedback Need input from accessibility label Sep 5, 2023
@joedolson
Copy link
Contributor

I understand that the snackbar notifications are of low importance, however, I'm not sure it's a great experience when the user can only see a portion of the notification when it's obscured by the modal, especially in the case where a snackbar notification is shown while a modal is already opened (for example, in the new Font Library modal, shown in the video #52609).

Based on the instructions to test above, the scenario being tested is when an action spawns a snackbar and then a modal is opened. In that scenario, the snackbar should remain below the modal. If the snackbar is spawned after the modal is opened, then that's a different situation. The ordering of layers should match the ordering of events: if a modal is opened on top of an existing snackbar, then it should not be blocked by it.

And @alexstine's concern is extremely important: a user needs to be able to dismiss a visible notification without needing to first dismiss their current context.

@mikachan
Copy link
Member Author

mikachan commented Sep 5, 2023

One of the concerns I also had as a keyboard user was not being able to dismiss the notification since focus is trapped in Modal component.

Yes I agree, I think this may be the biggest blocker to re-positioning the snackbar to being above the modal. From testing it just now, I believe focus is trapped in the modal, so you're unable to interact with the snackbar with the modal open.

It is also entirely possible the notification UI is no longer accessible after being wrapped in React portal, you must be careful with accessibility in portals as DOM positioning can be tricky.

I think this is also true 😞 Apologies, I should've tested this more thoroughly before suggesting it as a solution.

Is there a way I can test this in the post editor? No site editor, please.

You can open the Preferences modal from the post editor, but I'm not sure of a setting that triggers the snackbar in the post editor. You need a combination of a snackbar notification being open whilst a modal is open, and I'm not sure this is as easily triggered in the post editor. For more context on why this combination is important, it's because this issue was raised whilst working on the Font Library front end PR, which introduces a modal to manage font files. Adding and removing fonts currently triggers a snackbar notification whilst the Font Library modal is open, but the snackbar is underneath the modal, and can be difficult to read depending on the screen size.

Thanks @alexstine, I think your above comments alone are enough for us to discuss an alternative solution. 🙇

I'm not sure of any other settings in either editor that trigger a snackbar notification whilst a modal is open, so this may be specific to the Font Libary work. What are good alternatives to using the snackbar for update notifications in this case? Here are a couple I can think of:

  • Introduce a notification element within the modal itself. This could be used whenever a notification is triggered when a modal is already open.
  • Don't use notifications at all when a modal is open, including for the new Font Library. How important is the visual feedback that something has been updated? For example, the Preferences modal does not offer any feedback when a setting is changed.

@alexstine
Copy link
Contributor

Second option sounds good to me personally. 👍

@jasmussen
Copy link
Contributor

Good PR, I will defer to the experts already in the thread, and only respond to this:

This issue may be specific to the new Font Library modal, so perhaps some additional design input is needed here. @jasmussen, what do you think of the current snackbar notifications that show when adding and deleting fonts in the new Font Library modal? They currently display like this:

The font library should not use a snackbar at all, as I also suggested in this comment. The upload flow as it works at the moment is a bit flawed, and diverges quite a bit from how you upload images. The models there should be identical so uploading always follows the same model. In the comment I go into more depth, but essentially uploading should be either:

  • Drop the font(s) file into the modal OR click the button and upload fonts
  • Font(s) upload immediately
  • You are taken to the font page of the uploaded font

This is visualized in the mockups in #45271 and it is quite important that those mockups are followed, the current PR diverges in a few places.

@matiasbenedetto
Copy link
Contributor

matiasbenedetto commented Sep 9, 2023

The font library should not use a snackbar at all, as I also suggested in #53884 (comment).

OK, we can remove them.

The upload flow as it works at the moment is a bit flawed, and diverges quite a bit from how you upload images. The models there should be identical so uploading always follows the same model. In the comment I go into more depth, but essentially uploading should be either:

Drop the font(s) file into the modal OR click the button and upload fonts
Font(s) upload immediately
You are taken to the font page of the uploaded font

It's currently working like this. The last time you tested the PR, it was different. Please try the latest version.

Apart from that, @jasmussen, please provide an alternative to display error messages when the user actions fail (for example: WordPress doesn't have write permissions on the server).

@jasmussen
Copy link
Contributor

please provide an alternative to display error messages when the user actions fail (for example: WordPress doesn't have write permissions on the server).

How's this?

Upload error

That's the regular Notice component. I think the question around the snackbar for this error is valid enough, as is the PR here. Important just to not let this block the fonts library, if the notice can help that to happen.

@mikachan
Copy link
Member Author

mikachan commented Sep 11, 2023

Thanks for your detailed feedback here, @jasmussen. I didn't realise the Snackbar shouldn't be used; I think that means we should close this PR and the related issue at #52609.

@mikachan mikachan closed this Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Accessibility Feedback Need input from accessibility [Package] Base styles /packages/base-styles [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notification snackbar is below modals
7 participants