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

πŸ›πŸ–οΈ Preventing publishers CSS from overriding stories system layer and bookend. #13270

Merged
merged 2 commits into from
Feb 6, 2018

Conversation

gmajoulet
Copy link
Contributor

Before we are able to render the system layer and bookend into shadow DOM, here is a temporary fix.

Related-to #12917

@jridgewell jridgewell changed the title πŸ› πŸ–οΈ Preventing publishers CSS from overriding stories system layer and bookend. πŸ›πŸ–οΈ Preventing publishers CSS from overriding stories system layer and bookend. Feb 5, 2018
Copy link
Contributor

@newmuis newmuis left a comment

Choose a reason for hiding this comment

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

I think we'll also want this class on the AmpStoryHint container, and the LANDSCAPE_ORIENTATION_WARNING and UNSUPPORTED_BROWSER_WARNING shown in AmpStory.

@@ -123,6 +123,20 @@ amp-story[standalone]:fullscreen {
background-color: rgba(0, 0, 0, 0.2) !important;
}

.i-amphtml-story-system-reset,
.i-amphtml-story-system-reset * {
border: none !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you try all? Do we know if that's a reasonable approach or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline, all wouldn't let us override properties.

Copy link
Contributor Author

@gmajoulet gmajoulet left a comment

Choose a reason for hiding this comment

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

Not super happy with the hacky overrides, but that should do it until we render the components in shadow dom.

@@ -123,6 +123,20 @@ amp-story[standalone]:fullscreen {
background-color: rgba(0, 0, 0, 0.2) !important;
}

.i-amphtml-story-system-reset,
.i-amphtml-story-system-reset * {
border: none !important;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline, all wouldn't let us override properties.

@newmuis newmuis merged commit d8df163 into ampproject:master Feb 6, 2018
protonate pushed a commit to protonate/amphtml that referenced this pull request Feb 26, 2018
…d bookend. (ampproject#13270)

* Preventing CSS overrides.

* Preventing CSS overrides for the navigation hint, landscape orientation and unsupported browser.
RanAbram pushed a commit to RanAbram/amphtml that referenced this pull request Mar 12, 2018
…d bookend. (ampproject#13270)

* Preventing CSS overrides.

* Preventing CSS overrides for the navigation hint, landscape orientation and unsupported browser.
protonate pushed a commit to protonate/amphtml that referenced this pull request Mar 15, 2018
…d bookend. (ampproject#13270)

* Preventing CSS overrides.

* Preventing CSS overrides for the navigation hint, landscape orientation and unsupported browser.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants