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

Improve handling of !important transformation specificity #1320

Merged
merged 5 commits into from
Aug 5, 2018

Conversation

hellofromtonya
Copy link
Contributor

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.

Tonya Mork added 5 commits August 5, 2018 10:06
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"
…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 the Bug Something isn't working label Aug 5, 2018
@hellofromtonya hellofromtonya added this to the v1.0 milestone Aug 5, 2018
Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

It works! Great job.

@westonruter westonruter merged commit 0ebc97f into develop Aug 5, 2018
@westonruter westonruter deleted the add/improve-important-specificity branch August 5, 2018 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve handling of !important transformation specificity
2 participants