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

Making pagination buttons available on panels and fullbleed desktop experience. #19315

Merged
merged 1 commit into from Nov 16, 2018

Conversation

gmajoulet
Copy link
Contributor

  • Small improvement to only build the navigation buttons when needed (never on mobile)
  • No page sentinel on the fullbleed desktop UI, to make sure the whole page stays clickable (it'd prevent from accessing elements beneath it, like future interactive components)
  • Higher opacity on fullbleed desktop UI
  • Not rewriting any CSS, just moving the pagination buttons code to their own file.
    story-desktop.css still makes a few overrides, like setting the size and position of the page-sentinel, since it's very specific to that panels UI.
  • Publishers who already have their own navigation arrows will be able to hide them using the [desktop-fullbleed] attribute set on the amp-story tag.

Demo:
https://stamp-desktop-fullbleed.firebaseapp.com/examples/s20/body-painting/index.html
https://stamp-desktop-fullbleed.firebaseapp.com/examples/s20/body-painting/index-panels.html

#19270

@gmajoulet
Copy link
Contributor Author

Friendly ping: there's actually not much to review besides the demos, since it's just moving code :) lmk if you want to talk about it in person to make it easier

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.

Not difficult to review, just forgot thanks to viewing it once in my GH notifications and never seeing it again

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