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

CSS selectors with !important not working in next pages with amp-next-page component #6909

Closed
danirueda opened this issue Feb 10, 2022 · 7 comments · Fixed by #6312
Closed
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. CSS
Milestone

Comments

@danirueda
Copy link

Bug Description

Target

I am working on implement AMP infinite scroll experience with <amp-next-page> component.

Context

I am in transitional mode using same post template for AMP and no AMP version. Also I am using Bootstrap 4 for front-end building. I am using its utilities classes to get my expected CSS behaviour like d-flex, d-none, align-items-center... etc.

Problem

I started putting next pages JSON into <amp-next-page> component as documentation says to see what was first look.

OK next pages loaded. But my surprise was some elements in them didn't look like first page, and those elements had Bootstrap utilities classes with values with !important word. For example d-flex.

.d-flex {
    display: flex !important;
}

Shadow DOM and new CSS selector to clean !important

Doing my investigation in Google and into plugin code I found new CSS selector with :root and repeated :not(#_) pseudo classes is made to transform CSS !important behaviour. This is OK for first page.

But for next pages I found each of them is inserted into a shadow DOM and consequently, its styles. But that generated CSS selector described before for CSS rules with !important is not working because the root element stills being <html> tag and shadow DOM is in the middle of the problem.

Expected Behaviour

Expected behaviour should be all CSS rules with !important will work in first and next pages in <amp-next-page> component.

I can control my CSS code to be cleaned of !important but this problem can't happen because a commonly front-end used framework is using !important also.

Screenshots

Let's put as example the social buttons.

First page

image

Next page

image

image

PHP Version

7.4.21

Plugin Version

2.1.4

AMP plugin template mode

Transitional

WordPress Version

5.8.1

Site Health

No response

Gutenberg Version

No response

OS(s) Affected

No response

Browser(s) Affected

No response

Device(s) Affected

No response

Acceptance Criteria

No response

Implementation Brief

No response

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

@danirueda danirueda added the Bug Something isn't working label Feb 10, 2022
@westonruter
Copy link
Member

@danirueda Thanks for reporting this issue. I think this may be resolved by #6312 which takes an alternative approach to hacking the specificity of CSS selectors.

So given input CSS of:

.d-flex {
    display: flex !important;
}

Before this would get processed as:

:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .d-flex {
	display: flex;
}

And after it would instead be:

.d-flex:not(#_#_#_#_#_#_#_#_#_) {
	display: flex;
}

As you can see, the new proposed approach does not use :root.

Could you test that PR to see if it fixes the issue for you?

@danirueda
Copy link
Author

@westonruter Yes it does!

I've changed selector in next page and it works. That PR is the solution.

When will those changes be merged into plugin?

Thank you!

@westonruter
Copy link
Member

Excellent. Then I'll milestone this for the next minor release (v2.2.2). This would be likely due out in the next couple weeks.

@danirueda
Copy link
Author

danirueda commented Feb 12, 2022

@westonruter Thank you!

@westonruter
Copy link
Member

Reopening since we'll close via the PR being merged.

@westonruter
Copy link
Member

@danirueda You can use the latest 2.2.x build to utilize the fix now.

@danirueda
Copy link
Author

@westonruter Ok!

Thank you!

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. CSS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants