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
Publisher opt in into a story landscape fullbleed desktop + mobile experience. #19646
Publisher opt in into a story landscape fullbleed desktop + mobile experience. #19646
Conversation
box-sizing: border-box !important; | ||
min-height: 100vh !important; | ||
padding: 104px 156px 64px !important; | ||
margin: 0 !important; | ||
} | ||
|
||
[desktop] .i-amphtml-story-bookend-inner::before. | ||
[desktop-fullbleed] .i-amphtml-story-bookend-inner::before { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this not needed anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, we actually want that opacity layer, even on desktop :))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI I don't think we can use Array.prototype.includes
, but are supposed to use findIndex
from array.js
and compare it to -1
.
I feel like this is the kind of thing @jridgewell would know for sure.
this.element.getAttribute(Attributes.SUPPORTED_ORIENTATIONS); | ||
|
||
if (!supportedOrientationsAttribute) { | ||
return ['portrait']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a preference for making constants out of 'portrait'
and 'landscape'
, so that we get compiler errors instead of bugs if we mistype them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
PTAL |
8fce743
to
c40a6ea
Compare
Friendly ping |
|
c40a6ea
to
eb94bb8
Compare
…perience. (ampproject#19646) * Renaming UIType.DESKTOP into UIType.DESKTOP_PANELS. * Replacing [desktop] CSS selectors, and supported-orientations opt in. * Detecting the supported-orientations config. * Allow landscape mobile if opted in. * Reviews.
supported-orientations="portrait,landscape"
on the<amp-story>
tag (pending validation)supported-orientations="portrait"
(which is the default), or nosupported-orientations
attribute at alllandscape
optin also allows mobile landscapeUIType.DESKTOP
intoUIType.DESKTOP_PANELS
[desktop]
now means "Viewport size should trigger a desktop UI", which could be PANELS or FULLBLEED depending on the configurationi-amphtml-story-desktop-panels
andi-amphtml-story-desktop-fullbleed
to tweak the CSSamp-story-desktop.css
intoamp-story-desktop-panels.css
Demo
#19270