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.

Sign up for GitHub

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

Restore html scope in reset.scss selectors #61959

Closed
wants to merge 2 commits into from
Closed

Restore html scope in reset.scss selectors #61959

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a208ef
Restore `html` scope in reset.scss selectors
sabernhardt Feb 17, 2024
15fa9e1
Merge branch 'WordPress:trunk' into reset-html-selector
sabernhardt Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/block-library/src/reset.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

// We use :where to keep specificity minimal.
// https://css-tricks.com/almanac/selectors/w/where/
:where(.editor-styles-wrapper) {
html :where(.editor-styles-wrapper) {
Copy link
Member

Choose a reason for hiding this comment

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

From recent other PR's such as #61638 I think :root would be a better selector here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used html so this would be exactly as safe as it was in WordPress 6.3.

The only selector that I know needs an increased specificity is :where(.editor-styles-wrapper), so it can override the admin styles on body, and 0-0-1 would be enough for that element.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given this is a reset stylesheet, I'm inclined towards the 0-0-1 specificity as well.

Perhaps we could further isolate the change here by extracting only the rules needed to override the common.css body styles under the restored selector. For example:

html :where(.editor-styles-wrapper) {
	/**
	* The following styles revert to the browser defaults overriding the WPAdmin styles.
	* This is only needed while the block editor is not being loaded in an iframe.
	*/
	font-family: serif; // unfortunately initial doesn't work for font-family.
	font-size: initial;
	line-height: initial;
	color: initial;
	// Many themes with white backgrounds load editor styles but fail to also provide
	// an explicit white background color, assuming a white editing canvas.
	// So to match browser defaults, we provide a white default here as well.
	background: #fff;
}

The rest could remain under :where(.editor-styles-wrapper).

There is also another PR around the editor styles wrapper that might be worth taking a look at to see how these changes might be affected.

Copy link
Contributor Author

@sabernhardt sabernhardt May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If you want to isolate the one element, you could use the html & method I mentioned in #57176 (comment)

However, a PR like that is more complex than I should try to create within the browser, so someone else should make it.

Plus, the comment about not being in an iframe is inaccurate (with classic themes), so that sentence could be removed too.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
html :where(.editor-styles-wrapper) {
:root :where(.editor-styles-wrapper) {

/**
* The following styles revert to the browser defaults overriding the WPAdmin styles.
* This is only needed while the block editor is not being loaded in an iframe.
Expand Down
Loading