✨ Update amp-access-scroll#26810
Merged
dvoytenko merged 7 commits intoampproject:masterfrom Feb 22, 2020
Merged
Conversation
2c6556b to
8535f4f
Compare
This PR - adds a protocol version query param to requests made from amp-access-scroll for compatibility - adds a "holdback" flag and styles to prevent mismatched rendeirng with new layout - renames ScrollAudio to ScrollSheet - adds ability to dynamically size horizontal layout for ScrollComponents. Special care was taken to ensure these properties cannot interrupt vertical browser layout; only width, left, and right are allowed. Dynamic sizing is necessary to pervent invisible iframes being drawn over usable parts of the underlying page, which would cause UX and accessibility problems in certain layout scenarios. Dynamic sizing solves this by shrinking the iframe to fit the size neccessary to fit the rendered controls.
8535f4f to
7ec8d4b
Compare
b32000b to
288854d
Compare
72d0271 to
d0c42b5
Compare
dbow
reviewed
Feb 14, 2020
| } | ||
|
|
||
| .amp-access-scroll-sheet.amp-access-scroll-holdback { | ||
| position: fixed; |
Contributor
There was a problem hiding this comment.
seems like these styles are unnecessary with the .amp-access-scroll-sheet classname providing them too?
dbow
reviewed
Feb 14, 2020
| } | ||
| } | ||
|
|
||
| .amp-access-scroll-sheet { |
Contributor
There was a problem hiding this comment.
could you put this classname above the modifications to it?
Contributor
Author
|
To clarify, the holdback code / css classes will be removed in a future PR. |
kushal
approved these changes
Feb 14, 2020
2ae3590 to
b6dee79
Compare
dvoytenko
approved these changes
Feb 21, 2020
Contributor
|
LGTM from me. Ready to be merged? |
Contributor
Author
|
@dvoytenko yep, thanks! |
Contributor
|
Yup! |
Contributor
|
/cc @jpettitt |
robinvanopstal
added a commit
to jungvonmatt/amphtml
that referenced
this pull request
Feb 24, 2020
* master: (41 commits) custom-element: Minor test improvements (ampproject#26923) amp-pixel: Minor test improvements (ampproject#26918) viewer: Minor test improvements (ampproject#26906) dom: Minor test improvements (ampproject#26913) amp-action: Support whitelist lookup in AmpDocShadow (ampproject#26684) ✨ Update amp-access-scroll (ampproject#26810) 🚀 Remove doc css and base css from ESM build (ampproject#26889) 📖 [amp-story-player] Initial docs (ampproject#26606) Amp consent restrict fullscreen prod flag (ampproject#26909) 📖 Clarify SXG duration minimum (ampproject#26890) Improve test vendor requests macros (ampproject#26828) 🚀 Move scroll left and top macros out of url-replacement-impl (ampproject#25594) Update consent string maximum size to 200 bytes (ampproject#26741) ✨[amp-story-player] Adds tap-to-next/previous story (ampproject#26865) update owners file with correct syntax (ampproject#26899) amp-sticky-ad: Fix unit test (ampproject#26855) Add performance metrics to README (ampproject#26891) 🐛 Bug fix: check links test (ampproject#26739) ✨Idealmedia uniq ad (ampproject#25838) 📦 Update dependency jsdom to v16.2.0 (ampproject#26591) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR
was taken to ensure these properties cannot interrupt vertical browser layout;
only width, left, and right are allowed.
Dynamic sizing is necessary to pervent invisible iframes being drawn over usable
parts of the underlying page, which would cause UX and accessibility problems in
certain layout scenarios. Dynamic sizing solves this by shrinking the iframe to fit
the size neccessary to fit the rendered controls.