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

[WIP] Styles: Remove the ! important tag from user predefined style selections #40121

Closed
wants to merge 6 commits into from

Conversation

glendaviesnz
Copy link
Contributor

@glendaviesnz glendaviesnz commented Apr 7, 2022

What?

Removes the ! important that is being applied to user applied style presets. This was added here in order to fix an issue with theme style settings overriding user selected settings.

Why?

The application of the ! important across all the predefined classes has been a breaking change across a range of themes (#38252, #34575, #37590) as it overrides the valid use of some of these presets, and in some cases in a way which can't be worked around at the theme level, particularly with responsive typography.

While it is important to ensure that the styles applied by the user in the editor take priority over global styles, and theme styles, I think it is worth reconsidering the use of ! important to achieve this for the following reasons:

  • As mentioned already it is a breaking change for many Classic themes, many of which may no longer be maintained so can't be updated to account for this change, or can't be fixed due to lack of support for responsive typography in theme.json at this point
  • While it works in many cases to achieve the desired aim, it is not a guarantee that the user selected styles will be applied, eg. if a theme adds a higher specificity selector with ! important applied it will still override the user selection
  • The initial case for which it was introduced no longer needs the ! important in order to work due to the removal of a wrapper around the Post Title block. While it will still be an issue with any blocks applying these styles with a selector of specificity higher than (0,1,0) this can be solved on a case by case basis with the use of :where() or :not()

While this change may make it easier for theme styles to override, either accidentally or intentionally, user applied styles, clear documentation for theme authors on how to prevent this with the use of lower specificity selectors may be a better long term solution than trying to enforce it with ! important.

How?

  • Removes the automatic appending of !important from the preset classes
  • Reduces the specificity of the .wp-block-table > table experimental selector
  • Reduces specificity of theme.json element selectors

N.B I have done a quick test across all the core blocks using TT2 theme and the table block was the only one I found that didn't have users settings applied as expected with the ! important removed, but with reducing the specificity of the theme.json element selectors there is a risk that in some cases block defaults could take priority over theme styles so an detailed audit of all blocks needs to be undertaken to see if this causes any issues in this regard.

Testing Instructions

  • Add a table and set the background and text color in global styles
  • Add a post with two tables, and select user defined colors on one, and make sure both display as expected in frontend
  • Via theme.json target the post title for color styles by adding:
"core/post-title": {
    "color": {
        "text": "green",
        "background": "yellow"
    }
}
  • Go to the editor, add three post title blocks:

    • Select text & background preset colors (not custom) for the first.
    • Select text & background custom colors (not preset) for the second.
    • Leave the third untouched.
  • Publish the post and go to the front end.

  • Add some theme.json element settings:

// under the styles.elements section
			"h2": {
				"color": {
					"text": "purple",
					"background": "pink"
				},
				"typography": {
					"fontSize": "var(--wp--preset--font-size--extra-large)"
				}
			}

//under the styles.blocks section
			"core/columns": {
				"elements": {
					"h2": {
						"color": {
							"text": "red",
							"background": "black"
						},
						"typography": {
							"fontSize": "var(--wp--preset--font-size--small)"
						}
					}
				}
			}
  • Add an H2 heading block out of a column and within a column block and make sure the above colors apply as expected.
  • Add additional H2 heading in and out of column block and select predefined text/background colors and font size and check that they appear as expected

The expected behavior would be that the user choices are respected, both in the editor and front-end.

…duce specificity of table element selector to prevent it overriding user applied styles
@glendaviesnz glendaviesnz self-assigned this Apr 7, 2022
@glendaviesnz glendaviesnz added the [Status] In Progress Tracking issues with work in progress label Apr 7, 2022
@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Apr 7, 2022

FYI - this is very much still a work in progress - quite a bit of testing is needed to check that reducing the specificity of the theme.json selectors isn't causing block defaults to be incorrectly applied.

Also, this is only in the 5.9 compat code for ease of quick setup and test - if this approach moves forward it would need to be migrated to 6.0+ compat code

@glendaviesnz
Copy link
Contributor Author

So initial testing indicates that the :where would also need to be applied to the theme.json block-element and element selectors and to any presets defined at the block style.scss level for this approach to work.

@markhowellsmead
Copy link

See also #40159.

@priethor priethor added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Apr 7, 2022
@github-actions
Copy link

github-actions bot commented Apr 7, 2022

Size Change: +78 B (0%)

Total Size: 1.22 MB

Filename Size Change
build/block-editor/index.min.js 149 kB +365 B (0%)
build/block-editor/style-rtl.css 15.5 kB +66 B (0%)
build/block-editor/style.css 15.5 kB +76 B (0%)
build/block-library/blocks/button/style-rtl.css 564 B +4 B (+1%)
build/block-library/blocks/button/style.css 564 B +4 B (+1%)
build/block-library/blocks/code/theme-rtl.css 120 B -4 B (-3%)
build/block-library/blocks/code/theme.css 120 B -4 B (-3%)
build/block-library/blocks/pullquote/theme-rtl.css 179 B +12 B (+7%) 🔍
build/block-library/blocks/pullquote/theme.css 179 B +12 B (+7%) 🔍
build/block-library/blocks/search/style-rtl.css 407 B +10 B (+3%)
build/block-library/blocks/search/style.css 408 B +10 B (+3%)
build/block-library/blocks/separator/style-rtl.css 240 B +7 B (+3%)
build/block-library/blocks/separator/style.css 240 B +7 B (+3%)
build/block-library/index.min.js 174 kB +255 B (0%)
build/block-library/style-rtl.css 11.3 kB +26 B (0%)
build/block-library/style.css 11.3 kB +26 B (0%)
build/block-library/theme-rtl.css 692 B +3 B (0%)
build/block-library/theme.css 696 B +2 B (0%)
build/edit-post/index.min.js 29.6 kB -87 B (0%)
build/edit-post/style-rtl.css 7.06 kB -39 B (-1%)
build/edit-post/style.css 7.06 kB -40 B (-1%)
build/editor/index.min.js 38.4 kB -308 B (-1%)
build/editor/style-rtl.css 3.71 kB -160 B (-4%)
build/editor/style.css 3.71 kB -165 B (-4%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 993 B
build/admin-manifest/index.min.js 1.24 kB
build/annotations/index.min.js 2.77 kB
build/api-fetch/index.min.js 2.27 kB
build/autop/index.min.js 2.15 kB
build/blob/index.min.js 487 B
build/block-directory/index.min.js 6.49 kB
build/block-directory/style-rtl.css 1.01 kB
build/block-directory/style.css 1.01 kB
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 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 65 B
build/block-library/blocks/archives/style.css 65 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 111 B
build/block-library/blocks/audio/style.css 111 B
build/block-library/blocks/audio/theme-rtl.css 125 B
build/block-library/blocks/audio/theme.css 125 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 59 B
build/block-library/blocks/avatar/style.css 59 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 445 B
build/block-library/blocks/button/editor.css 445 B
build/block-library/blocks/buttons/editor-rtl.css 292 B
build/block-library/blocks/buttons/editor.css 292 B
build/block-library/blocks/buttons/style-rtl.css 275 B
build/block-library/blocks/buttons/style.css 275 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 103 B
build/block-library/blocks/code/style.css 103 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-template/style-rtl.css 127 B
build/block-library/blocks/comment-template/style.css 127 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-query-loop/editor-rtl.css 95 B
build/block-library/blocks/comments-query-loop/editor.css 95 B
build/block-library/blocks/cover/editor-rtl.css 546 B
build/block-library/blocks/cover/editor.css 547 B
build/block-library/blocks/cover/style-rtl.css 1.56 kB
build/block-library/blocks/cover/style.css 1.56 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 417 B
build/block-library/blocks/embed/style.css 417 B
build/block-library/blocks/embed/theme-rtl.css 124 B
build/block-library/blocks/embed/theme.css 124 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 255 B
build/block-library/blocks/file/style.css 255 B
build/block-library/blocks/file/view.min.js 353 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 961 B
build/block-library/blocks/gallery/editor.css 964 B
build/block-library/blocks/gallery/style-rtl.css 1.51 kB
build/block-library/blocks/gallery/style.css 1.51 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 333 B
build/block-library/blocks/group/editor.css 333 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 332 B
build/block-library/blocks/html/editor.css 333 B
build/block-library/blocks/image/editor-rtl.css 731 B
build/block-library/blocks/image/editor.css 730 B
build/block-library/blocks/image/style-rtl.css 529 B
build/block-library/blocks/image/style.css 535 B
build/block-library/blocks/image/theme-rtl.css 124 B
build/block-library/blocks/image/theme.css 124 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 199 B
build/block-library/blocks/latest-posts/editor.css 198 B
build/block-library/blocks/latest-posts/style-rtl.css 447 B
build/block-library/blocks/latest-posts/style.css 446 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 493 B
build/block-library/blocks/media-text/style.css 490 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 708 B
build/block-library/blocks/navigation-link/editor.css 706 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 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation-submenu/view.min.js 375 B
build/block-library/blocks/navigation/editor-rtl.css 2.03 kB
build/block-library/blocks/navigation/editor.css 2.04 kB
build/block-library/blocks/navigation/style-rtl.css 1.93 kB
build/block-library/blocks/navigation/style.css 1.92 kB
build/block-library/blocks/navigation/view.min.js 2.85 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 363 B
build/block-library/blocks/page-list/editor.css 363 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 260 B
build/block-library/blocks/paragraph/style.css 260 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/style-rtl.css 446 B
build/block-library/blocks/post-comments-form/style.css 446 B
build/block-library/blocks/post-comments/style-rtl.css 521 B
build/block-library/blocks/post-comments/style.css 521 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 721 B
build/block-library/blocks/post-featured-image/editor.css 721 B
build/block-library/blocks/post-featured-image/style-rtl.css 153 B
build/block-library/blocks/post-featured-image/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 323 B
build/block-library/blocks/post-template/style.css 323 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 80 B
build/block-library/blocks/post-title/style.css 80 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 370 B
build/block-library/blocks/pullquote/style.css 370 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 234 B
build/block-library/blocks/query-pagination/style.css 231 B
build/block-library/blocks/query/editor-rtl.css 131 B
build/block-library/blocks/query/editor.css 132 B
build/block-library/blocks/quote/style-rtl.css 213 B
build/block-library/blocks/quote/style.css 213 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/theme-rtl.css 64 B
build/block-library/blocks/search/theme.css 64 B
build/block-library/blocks/separator/editor-rtl.css 140 B
build/block-library/blocks/separator/editor.css 140 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 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 759 B
build/block-library/blocks/site-logo/editor.css 759 B
build/block-library/blocks/site-logo/style-rtl.css 181 B
build/block-library/blocks/site-logo/style.css 181 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 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 177 B
build/block-library/blocks/social-link/editor.css 177 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.37 kB
build/block-library/blocks/social-links/style.css 1.36 kB
build/block-library/blocks/spacer/editor-rtl.css 332 B
build/block-library/blocks/spacer/editor.css 332 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 471 B
build/block-library/blocks/table/editor.css 472 B
build/block-library/blocks/table/style-rtl.css 481 B
build/block-library/blocks/table/style.css 481 B
build/block-library/blocks/table/theme-rtl.css 188 B
build/block-library/blocks/table/theme.css 188 B
build/block-library/blocks/tag-cloud/style-rtl.css 226 B
build/block-library/blocks/tag-cloud/style.css 227 B
build/block-library/blocks/template-part/editor-rtl.css 149 B
build/block-library/blocks/template-part/editor.css 149 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 571 B
build/block-library/blocks/video/editor.css 572 B
build/block-library/blocks/video/style-rtl.css 173 B
build/block-library/blocks/video/style.css 173 B
build/block-library/blocks/video/theme-rtl.css 124 B
build/block-library/blocks/video/theme.css 124 B
build/block-library/common-rtl.css 934 B
build/block-library/common.css 932 B
build/block-library/editor-rtl.css 10.1 kB
build/block-library/editor.css 10.1 kB
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.83 kB
build/blocks/index.min.js 46.9 kB
build/components/index.min.js 223 kB
build/components/style-rtl.css 14.9 kB
build/components/style.css 14.9 kB
build/compose/index.min.js 11.2 kB
build/core-data/index.min.js 14.5 kB
build/customize-widgets/index.min.js 11 kB
build/customize-widgets/style-rtl.css 1.39 kB
build/customize-widgets/style.css 1.39 kB
build/data-controls/index.min.js 663 B
build/data/index.min.js 8.64 kB
build/date/index.min.js 32 kB
build/deprecated/index.min.js 518 B
build/dom-ready/index.min.js 336 B
build/dom/index.min.js 4.58 kB
build/edit-navigation/index.min.js 15.8 kB
build/edit-navigation/style-rtl.css 4.04 kB
build/edit-navigation/style.css 4.05 kB
build/edit-post/classic-rtl.css 546 B
build/edit-post/classic.css 547 B
build/edit-site/index.min.js 46.8 kB
build/edit-site/style-rtl.css 7.77 kB
build/edit-site/style.css 7.75 kB
build/edit-widgets/index.min.js 16.3 kB
build/edit-widgets/style-rtl.css 4.4 kB
build/edit-widgets/style.css 4.39 kB
build/element/index.min.js 4.29 kB
build/escape-html/index.min.js 548 B
build/format-library/index.min.js 6.62 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.66 kB
build/html-entities/index.min.js 454 B
build/i18n/index.min.js 3.79 kB
build/is-shallow-equal/index.min.js 535 B
build/keyboard-shortcuts/index.min.js 1.83 kB
build/keycodes/index.min.js 1.41 kB
build/list-reusable-blocks/index.min.js 1.75 kB
build/list-reusable-blocks/style-rtl.css 838 B
build/list-reusable-blocks/style.css 838 B
build/media-utils/index.min.js 2.94 kB
build/notices/index.min.js 957 B
build/nux/index.min.js 2.12 kB
build/nux/style-rtl.css 751 B
build/nux/style.css 749 B
build/plugins/index.min.js 1.98 kB
build/preferences/index.min.js 1.2 kB
build/primitives/index.min.js 949 B
build/priority-queue/index.min.js 611 B
build/react-i18n/index.min.js 704 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.69 kB
build/reusable-blocks/index.min.js 2.24 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 11.2 kB
build/server-side-render/index.min.js 1.61 kB
build/shortcode/index.min.js 1.52 kB
build/token-list/index.min.js 668 B
build/url/index.min.js 1.99 kB
build/vendors/react-dom.min.js 38.5 kB
build/vendors/react.min.js 4.34 kB
build/viewport/index.min.js 1.08 kB
build/warning/index.min.js 280 B
build/widgets/index.min.js 7.21 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.07 kB

compressed-size-action

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Apr 7, 2022

FYI, due to the questions about the support of :where() in older non-upgradable browsers I also looked at using :not(), but it is not possible to use :not() to fix this issue as there are multiple classes that need to be excluded, eg. :not([class*="-color"]):not([class*="-font-size"]), and the problem being that if these are applied to the theme.json elements and block elements, if a block has a user-defined color preset applied then any theme.json element font settings would fail to be applied, and vice versa.

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Apr 8, 2022

Just an additional note for clarification. This approach would by no means be a silver bullet solution to this problem, and has the following limitations:

  • Questions have been raised about the use of :where()
  • There could be unexpected side effects if CSS somewhere in core/themes/plugins is expecting some of these changed selectors to still have a specificity > 0
  • There could be issues with blocks that skip serialization and manually apply classes on wrappers other than root - we should at least do an audit of any current blocks that are doing this
  • It would also require ongoing vigilance when adding any new CSS selectors to make sure their specificity wasn't overriding user applied defaults
  • It is easier for themes/plugins to override user applied presets, this is probably more at the unintentional level so would be a matter of education about how to avoid it. If they want to deliberately override user presets this is already reasonably easy with likes of .wp-block-post-title.has-default-color { color: purple ! important; }

If this approach is used it probably should be seen as a temporary solution until the application of styles can be handled in a much more fine-tuned way server-side by the style engine.

@ramonjd
Copy link
Member

ramonjd commented Apr 11, 2022

I've been testing this in twentytwentytwo and empty theme and it works as described: my presets are applied to blocks and elements in theme.json, but are overwritten by global styles > user defined styles.

Personally I like your approach. Removing !important will pay off in the future I think, especially if we include sub-theme.jsons (see Style Partials in this issue), which will contain their own styles and inject their own specificity requirements.

there could be unexpected side effects if CSS somewhere in core/themes/plugins is expecting some of these changed selectors to still have a specificity > 0

This is true. My initial thought is that the benefits of having a predictable cascading hierarchy, without the brute force approach of ! important will allow for neater style rule composition.

There could be issues with blocks that skip serialization and manually apply classes on wrappers other than root - we should at least do an audit of any current blocks that are doing this

I think search block might be an example. It adds color classes to the button

If this approach is used it probably should be seen as a temporary solution until the application of styles can be handled in a much more fine-tuned way server-side by the style engine.

Specificity is a good thing to keep in mind for the style engine, though it's going to be interesting to work out how to instruct it to output the right classes in the right scenario.

It's still up in the air, but I imagine that the style engine, at least initially, would blindly follow input instructions, e.g., we'd have to make sure the arguments we pass to the style engine correspond with the specificity we want for each level of classnames (global, block etc).

Having things centralized however will definitely make the work easier I hope! 🤞

It would also require vigilance when adding any new CSS selectors to make sure their specificity wasn't overriding user applied defaults

👍

@glendaviesnz glendaviesnz marked this pull request as ready for review April 11, 2022 04:02
@glendaviesnz glendaviesnz removed the [Status] In Progress Tracking issues with work in progress label Apr 11, 2022
@glendaviesnz
Copy link
Contributor Author

@oandregal, @youknowriad - This PR put in place what you suggested here André, ie. to go through and reduce global style and block style specificity in order to allow the ! important to be removed. It works, but there are a number of limitations to this approach noted here.

Do you think it is worth exploring this further as a temporary solution to the issues caused by the addition of the ! important to user presets while we wait for the style engine development to hopefully provide more flexible outputting of these styles on the server end?

@glendaviesnz
Copy link
Contributor Author

@markhowellsmead, @cbirdsong I would be interested to know if you had any thoughts on how best to ensure that user preset classes that a user applies in the editor can be given priority over any styles set in theme.json or via global styles in the site editor if the ! important is removed?

This PR is experimenting with :where but as noted already there are concerns about the use of this. Also, :not isn't an option at all for this one, and as you have noted elsewhere Mark this can also introduce other specificity issues.

As an example of the current potential hierarchy of block styles, an h2 block could have the following cascade of color settings which may need to be applied, with the one at the top needing to have the highest specificity:

user applied preset in editor
    element setting in theme.json styles.blocks section
        element setting in theme.json styles.elements section
            defaults from block style.scss

Cory, you suggested the use of inherit, but with many blocks, including the post title one in the example that has been used in this discussion now, there is no wrapper to which the styles are applied, eg. post title is now <h2 class="wp-block-post-title has-red-color> not <div class="wp-block-post-title has-red-color><h2>, so the inherit model doesn't work in this scenario, that I can see, but I might be missing something.

As mentioned in the above comments the ideal situation is server-side rendering of all these styles in a way that can be smarter about when and in which order they are applied, but we are still a way off from being able to implement that, so if you can think of any alternative solutions in the meantime let me know.

@cbirdsong
Copy link

post title is now <h2 class="wp-block-post-title has-red-color"> not <div class="wp-block-post-title has-red-color><h2>

That makes things easier, then! The cleanest solution would be making sure these styles are added to the <head> in the inverse of the order you list, which would then let the cascade take over. This is how Bootstrap and ITCSS organize similar utility classes:

Poking at a stock install, seems like the easiest solution might be just moving the <style id="global-styles-inline-css"> block to the end of the <head>, though I'm not sure how easily that can be done hooking into wp_head or wp_enqueue_style or whatever other method is in use?

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Apr 11, 2022

That makes things easier, then! The cleanest solution would be making sure these styles are added to the <head> in the inverse of the order you list

Thanks for the response @cbirdsong - sorry, I left out the critical detail in my summary that some of the styles have a higher specificity further up the cascade.

The css is already loaded in the correct order which is why adding the :where() is working - the issue is that some styles higher up the cascade have more specific selectors than the 0,1,0 of the user preset classes, eg. the table block uses .wp-block-table > table to apply the default styles, but the .-color classes are applied to the table element, so currently inherit option won't work here either.

I will take another look and see if there is some way to just handle the exceptions like the table block, rather than a blanket application of :where which in most cases isn't needed as the cascade is handling it already as expected.

@glendaviesnz
Copy link
Contributor Author

There is another approach here which avoids the need for :where by appending the preset classes to any custom block selectors that will push the specificity over the (0,1,0) of the individual preset class. The disadvantage of this approach is that it adds a lot of additional CSS (approx 50KB).

@cbirdsong
Copy link

Yeah, I think it's just going to be a lot of figuring out how to best handle each one of these edge cases. In the case of the table block, that inconsistency with other blocks is very odd1. Is the issue you're seeing is-style-stripes?

Footnotes

  1. Though it should help smooth the future process of the table block ditching that figure wrapper, which is very weird HTML!

@glendaviesnz
Copy link
Contributor Author

Is the issue you're seeing is-style-stripes?

The problem is the fact that if a color is assigned via global styles it is added to the blocks custom selector of .wp-block-table > table, and so this overrides the user preset which is just a single class selector.

The theme.json styles.elements and styles.blocks.elements cause the same issue, eg. an h2 element setting in a column block in theme.json gets added via a .wp-block-columns h2 selector.

Will keep thinking about options for getting around this that don't involve the extra 50kb of CSS that this solution creates.

Though it should help smooth the future process of the table block ditching that figure wrapper, which is very weird HTML!

oh, good, it wasn't just me that thought this was a little strange 😄

@cbirdsong
Copy link

Have you considered custom properties? The way I handled this in my non-theme.json theme was setting a custom property in the .has-X-background-color class instead of directly setting the background color:

.has-magenta-background-color { 
  --background-color: magenta;
}

Then elsewhere that custom property can be used however it's needed for that particular element. The table example would work like this:

.wp-block-table {
  background-color: var(--background-color);

  &.is-style-stripes {
    $default-color: $gray-100;

    background-color: transparent;

    tbody tr:nth-child(odd) {
      background-color: $default-color;
      background-color: var(--background-color, #{$default-color});
    }
  }
}

This method could also flatten out the selector generated by custom colors applied in global styles. However, I'm not sure about this:

The theme.json styles.elements and styles.blocks.elements cause the same issue, eg. an h2 element setting in a column block in theme.json gets added via a .wp-block-columns h2 selector.

Can you tell me how to get the editor into this state? I don't see this functionality in the site editor.

@glendaviesnz
Copy link
Contributor Author

Have you considered custom properties?

Thanks for that example, I will have a think about what might be possible with this approach.

Can you tell me how to get the editor into this state? I don't see this functionality in the site editor.

I don't think it is possible in the editor at the moment, you would need to have a theme.json and add the following under the styles.blocks section and then add a heading in a column block in the editor to see the effect:

"core/columns": {
	"elements": {
		"h2": {
			"color": {
				"text": "red",
				"background": "black"
			},
		}
	}
}

@glendaviesnz
Copy link
Contributor Author

Have you considered custom properties?

@cbirdsong I have a draft PR up here using this concept, seems to work at a basic level, but will do more testing over the next few days and see if there are any gotchas. Thanks again for the suggestion.

@glendaviesnz
Copy link
Contributor Author

Closing in favour of #40335

@oandregal oandregal deleted the try/removing-important branch April 20, 2022 07:27
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants