[amp-story] Allow more parameters for pan/zoom animations ✨ #20778

Enriqe · 2019-02-11T22:37:26Z

Closes #20260

New proposal is to expose the start/end arguments of the zoom and pan animations:

Scales the image from 1x to 3x over 4 seconds:
animate-in="zoom-out" animate-in-duration="4s" scale-start="1" scale-end="3"

Translates the image 200px horizontally to the right over 4 seconds:
animate-in="pan-right" animate-in-duration="4s" translate-x="200px"

Potential question: why we chose to keep using pan-right instead of just using pan for all general directions and use the sign (+/-) in the value to get direction. The reasoning is that we need to be backwards compatible and not introduce any breaking changes.

A follow-up will be sent with validator changes and addition to docs.

newmuis

I definitely do find it weird that you could do pan-right with horizontal-translate="-200px". Maybe we should do runtime validation to enforce:

only positive numbers for zoom-start, zoom-end, horizontal-translate, and vertical-translate
for zoom-in, enforce that zoom-start < zoom-end
for zoom-out, enforce that zoom-start > zoom-end

newmuis · 2019-02-11T23:33:03Z

extensions/amp-story/1.0/animation.js

+/** const {string} */
+const ZOOM_END_ATTRIBUTE_NAME = 'zoom-end';
+/** const {string} */
+const HORIZONTAL_TRANSLATE_ATTRIBUTE_NAME = 'horizontal-translate';


nit: I think we should prefer translate-x and translate-y over horizontal-translate and vertical-translate, to stick with the defined conventions.

gmajoulet · 2019-02-12T23:27:41Z

extensions/amp-story/1.0/animation-presets.js

+        easing: 'linear',
+        keyframes: [
+          {transform: `scale(${zoomStart || 3}, ${zoomStart || 3})`},
+          {transform: `scale(${zoomEnd || 1}, ${zoomEnd || 1})`},


Nit: You can do scale(zoomEnd) with only one parameter so you save a few bytes : )

gmajoulet · 2019-02-12T23:29:20Z

extensions/amp-story/1.0/animation.js

+/** const {string} */
+const ZOOM_START_ATTRIBUTE_NAME = 'zoom-start';
+/** const {string} */
+const ZOOM_END_ATTRIBUTE_NAME = 'zoom-end';


Just like for translate-y, I like the idea of having an API that looks like the CSS properties it's going to set. Should we do scale- instead of zoom-?

Enriqe

PTAL

Enriqe · 2019-02-13T15:46:52Z

extensions/amp-story/1.0/animation.js

+/** const {string} */
+const ZOOM_START_ATTRIBUTE_NAME = 'zoom-start';
+/** const {string} */
+const ZOOM_END_ATTRIBUTE_NAME = 'zoom-end';


Enriqe · 2019-02-13T15:46:58Z

extensions/amp-story/1.0/animation-presets.js

+        easing: 'linear',
+        keyframes: [
+          {transform: `scale(${zoomStart || 3}, ${zoomStart || 3})`},
+          {transform: `scale(${zoomEnd || 1}, ${zoomEnd || 1})`},


gmajoulet

LGTM

One question for later:
<amp-img animate-in="pan-up" animate-in-duration="4s" translate-y="100px">...
At this point, do we still need to require this animate-in attribute? I share Jon's concern about being able to to a pan-up but specify a translate-y="-100px", which would result in a pan-down. That's not a big deal, but that might be a sign that we need to re-think parts of this API to reflect the latest changes.

Enriqe · 2019-02-13T20:25:17Z

We do other things in the pan-up/down/left/right though, like expanding the image to make sure it's larger than the viewport and placing it in a specific side depending on the preset. So it's not only about the translate, we do actually need the preset name to know where to position the image.

e.g. for pan-left we expand the image and "place" the viewport on the right edge of the image, so that it gives us the feeling we're panning left.

gmajoulet · 2019-02-13T20:51:26Z

Thanks for explaining this :))

newmuis · 2019-02-14T00:18:29Z

extensions/amp-story/1.0/animation-presets.js

+        duration: 1000,
+        easing: 'linear',
+        keyframes: [
+          {transform: `scale(${scaleStart || 1})`},


nit: can you hoist the defaults (1 and 3) here and below up to constants?

commit 08466820bf2c8c13e21abc5c26159d228edb345c Author: Esther Kim <[email protected]> Date: Fri Feb 15 10:47:56 2019 -0500 Whitespace commit cdfd719 Author: Esther Kim <[email protected]> Date: Fri Feb 15 10:34:13 2019 -0500 Stop timer logging commit 4b63cd7 Author: Esther Kim <[email protected]> Date: Fri Feb 15 10:08:59 2019 -0500 Typos commit 7db4b28 Author: Esther Kim <[email protected]> Date: Thu Feb 14 18:12:37 2019 -0500 Review comments commit ced9c5d Author: Esther Kim <[email protected]> Date: Thu Feb 14 18:01:12 2019 -0500 Review comments commit 8fa0e88 Merge: e65dd15 893ff3f Author: Esther Kim <[email protected]> Date: Thu Feb 14 17:29:46 2019 -0500 Merge branch 'master' into travis-build-matrix commit e65dd15 Author: Esther Kim <[email protected]> Date: Thu Feb 14 17:28:35 2019 -0500 Review comments commit d69c38a Author: Esther Kim <[email protected]> Date: Thu Feb 14 17:08:20 2019 -0500 Update logging commit 7e4f701 Author: Esther Kim <[email protected]> Date: Thu Feb 14 17:00:38 2019 -0500 Review changes commit 893ff3f Author: Aaron Turner <[email protected]> Date: Thu Feb 14 13:49:16 2019 -0800 Return 400 when recaptcha example fails (ampproject#20668) commit 3f0d6c6 Author: Enrique Marroquin <[email protected]> Date: Thu Feb 14 16:37:15 2019 -0500 [amp-story] Allow more parameters for pan/zoom animations ✨ (ampproject#20778) * allow more parameters in for pan/zoom animations. * change attribute names * types * fix test * assert values for params, change attribute naming. * change attribute names do scale-start, scale-end. * hoist defaults to constants commit 35d6030 Author: Justin Ridgewell <[email protected]> Date: Thu Feb 14 16:02:00 2019 -0500 🐛 Assert that values are never returned from vsync callbacks (ampproject#20836) * Assert that values are never returned from vsync callbacks Vsync does not propogate those values in any way. This is specifically targetting code like: ```js vsync.measure(() => { return el.getBoundingClientRect(); }); // or vsync.mutatePromise(() => { return someOtherPromise; }); ``` * Fix dev call * Lint * Don't use devAssert so this stays in production * Type checks commit b9ba432 Author: Esther Kim <[email protected]> Date: Thu Feb 14 15:52:54 2019 -0500 Fix nit, hopefully fix chrome start commit 3cb87fa Author: Esther Kim <[email protected]> Date: Thu Feb 14 15:08:42 2019 -0500 bash commit 5d9844e Author: Esther Kim <[email protected]> Date: Thu Feb 14 14:54:40 2019 -0500 rm commit b027eb0 Author: Esther Kim <[email protected]> Date: Thu Feb 14 14:39:52 2019 -0500 Review comments commit fe27d76 Author: sohanirao <[email protected]> Date: Thu Feb 14 11:38:43 2019 -0800 SwG Release 0.1.22.44 (ampproject#20834) commit 736d506 Author: Esther Kim <[email protected]> Date: Thu Feb 14 14:25:37 2019 -0500 Install chrome for dist test job, add EXTENSIONS_CSS_MAP to build artifact zip commit a36ee5d Author: delima02 <[email protected]> Date: Thu Feb 14 14:17:42 2019 -0500 🐛 Guard against getBoundingClientRect to prevent unspecified errors in IE. (ampproject#20731) * Guard against `Element.getBoundingClientRect`. * Refactor solution. * Check if element exists in the DOM. * Fix solution using `Node.isConnected`. * Correct the default value of the desired DOMRect * Patch getBoundingClientRect for unsupported browsers. Alternative solution * Refactor patch. * Fix - use assigned variable rect, previously unused. * Fix imports. * Minor fixes. * Make the return type of getBoundingClientRect be consistent with usages * File overview comment and a minor fix. commit e9acbd2 Author: Cathy Zhu <[email protected]> Date: Fri Feb 15 03:17:37 2019 +0800 Undo space reservation after successful height change (ampproject#20856) commit ccea3e2 Author: Nikita Beloglazov <[email protected]> Date: Thu Feb 14 11:12:16 2019 -0800 Export few data types from content-recommendation.js (ampproject#20854) It allows users of that file to use the types. Given that we return objects of these types from exported functions - exporting types themselves makes sense. Tested: presubmit commit 80f9fa9 Author: Sepand Parhami <[email protected]> Date: Thu Feb 14 11:10:11 2019 -0800 🐛Do not perform static layout for cloned nodes with static layout. (ampproject#20724) commit a751cc7 Author: keithwrightbos <[email protected]> Date: Thu Feb 14 13:47:48 2019 -0500 AdSense Fast Fetch delay 3 viewport experiment - respect loading strategy (ampproject#20866) * initial commit * modify dep-check * truly fix dep issue commit 806db59 Author: Carlos Vializ <[email protected]> Date: Thu Feb 14 13:21:26 2019 -0500 ✨Implement autoexpand for amp-form textarea element (ampproject#20772) * Add expanding textarea feature to amp-form * Add autoshrink behavior in addition to autoexpand * reset * reset2 it works * Does not stay at top * Prevent scrollbar from flashing before max-height * default autoshrink. Prevent shrinking if the textarea hasn't yet expanded * Use class style * Add manual test * Cleanup * Review feedback. Remove autoshrink attr. Lazy-load feature. * Encapsulate all textarea logic, including installation. * Mark expensive property accesses as OK * Warn and forbid textarea with initial overflow * Add tests for confidence. Fix presubmit errors. commit cb95582 Author: Esther Kim <[email protected]> Date: Thu Feb 14 11:51:37 2019 -0500 Separate PR jobs commit 4679a4c Author: Gabriel Majoulet <[email protected]> Date: Thu Feb 14 10:52:06 2019 -0500 Displaying more specific messages when users have to resize their window, + refactoring. (ampproject#20835) commit b9814f5 Author: keithwrightbos <[email protected]> Date: Thu Feb 14 10:43:42 2019 -0500 initial commit (ampproject#20865) commit e570d46 Author: Hongfei Ding <[email protected]> Date: Wed Feb 13 22:15:49 2019 -0800 amp-ad-exit: use HostService if exists. (ampproject#20357) * amp-ad-exit: use HostService if exists. * naming & docs * rename host-api to host-service * lint commit ecc3daa Author: Cathy Zhu <[email protected]> Date: Thu Feb 14 13:34:08 2019 +0800 Make is-my-pr-in-production-yet more prominent in our docs (ampproject#20857) commit 12d441c Author: Alan Orozco <[email protected]> Date: Wed Feb 13 13:44:50 2019 -0800 ✨auto-lightbox: Discard tap actions with invalid references (ampproject#20789) Exposes `ActionService.hasResolvableAction` to: - Discard tap action in auto-lightbox criteria when it does not resolve to a valid target. - Fix `amp-lightbox-gallery` so it will properly traverse up the tree to discard targets, and not to discard those dynamically set. commit dd4037d Author: Nikita Beloglazov <[email protected]> Date: Wed Feb 13 13:25:13 2019 -0800 Improve Google matched content logic to reflect adsbygoogle.js. (ampproject#20624) The commit introduces core-shared.js file which supposed to be shared between AMP and internal Google adsbygoogle.js codebase. The file has no dependencies on environment to make importing these files in other projects easier. In particular this commit adds support for publisher settings as described in https://support.google.com/adsense/answer/7533385 Tested: unit tests manually on fast festh and delayed fetch commit 76a7c0f Author: keithwrightbos <[email protected]> Date: Wed Feb 13 16:10:58 2019 -0500 initial commit (ampproject#20837) commit f1fb33b Author: keithwrightbos <[email protected]> Date: Wed Feb 13 15:37:39 2019 -0500 initial commit (ampproject#20833) commit 6831b63 Author: Nikita Beloglazov <[email protected]> Date: Wed Feb 13 12:37:10 2019 -0800 Cleanup adsense amp-auto-ads holdout experiment. Close ampproject#9247. (ampproject#20774) It has been running for a while and we no longer need it. If someone decides to look at results in future - they can rollback this commit and restart experiment. Tested: presubmit commit 4f3cd0d Author: William Chou <[email protected]> Date: Wed Feb 13 15:28:26 2019 -0500 Don't send amp-bind state to untrusted viewers (ampproject#20822) * Only send state for history updates to trusted viewers. * Add and fix tests. * Fix type check. * Fix presubmit. commit ab467bc Author: Carlos Vializ <[email protected]> Date: Wed Feb 13 14:26:35 2019 -0500 ✅ Resolve some flakiness in e2e tests (ampproject#20794) * Resolve some flakiness in e2e tests * Unskip passing test commit e13a531 Author: Aaron Labiaga <[email protected]> Date: Wed Feb 13 13:35:59 2019 -0500 Make test-resource.js not flaky (ampproject#20514) Remove call count expectations on getRect method call which is the culprit of the flakiness of the tests mentioned. This should be fine as it doesn't take away from what's being primarily tested as it occurs in the afterEach call. Note that this is not a permanent fix but a temporary one to alleviate the flakiness. I will keep ampproject#16124 open. Tested locally and ran it several times in saucelabs. commit 4a7282f Author: Carlos Vializ <[email protected]> Date: Wed Feb 13 12:01:23 2019 -0500 Skip date picker visual diff tests for flakes (ampproject#20809) commit 703ab02 Author: William Chou <[email protected]> Date: Wed Feb 13 11:53:05 2019 -0500 Clean up 'user-error-reporting' (ampproject#20779) * Clean up user-error-reporting. * Fix lint. commit 229d909 Author: Alan Orozco <[email protected]> Date: Wed Feb 13 02:44:33 2019 -0800 🛑 <amp-video-iframe> integration safeguards (ampproject#20826) - Prefix all frame-level errors with `<amp-video-iframe>` to let authors know source of integration mistakes. - Fail to `adopt` window twice, notify that script tag should only be included once. - Display valid listener types when trying to `listenTo('invalidFramework')`. - Don't let author `listenTo` twice. commit 84f701f Author: Ali Ghassemi <[email protected]> Date: Tue Feb 12 23:21:22 2019 -0800 ✅ amp-youtube: add amp=1 to the url (ampproject#20821) commit b58b6fa Author: Alan Orozco <[email protected]> Date: Tue Feb 12 23:03:58 2019 -0800 ✨ Set #amp=1 fragment in amp-video-iframe src (ampproject#20823) Docs say we do it, but we don't, so let's do it! Also cleaned up some tests to be clearer and to use `async/await`. commit f92f627 Author: Justin Ridgewell <[email protected]> Date: Tue Feb 12 20:24:26 2019 -0500 Optimize scopeSelector (ampproject#20819) https://jsbench.github.io/#e00c46505b5b875d2dcd5894d6e46adc commit 0298883 Author: Alan Orozco <[email protected]> Date: Tue Feb 12 16:57:41 2019 -0800 🐛Reset min dimensions on undock (ampproject#20817) Otherwise the video can be oversized and clipped when docked and resizing the window. commit bcce892 Author: Caleb Cordry <[email protected]> Date: Tue Feb 12 16:33:05 2019 -0800 📖amp story ads RTC & Targeting docs (ampproject#20818)

…ct#20778) * allow more parameters in for pan/zoom animations. * change attribute names * types * fix test * assert values for params, change attribute naming. * change attribute names do scale-start, scale-end. * hoist defaults to constants

googlebot added the cla: yes label Feb 11, 2019

Enriqe changed the title ~~[amp-story] Allow more parameters in for pan/zoom animations ✨~~ [amp-story] Allow more parameters for pan/zoom animations ✨ Feb 11, 2019

Enriqe requested a review from newmuis February 11, 2019 23:15

Enriqe self-assigned this Feb 11, 2019

Enriqe mentioned this pull request Feb 11, 2019

[amp-story] Add stop-animation-after=x parameter to animations. #20281

Closed

Enriqe requested a review from gmajoulet February 12, 2019 15:06

newmuis reviewed Feb 12, 2019

View reviewed changes

Enriqe added 5 commits February 12, 2019 18:25

allow more parameters in for pan/zoom animations.

74883b4

change attribute names

9d862b0

types

dc7c633

fix test

f167f43

assert values for params, change attribute naming.

12110bb

Enriqe force-pushed the vfx-parameters branch from 8d054b0 to 12110bb Compare February 12, 2019 23:25

gmajoulet reviewed Feb 12, 2019

View reviewed changes

change attribute names do scale-start, scale-end.

87f0ec1

Enriqe commented Feb 13, 2019

View reviewed changes

gmajoulet approved these changes Feb 13, 2019

View reviewed changes

newmuis approved these changes Feb 14, 2019

View reviewed changes

hoist defaults to constants

0c5ec26

Enriqe merged commit 3f0d6c6 into ampproject:master Feb 14, 2019

Enriqe deleted the vfx-parameters branch March 4, 2019 15:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[amp-story] Allow more parameters for pan/zoom animations ✨ #20778

[amp-story] Allow more parameters for pan/zoom animations ✨ #20778

Enriqe commented Feb 11, 2019 •

edited

Loading

newmuis left a comment •

edited

Loading

newmuis Feb 11, 2019

Enriqe Feb 12, 2019

gmajoulet Feb 12, 2019

Enriqe Feb 13, 2019

gmajoulet Feb 12, 2019

Enriqe Feb 13, 2019

Enriqe left a comment

Enriqe Feb 13, 2019

Enriqe Feb 13, 2019

gmajoulet left a comment

Enriqe commented Feb 13, 2019

gmajoulet commented Feb 13, 2019

newmuis Feb 14, 2019

[amp-story] Allow more parameters for pan/zoom animations ✨ #20778

[amp-story] Allow more parameters for pan/zoom animations ✨ #20778

Conversation

Enriqe commented Feb 11, 2019 • edited Loading

newmuis left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Enriqe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmajoulet left a comment

Choose a reason for hiding this comment

Enriqe commented Feb 13, 2019

gmajoulet commented Feb 13, 2019

Choose a reason for hiding this comment

Enriqe commented Feb 11, 2019 •

edited

Loading

newmuis left a comment •

edited

Loading