🐛 Position offscreen amp-story-pages using viewport widths#16092
Merged
newmuis merged 5 commits intoampproject:masterfrom Jun 21, 2018
Merged
🐛 Position offscreen amp-story-pages using viewport widths#16092newmuis merged 5 commits intoampproject:masterfrom
newmuis merged 5 commits intoampproject:masterfrom
Conversation
…6067-offscreen-pages-position
…6067-offscreen-pages-position
newmuis
reviewed
Jun 18, 2018
|
|
||
| [desktop] amp-story-page { | ||
| transform: scale(1.0) translateX(-350%) translateY(0%) !important; | ||
| transform: scale(1.0) translateX(calc(-50vw - 100%)) translateY(0%) !important; |
Contributor
There was a problem hiding this comment.
LGTM, but I wonder if the precision is worth the extra computation? Since it's offscreen, we could do something like -200vw and it should be okay.
…6067-offscreen-pages-position
newmuis
approved these changes
Jun 18, 2018
alabiaga
added a commit
that referenced
this pull request
Jun 22, 2018
…6227) * Revert "Revert "Update dependencies for default 🌴 (#16221)" This reverts commit f8da39e. * Revert "Mark 4 more visual diff tests as flaky (#16214)" This reverts commit 867e903. * Revert "Update dependencies for default 🌴 (#16195)" This reverts commit 34d1977. * Revert "Fix selector (#16197)" This reverts commit f741868. * Revert "🐛 Position offscreen amp-story-pages using viewport widths (#16092)" This reverts commit fa85965. * Revert "Use performance.now over Date.now in performance metrics (#16119)" This reverts commit 0d2b567. * fix lint errors * fix lint error in constructor * Update performance-impl.js fix more lint errors * Update url-replacements-impl.js more lint.. * Update test-amp-access.js fix test error. * Update test-amp-access-source.js fix more tests...
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
3 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.
Closes #16067
@newmuis To accommodate for varying desktop widths I have refactored the CSS to use viewport units to position offscreen pages.
What I did was to replace instances of:
With:
And instances of:
With:
The
-100%is to push the<amp-story-page>further to the left by its own width.In some cases the
scaleis set to0.9in which case I multiply the values by1.1in order to compensate.Before
After
(nb: There is some artifacting in the screenshots due to compression)