Skip to content

Conversation

cornedor
Copy link
Contributor

Description

The :root selector caused invalid selectors because the namespace was added before it. This PR handles :root the same way as html and body are handled. This is useful for defined css variables. For example:

:root {
	--color-primary: #FF0;
}

will now become:

.editor-styles-wrapper {
	--color-primary: #FF0;
}

How has this been tested?

I've build a test case and ran the tests

Screenshots

Types of changes

This is a bug fix. Without this PR :root will be ignored by the browser.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@gziolo gziolo requested a review from youknowriad January 22, 2019 08:56
@gziolo gziolo added the [Feature] Custom Editor Styles Functionality for adding custom editor styles label Jan 22, 2019
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me. Thoughts @jasmussen

@youknowriad youknowriad added this to the 5.0 (Gutenberg) milestone Jan 22, 2019
@jasmussen
Copy link
Contributor

Seems reasonable to me as well. I've very rarely used :root, but the use case seems okay to me.

Can we think of any situations where a web developer might not want :root to be rewritten?

@cornedor
Copy link
Contributor Author

:root might be a way to break out of the namespace, instead of replacing :root with with .editor-styles-wrapper it should not be prepended, but I think personally that it could become confusing. .editor-styles-wrapper :root is an invalid selector and does nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Custom Editor Styles Functionality for adding custom editor styles
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants