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

Global styles: keep custom CSS when switching between style variations #56623

Closed

Conversation

glendaviesnz
Copy link
Contributor

@glendaviesnz glendaviesnz commented Nov 29, 2023

What?

Preserves user custom CSS when switching between style variations

Why?

User can find it surprising that their custom CSS is lost when they switch to a different theme variation.

Fixes: #47433

How?

Adds a toggle to specify if any existing custom CSS should be copied over to the new style variation CSS

Testing Instructions

  • Add some global custom CSS to a theme including block custom CSS
  • Go to the site editor style variation picker, pick another style and check that your custom CSS is preserved
  • Toggle off Keep additional CSS and check that the custom CSS is wiped

Screenshots or screencast

Before:

save-css-before.mp4

After:

save-css-after.mp4

@glendaviesnz glendaviesnz added [Type] Enhancement A suggestion for improvement. Needs Design Needs design efforts. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Nov 29, 2023
@glendaviesnz glendaviesnz self-assigned this Nov 29, 2023
@glendaviesnz
Copy link
Contributor Author

@richtabor, @SaxonF this is a draft PR that stops custom CSS being wiped when switching theme variations.

UX wise, do you think it needs some explanation on the variation switcher menu to say the custom CSS will be preserved, or an option there to toggle whether it is preserved or wiped? If you think the toggle to choose is needed where would you place that and how would you word it?

Copy link

github-actions bot commented Nov 29, 2023

Size Change: +1.69 kB (0%)

Total Size: 1.72 MB

Filename Size Change
build/block-editor/index.min.js 248 kB +4 B (0%)
build/block-library/blocks/social-links/style-rtl.css 1.47 kB +19 B (+1%)
build/block-library/blocks/social-links/style.css 1.47 kB +19 B (+1%)
build/block-library/index.min.js 213 kB +674 B (0%)
build/block-library/style-rtl.css 14.7 kB +20 B (0%)
build/block-library/style.css 14.7 kB +21 B (0%)
build/components/index.min.js 256 kB +23 B (0%)
build/components/style-rtl.css 12.1 kB +57 B (0%)
build/components/style.css 12.1 kB +58 B (0%)
build/edit-post/style-rtl.css 7.6 kB +26 B (0%)
build/edit-post/style.css 7.59 kB +26 B (0%)
build/edit-site/index.min.js 209 kB +406 B (0%)
build/edit-site/style-rtl.css 14.6 kB +156 B (+1%)
build/edit-site/style.css 14.6 kB +159 B (+1%)
build/editor/index.min.js 47.4 kB +3 B (0%)
build/editor/style-rtl.css 3.74 kB -2 B (0%)
build/editor/style.css 3.73 kB -1 B (0%)
build/interactivity/index.min.js 12.5 kB +21 B (0%)
ℹ️ 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 590 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.29 kB
build/block-editor/content.css 4.28 kB
build/block-editor/default-editor-styles-rtl.css 403 B
build/block-editor/default-editor-styles.css 403 B
build/block-editor/style-rtl.css 15.7 kB
build/block-editor/style.css 15.7 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 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 322 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.75 kB
build/block-library/blocks/gallery/style.css 1.75 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 834 B
build/block-library/blocks/image/editor.css 833 B
build/block-library/blocks/image/style-rtl.css 1.61 kB
build/block-library/blocks/image/style.css 1.6 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.02 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.04 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 647 B
build/block-library/blocks/quote/style-rtl.css 237 B
build/block-library/blocks/quote/style.css 237 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 140 B
build/block-library/blocks/read-more/style.css 140 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 475 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/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/blocks/index.min.js 51 kB
build/commands/index.min.js 15.5 kB
build/commands/style-rtl.css 947 B
build/commands/style.css 942 B
build/compose/index.min.js 12.7 kB
build/core-commands/index.min.js 2.72 kB
build/core-data/index.min.js 72.6 kB
build/customize-widgets/index.min.js 12.1 kB
build/customize-widgets/style-rtl.css 1.43 kB
build/customize-widgets/style.css 1.43 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/edit-post/index.min.js 35.8 kB
build/edit-widgets/index.min.js 17.2 kB
build/edit-widgets/style-rtl.css 4.65 kB
build/edit-widgets/style.css 4.65 kB
build/element/index.min.js 4.87 kB
build/escape-html/index.min.js 548 B
build/format-library/index.min.js 7.76 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/file.min.js 442 B
build/interactivity/image.min.js 2.15 kB
build/interactivity/navigation.min.js 1.16 kB
build/interactivity/query.min.js 791 B
build/interactivity/search.min.js 610 B
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.11 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/modules/importmap-polyfill.min.js 12.2 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/index.min.js 4.84 kB
build/patterns/style-rtl.css 564 B
build/patterns/style.css 564 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/index.min.js 2.74 kB
build/reusable-blocks/style-rtl.css 265 B
build/reusable-blocks/style.css 265 B
build/rich-text/index.min.js 9.98 kB
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

@jasmussen
Copy link
Contributor

Question: what happens if you switch from a block theme to a classic theme, is it preserved?

Asking because when you switch from a block theme to a block theme, we surface this if you preview first:

2023-11-29 09 10 50 local-wordpress local 7370759f6b1f

Really, that activation dialog should always show, regardless of whether you preview or not, when you switch from a block theme to a block theme. Can we make that happen? And can we show, on that list, a checkbox that says "Custom CSS", checked by default? We ideally need to show every change, in a long and detailed list, in that modal, just like we do when you change titles or styles:

2023-11-29 09 11 18 local-wordpress local e7e9498a695b

If we can't do this immediately, we might need a lower level solution — a notice? A snackbar? But long term, we need to make progress on block theme switching improvements. Here are some thoughts that deserve a good mockup to tie the room together:

  • The multi entity saving dialog could use visual improvements, this is tracked separately by @jameskoster I believe.
  • This dialog should break out every different aspect we can, and show help text for each of them explaining what change is being made.
  • This same dialog should be used not just for saving multiple changes in the site editor, but for scheduling such changes, and even for switching from block theme to block theme.
  • When switching from a block theme to another block theme, this same dialog should break out every aspect of that theme switching flow: keep or discard old/new patterns, keep or discard old/new colors, keep or discard old/new fonts, keep or discard old/new templates even on a per-template level through an advanced interface.

Ultimately you should be able to combine all of those aspects together. Consider this: for Black Friday I'm having a sale on my store. So I change my site logo, I add a suffix to my site title, I change my colors and fonts, I add a banner at the top of my header pattern, and I switch to a new block theme but only to get the new templates, nothing else. Not only that, but after queueing up all these changes, I schecule them to go live on November 24th.

It's a tall order, but the single interface here should be able to break out all those changes, list them, allow me to schedule in a split save button/dropdown combo. Considering this as the end state, what do we do today?

Nice PR!

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Nov 29, 2023

Question: what happens if you switch from a block theme to a classic theme, is it preserved?

The custom CSS is saved against the block theme, so if you switch to a classic theme it won't transfer to the classic theme, but it will still be there when you switch back to the block theme.

The switching between block themes is a slightly different issue as each theme maintains its own global styles entity, so you don't lose your custom CSS in the same way when switching themes as you do when switching variations.

Your suggested enhancements sound great, but I wonder if we should focus on just the custom CSS data loss issue when switching variations in this PR - but in doing so being mindful of the direction the wider theme switching flows might be heading?

@jasmussen
Copy link
Contributor

The switching between block themes is a slightly different issue as each theme maintains its own global styles entity, so you don't lose your custom CSS in the same way when switching themes as you do when switching variations.

Ah yes, so it's definitely similar to copying over the patterns, templates, colors, fonts, or other styles from your old block theme to the new one. Only, those don't "feel" like data-loss, because they are understood to be theme related, whereas (and I agree), custom CSS feels like it's a separate layer on top.

Your suggested enhancements sound great, but I wonder if we should focus on just the custom CSS data loss issue when switching variations in this PR - but in doing so being mindful of the direction the wider theme switching flows might be heading?

Absolutely and exactly — it's just important that we visualize the end-state, so that whatever fixes we apply in the mean time do not prevent us from moving to that endstate at a later time.

  • Copying the custom CSS over without asking today, but letting you uncheck that in a multi entity saving window at a later time, possible?
  • Outside of any other ideas Rich and Saxon might suggest (happy to defer to them), can we just show a snackbar upon successful theme activation saying "Your custom CSS was copied over to this theme", perhaps even with a deep-link if possible to the custom CSS input field being open?

@glendaviesnz
Copy link
Contributor Author

Thanks for the feedback @jasmussen, I will explore some different options for notifying the user about what is happening - @richtabor, @SaxonF feel free to still add in any thoughts you might have on UX around this issue with custom css being lost when switching variations.

@glendaviesnz glendaviesnz changed the title Global styles: keep custom CSS when switching between variations Global styles: keep custom CSS when switching between style variations Nov 29, 2023
@glendaviesnz glendaviesnz marked this pull request as ready for review November 29, 2023 22:44
@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Nov 29, 2023

Having had another look at this, with the style variation switching a dialog to confirm keeping of custom CSS doesn't seem to work as users are likely to want to switch quite quickly between style variations and a dialog before applying each time is going to be painful.

I have added a toggle to this PR, defaulting to on, which preserves any additional CSS when toggling between variations - and they can then see what the variation looks like with and without their custom css.

This doesn't fix the wider issue here of all user global styles settings being lost - this is a much trickier issue as these would require a complex merging of user and variation styles. I think the solution to that issue would be to create new global styles entity for each variation switch so at least the custom styles are preserved for the variation they were applied to and still there when switching back. This sort of elevates a variation change to the same level as a theme change so it needs a bit more discussion about whether we want that or not.

We could fix the custom CSS though independently while we decide if the variation switching process should be reviewed or not.

This PR is definitely just a suggested approach, open to ideas/feedback on this.

...( blockStyles[ blockName ]
? blockStyles[ blockName ]
: {} ),
css: user.styles.blocks[ blockName ].css,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need to modify this as theme variations can have block level CSS set so we need to merge instead of overwriting here

@glendaviesnz
Copy link
Contributor Author

The toggle isn't quite working correctly yet - toggle on and off doesn't always remove and apply the custom CSS correctly - will get it fixed.

Copy link

Flaky tests detected in fbdd11a.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7041635765
📝 Reported issues:

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Nov 30, 2023

Ok, the toggle is working more how it should, providing a live preview of how the style variation looks with and without your custom CSS:

toggle.mp4

The code needs a tidy up and a good test, but I will hold off doing that until there is some agreement that this is worth implementing as a temporary fix to this issue while wider changes to variation/theme swapping are explored.

@glendaviesnz glendaviesnz changed the title Global styles: keep custom CSS when switching between style variations [PcC] Global styles: keep custom CSS when switching between style variations Nov 30, 2023
...variation,
settings: variation.settings ?? {},
styles: variation.styles ?? {},
} ) ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will extract the following into a method/hook, simplify the object spreading, and add some tests if there is some agreement to go with this approach

currentUserStyles.styles.css,
preserveAdditionalCSS,
] );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would be good to move the below to an event handler rather than useEffect, but need to refactor the update flows slightly to do this - will worry about that if we decide to go ahead with this refactor

@glendaviesnz glendaviesnz changed the title [PcC] Global styles: keep custom CSS when switching between style variations Global styles: keep custom CSS when switching between style variations Nov 30, 2023
@glendaviesnz
Copy link
Contributor Author

@richtabor, @SaxonF did you have any thoughts on this approach to keeping the custom css when switching variations? It seems like there needs to be some wider discussion about how user global style changes are managed in the variations context, but this change could at least help prevent additional CSS data loss in the meantime and does not affect the data structure at all so shouldn't impact any future changes in relation to style variation data storage.

@SaxonF
Copy link
Contributor

SaxonF commented Dec 11, 2023

Thanks for putting this together @glendaviesnz . Agree there is more thought to be put into style variation management, in particular if colours and fonts are pulled out. There is a proposal here that prompts a user if they want to save their changes to a variation before switching.

I think we could take a similar approach here so instead of a checkbox we just prompt if we detect CSS changes "Do you want to bring your CSS changes over to the new variation?"

@annezazu
Copy link
Contributor

I think we could take a similar approach here so instead of a checkbox we just prompt if we detect CSS changes "Do you want to bring your CSS changes over to the new variation?"

+1 to this.

Copy link

github-actions bot commented Apr 22, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: glendaviesnz <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: SaxonF <[email protected]>
Co-authored-by: annezazu <[email protected]>
Co-authored-by: jameskoster <[email protected]>
Co-authored-by: richtabor <[email protected]>
Co-authored-by: carolinan <[email protected]>
Co-authored-by: hagege <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@jasmussen
Copy link
Contributor

To expand a bit on the note here, there's a long term view that blurs the boundaries between switching between block themes, and saving multiple entities. Theoretically you should be able to batch site logo, title, page content, synced pattern changes and more with switching a theme, and then schedule it to go live at a certain time.

That's a mouthful and we want to carefully work towards that, even just unifying the publishing flow is one first step.

Any small changes that can unstick this in the mean time are fine to do. But just to further the idea of where we can go in the future, here's a slightly fresher sketch of what a block theme activation modal could look like:
block theme modal

Figma.

@jasmussen
Copy link
Contributor

Trying to unstick this, I want to ask a possibly obvious question: why would you ever untoggle "keep custom CSS"? Is there any use case for not keeping it when switching themes and style variations?

If we decided that no, custom CSS is kept until you manually edit or delete it, then we can remove the checkbox and solve the issue with a minimal version of this branch. CC: @jameskoster @annezazu @richtabor

@jameskoster
Copy link
Contributor

Keeping custom css when switching between style variations seems like a decision we can make.

But as for switching themes entirely, that is really hard to say. If your custom css is based on the active theme (maybe using css vars, or adding integration for a plugin) then you probably wouldn't want to bring those styles with you. I have no idea how common that is though.

Given it's relatively easy to manually remove custom css, it may be okay to remove the checkbox, if that will unstick this work?

@jasmussen
Copy link
Contributor

But as for switching themes entirely, that is really hard to say. If your custom css is based on the active theme (maybe using css vars, or adding integration for a plugin) then you probably wouldn't want to bring those styles with you. I have no idea how common that is though.

For switching themes, we should definitely have a checkbox in the theme switching modal. But for style variations, this PR can land with that always kept, IMO.

@richtabor
Copy link
Member

Trying to unstick this, I want to ask a possibly obvious question: why would you ever untoggle "keep custom CSS"? Is there any use case for not keeping it when switching themes and style variations?

I see it like this:

If unedited styles.css from theme.json, it should be discarded if there's incoming styles.css from a variation switch (which already happens).

If the styles.css from theme.json is overwritten by a custom entry from the user, then that value should carry over on variation switch, overwriting the style variation's styles.css.

@annezazu
Copy link
Contributor

Is there any use case for not keeping it when switching themes and style variations?

I could see a case where someone doesn't mind losing the CSS and would untoggle it. With that said, that toggle is still great information to have and, without it right now, it's leading to a form of content loss (where this all started). I think adding a toggle would go a long way or following what Rich shared above.

@jasmussen
Copy link
Contributor

I'm unsure how to move this forward. @glendaviesnz how are your feelings on this for 6.6? Although I don't love extracting this one piece from the flow and adding a toggle, it's also not the end of the world. Can the toggle be contextual so it only shows up if you do have custom CSS? Also CC @scruffian, as I think he had thoughts as well.

@glendaviesnz
Copy link
Contributor Author

sorry @jasmussen I have been out of the loop on this on a different project.

In terms of narrowing the discussion back down to just the question of custom CSS moving between variations on a block theme is the general agreement that we should just make this happen by default, so no toggle option, if the user wants to
stop the custom CSS moving between variations they just have to manually delete it? If that is the case I can see what I can do to modify this PR to do that in time for 6.6

@jasmussen
Copy link
Contributor

Thanks for the update. I'm a fan of that approach. I think @scruffian had some nuance, and CC @annezazu as well.

@annezazu
Copy link
Contributor

I'm a fan of that approach too. If someone is adding CSS, I'd expect them to want to keep it. Of course, some folks might forget they added it in the first place but, if they found the CSS to add, I would expect they would find it once more to remove.

@jasmussen
Copy link
Contributor

Okay, IMO that sounds like a reason to unblock this. @glendaviesnz if you have energy to pick this back up, should we try with no toggle for now?

@glendaviesnz
Copy link
Contributor Author

Something got messed up while trying to rebase this, going to just open a new PR given direction has changed slightly as will be quicker than sorting the rebase.

Closing in favour of #61752

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Design Needs design efforts. [Type] Enhancement A suggestion for improvement.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Custom CSS: Additional CSS is lost after browsing different style variations regardless of whether it's saved
6 participants