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

Improve handling of !important transformation specificity #1320

Merged
merged 5 commits into from Aug 5, 2018

Conversation

Projects
None yet
2 participants
@hellofromtonya
Collaborator

hellofromtonya commented Aug 5, 2018

This PR improves the !important selector specificity to ensure that it takes precedence over inline styles.

The inline selector's specificity is calculated using a literal value of 5. Let's call that 5 at inline specificity multiplier. The multiplier is used to generate the number of :not(#_) placeholders.

As @westonruter notes in the ticket #1073, with the following HTML:

<header id="header" style="display: none;>This is the header!</header>

It transforms the inline style to be:

:root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-224b51a {
    display: none;
}

However, what if there is a CSS ruleset that has display: block !important; as a declaration? This PR fixes that edge case to ensure the !important declaration takes precedence over the inline style.

How? It uses the same inline specificity multiplier and then adds 1 to it. The thought process is this:

  1. Inline selectors have 5 :not(#_)
  2. !important have 6 :not(#_)

Before

Here is the HTML:

<header id="header" class="amp-wp-224b51a">This is the header!</header>

Here is the CSS:

#header {
	display: block !important;
	width: 100%;
	background-color: #cc0000;
	padding: 20px;
	color: #fff;
}
:root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-224b51a {
    display: none;
}

issue-1073-before

After

Here is the HTML:

<header id="header" class="amp-wp-224b51a">This is the header!</header>

Here is the CSS:

#header {
	width: 100%;
	background-color: #cc0000;
	padding: 20px;
	color: #fff;
}
:root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-224b51a {
    display: none;
}
:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) #header {
    display: block;
}

issue-1073-after

Closes #1073.

hellofromtonya added some commits Aug 5, 2018

Add base inline specificity multiplier + 1 to !important selector.
A multiplier is used to generate the number of `:not(#_)` for the inline selector.  In order for the !important ruleset to take precedence, its selector's specificity must be 1 more than the inline style's selector.

This commit:

1. adds 1 to the inline specificity multiplier.
2. fixes all the !important tests.
3. adds a test to validate "important_takes_precedence_over_inline"
Move specificity multiplier literal to a constant to a single configu…
…ration property.

This literal integer is used in more than one place in the class.  Moving it to a constant on the class provides a single configuration point for all uses.

@hellofromtonya hellofromtonya added this to the v1.0 milestone Aug 5, 2018

@hellofromtonya hellofromtonya requested a review from westonruter Aug 5, 2018

@hellofromtonya hellofromtonya added the css label Aug 5, 2018

@westonruter

It works! Great job.

@westonruter westonruter merged commit 0ebc97f into develop Aug 5, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@westonruter westonruter deleted the add/improve-important-specificity branch Aug 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment