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

Opt in into the desktop full bleed experience. #19271

Merged
merged 2 commits into from Nov 14, 2018

Conversation

gmajoulet
Copy link
Contributor

Providing a way to opt in into the full bleed desktop experience, through a <amp-story desktop-mode="fullbleed"> parameter.

This PR doesn't change anything to the current MOBILE and DESKTOP modes, nor to the disable-amp-story-desktop experiment.

Demo

#19270

Copy link
Contributor

@Enriqe Enriqe left a comment

Choose a reason for hiding this comment

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

For later: should we show the education overlay when the user scrolls? Seems like landing on a story like this would be disorienting to someone that hasn't been using stories before.

/cc @hongwei1990 @newmuis

extensions/amp-story/1.0/amp-story-store-service.js Outdated Show resolved Hide resolved
@gmajoulet
Copy link
Contributor Author

should we show the education overlay when the user scrolls?

I think the plan is to test navigating to the next page on scroll, like Slides does. Might not be perfect, but the navigation arrows will help understanding how to navigate the story (cf internal doc)

const pages = createPages(story.element, 4, ['cover', '1', '2', '3']);
story.element.setAttribute('desktop-mode', 'fullbleed');

// Don't do this at home. :(
Copy link
Contributor

Choose a reason for hiding this comment

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

😂

@gmajoulet gmajoulet merged commit db0152a into ampproject:master Nov 14, 2018
Enriqe pushed a commit to Enriqe/amphtml that referenced this pull request Nov 28, 2018
* Opt in into the desktop full bleed experience.

* Renaming UIType.FULLBLEED into UIType.DESKTOP_FULLBLEED.
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

5 participants