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

apply root with higher priority #14350

Closed
wants to merge 3 commits into from
Closed

apply root with higher priority #14350

wants to merge 3 commits into from

Conversation

timelsass
Copy link
Contributor

@timelsass timelsass commented Mar 8, 2019

I see this has finally been given attention in PR #13325 I opened an issue and pr for this before gutenberg was added to core, along with asking for review in slack a few times :(. Anyways please check out some of the information from my original PR here: #11957 Also read the original issue which overs more: #11955

The current implementation that was merged in is giving priority to :root the same as html/body element, but :root needs to take precedence over those.

As a way of introducing the same level specificity that :root has over html with how the editor-style wrap is working, I think this would be a better approach. This gives .namespace:root applied as .namespace:not(EDITOR_STYLES). This uses a useless negation selector to increase the specificity, which is documented in the w3 spec. Also see note at bottom of that section for useless negations.

Checklist:
[ x ] My code is tested.
[ x ] My code follows the WordPress code style.
[ x ] My code follows the accessibility standards.
[ x ] My code has proper inline documentation.

@timelsass
Copy link
Contributor Author

timelsass commented Mar 8, 2019

I mainly reopened this given that a solution is being merged, and in testing :root as having certain characteristics over html with inheriting of css variables, I'd like to open it up for discussion to this proposed change so that way variables are inherited correctly when people use html && :root selectors together for inheritance and overriding.

For further reading on the treatment, MDN has a good explanation.

Basic explanation: we are creating a "pseudo document", .namespace, so this behaves as html. :root is the same element as html, except it has 1 higher priority. Using the useless negation, we provide the same level of specificity to .namespace, making it the same element as our "pseudo document", and the same level of specificity.

Basic use case would include defaults for style variables, set like:

 html {
     --bg-color: blue;
 }

Customizer dynamic typography controls override our default stylesheet:

:root {
    --bg-color: orange;
}

Frontend:

.something {
    background-color: var(--bg-color);
}

Inheritance like this would mean the following:

  1. Default background color on theme activation with no customizer background color applied my background is blue.
  2. I change color to orange in the customizer, which creates dynamic css shown above. My background color for .something is now orange because :root overrides html by 1.
  3. In some cases this may be fine for a single set of styles being wrapped in .namespace - only because of the order it's outputting. The order might not be guaranteed for the output of the editor wrap if there's multiple stylesheets and dynamic CSS being added. In which case the style should not only rely on the order, but also level of specificity, just as it normally would.

Current implementation means that the value for .namespace could just contain using a variable, and not inherit properly from the "pseudo document" root :

// Last to output in editor-style wrap
.namespace {
     background-color: var(--bg-color);
}

So this PR seeks to resolve that with output at one point higher specificity:

.namespace:not(EDITOR_STYLES) {
    background-color: orange;
}

Which could be inserted at any point to come before or after.

.namespace {
    background-color: var(--bg-color);
}

Having said all that - there's still a remaining issue of the context of html and body being treated equally when html is the root document. I think the above solution remedies most situations that are common for CSS variables, but it doesn't completely solve the namespacing issues that have cropped up.

In theory - a preliminary loop over the styles could be done and things are sorted out. It would keep reference to all of the IS_ROOT_TAG matches instead of performing the wrap immediately, once the matches are sorted out a namespace is constructed, then the namespace can handle html, body, :root in order of specificity as normally intended, but this would also mean inside specificity could vary for people who make blocks or use styles within .namespace.

Another technique may be just removing :root from the wrap process entirely, so variables are still inherited from the root level, and then apply the wrap as .namespace:not(EDITOR_STYLES) for the remaining elements, and allow html as the "pseudo document root at .namespace. This of course would increase the specificity of everything by 1 point, so there may be some basic changes required, and people who are already upset with the editor-wrap process probably hate :not() selectors too.
It seems like with css variables becoming more commonplace in plugins and themes, they should be namespacing themselves for controlling collisions like anything else in the public namespace, making :root a safe global place to set variables, but not styles. It's a tradeoff either way though.

Additional thought: There's been several people who have worked around this since release, and most solutions involve running some sort of postcss transform, find and replace, or various other things in preprocessing before distribution. It might be a nice place to have a hook so developers can modify the behavior a bit inside of the wrap process. I'm not sure if one exists or not off the top.

Either way - I think this PR provides at least a partial remedy towards the problem, but everyone knows that if we were honest with ourselves, the proper solution is isolation, either via shadowroot or iframe.

@timelsass
Copy link
Contributor Author

closing this since it keeps getting ignored on every release.

@timelsass timelsass closed this Mar 26, 2019
@timelsass timelsass deleted the fix/root-selector branch March 26, 2019 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant