Style unresolved elements#12077
Conversation
|
|
||
| .i-amphtml-layout-nodisplay { | ||
| .i-amphtml-layout-nodisplay, | ||
| [layout=nodisplay]:not(.i-amphtml-layout-nodisplay) |
There was a problem hiding this comment.
do you need the :not(.i-amphtml-layout-nodisplay) here? , i don't think it makes a difference
There was a problem hiding this comment.
Once it is resolved, the selector will fail to apply, so everything keeps the same specificity as before.
css/amp.css
Outdated
|
|
||
| .i-amphtml-layout-fixed { | ||
| .i-amphtml-layout-fixed, | ||
| /*, [width][height]:not([layout=fixed]) */ |
| .i-amphtml-layout-fixed { | ||
| .i-amphtml-layout-fixed, | ||
| /*, [width][height]:not([layout=fixed]) */ | ||
| [layout=fixed][width][height]:not(.i-amphtml-layout-fixed) |
There was a problem hiding this comment.
This is a breaking change (almost in all the places), since we are increasing the specificity here. It is probably going to break a lot of pages.
There was a problem hiding this comment.
Once it is resolved, the selector will fail to apply, so everything keeps the same specificity as before.
|
/to @dvoytenko |
dvoytenko
left a comment
There was a problem hiding this comment.
Looks good. LGTM with a couple of comments. PTAL and merge when ready.
css/amp.css
Outdated
|
|
||
| .i-amphtml-notbuilt:not(.i-amphtml-layout-container) > * { | ||
| .i-amphtml-notbuilt:not(.i-amphtml-layout-container) > * , | ||
| [layout]:not(.i-amphtml-element):not(.i-amphtml-layout-container) > *, |
There was a problem hiding this comment.
Perhaps [layout]:not([layout="container"]):{the rest of what you have}?
| } | ||
|
|
||
| .i-amphtml-notbuilt:not(.i-amphtml-layout-container) > * { | ||
| .i-amphtml-notbuilt:not(.i-amphtml-layout-container) > * , |
There was a problem hiding this comment.
Could you please double-check that the placeholder declaration below (.i-amphtml-element > [placeholder], etc) has higher specificity than this?
| .i-amphtml-layout-responsive { | ||
| .i-amphtml-layout-responsive, | ||
| [layout=responsive][width][height]:not(.i-amphtml-layout-responsive), | ||
| [width][height][sizes]:not(.i-amphtml-layout-responsive) |
There was a problem hiding this comment.
Consider additional, [height][width=auto] - it implies layout=responsive.
fb43db7 to
e9b155c
Compare
e9b155c to
dd003bd
Compare
* Target unresolved amp elements with expected layout * Apply layouts to unresolved elements * Ensure placeholder are shown * Remove comments * Update layout unresolved selectors
* Target unresolved amp elements with expected layout * Apply layouts to unresolved elements * Ensure placeholder are shown * Remove comments * Update layout unresolved selectors
This applies initial layouts to still unresolved AMP elements.
The changes to
layout=nodisplay,layout=responsive,layout=fixed-height,layout=container,layout=fill, andlayout=flex-itemjust apply while the element is unresolved (doesn't have a.i-amphtml-layout-Xclass). Once it is resolved, the class will be added and selector will fail to apply, so everything keeps the same specificity as before.The other changes to unresolved elements with any layout (
[layout]:not(.i-amphtml-element)) prevent the children of unresolved elements from being displayed, since the parent needs to resolve before we can figure out the measurements for the children. This fixes #11929./cc @brandondiamond