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

✨Handling sticky, fixed and hidden elements in amp-next-page v2 #26106

Merged
merged 8 commits into from
Jan 13, 2020

Conversation

wassgha
Copy link
Contributor

@wassgha wassgha commented Dec 19, 2019

Changes

  • Breaking: In contrast to amp-next-page v1 fixed elements will no longer be hidden by default, they will however be constrained to their document's bounds (they won't overflow beyond the document's borders when the document is not visible).
  • Introduces two new global attributes amp-next-page-hide, and amp-next-page-replace:
Attribute Value Description
amp-next-page-hide None Can be added to any element that should only appear in the host document (and not in subsequently loaded pages)
amp-next-page-replace Unique ID Denotes elements that are shared between the host document and subsequent next pages. The element belonging to the last visible page will replace any previous instances of the same ID

Demo page: Demo Site

@amp-owners-bot
Copy link

Hey @ampproject/wg-caching, these files were changed:

  • validator/validator-main.protoascii

@wassgha wassgha force-pushed the amp-next-page-v2-hide-elements branch 3 times, most recently from a31055c to 3a9b3b6 Compare December 19, 2019 20:43
@wassgha wassgha force-pushed the amp-next-page-v2-hide-elements branch from 3a9b3b6 to fd8de0d Compare December 19, 2019 21:14
@wassgha wassgha force-pushed the amp-next-page-v2-hide-elements branch from 0ab97c0 to 5733c3e Compare December 19, 2019 21:49
@Gregable
Copy link
Member

Gregable commented Jan 2, 2020

Validator changes look good, especially since they are just a comment change.

Copy link
Contributor

@caroqliu caroqliu left a comment

Choose a reason for hiding this comment

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

Nice job! Don't forget to add unit tests and update the documentation file with the new attributes.

extensions/amp-next-page/0.2/page.js Show resolved Hide resolved
extensions/amp-next-page/0.2/service.js Outdated Show resolved Hide resolved
extensions/amp-next-page/0.2/service.js Outdated Show resolved Hide resolved
extensions/amp-next-page/0.2/service.js Outdated Show resolved Hide resolved
Copy link
Contributor

@caroqliu caroqliu left a comment

Choose a reason for hiding this comment

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

This looks a lot more readable and clear now, thanks!

@wassgha wassgha requested a review from caroqliu January 10, 2020 08:09
@wassgha wassgha merged commit 374a04f into ampproject:master Jan 13, 2020
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.

5 participants