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

🐛 [Story a11y] Reduced motion should apply the animation's last frame to the elements #34466

Merged
merged 67 commits into from
Jul 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
85fa95f
Created e2e test
mszylkowski Apr 6, 2021
2d2dc25
Added tests
mszylkowski Apr 6, 2021
e596041
Fixed e2e tests
mszylkowski Apr 6, 2021
052e788
Removed comment
mszylkowski Apr 6, 2021
932f19c
Removed unused import
mszylkowski Apr 6, 2021
d7c648c
Merge branch 'master' of github.com:ampproject/amphtml into ampvideo_e2e
mszylkowski Apr 7, 2021
478589e
Addede e2es
mszylkowski Apr 8, 2021
73385c7
Moved field to html
mszylkowski Apr 8, 2021
6ea2c13
More work on e2e
mszylkowski Apr 9, 2021
1ccf0bf
Added load unload fixture
mszylkowski Apr 12, 2021
40ac1f1
Fixed tests
mszylkowski Apr 12, 2021
48cf2fb
Added more tests and cleaned up hooks
mszylkowski Apr 12, 2021
352f7a4
Added multiple sources on first videos to test flexible-bitrate
mszylkowski Apr 13, 2021
f7453d1
Merge branch 'main' of github.com:ampproject/amphtml into ampvideo_e2e
mszylkowski Apr 22, 2021
dd92f63
Merge branch 'main' of github.com:ampproject/amphtml into ampvideo_e2e
mszylkowski Apr 23, 2021
7389a6c
Changed e2es to work
mszylkowski Apr 23, 2021
e53b606
Removed whitespaces
mszylkowski Apr 23, 2021
d3cde6c
Merge branch 'main' of github.com:ampproject/amphtml into ampvideo_e2e
mszylkowski Apr 26, 2021
7ec9ac2
Adding cache fetch test
mszylkowski Apr 26, 2021
625d1fd
Add e2e fixture to disallowlist for validate
mszylkowski Apr 26, 2021
913c51a
Merge branch 'main' of github.com:ampproject/amphtml into ampvideo_e2e
mszylkowski May 3, 2021
cce1cae
Merge branch 'main' of github.com:ampproject/amphtml into ampvideo_e2e
mszylkowski May 4, 2021
b0d3b57
fixed host_url in e2e
mszylkowski May 4, 2021
7c63f98
Updated tests
mszylkowski May 18, 2021
4ce66ae
Changed order of test conditions for consistency
mszylkowski May 18, 2021
9468218
Update extensions/amp-video/0.1/test-e2e/test-amp-video-flexible-bitr…
gmajoulet May 18, 2021
114fd36
Update extensions/amp-video/0.1/test-e2e/test-amp-video-flexible-bitr…
gmajoulet May 18, 2021
21e9694
Merge branch 'main' of github.com:ampproject/amphtml into animation_r…
mszylkowski May 19, 2021
3a91465
Fixed reduced motion animation
mszylkowski May 19, 2021
13cd887
Fixed reduced motion
mszylkowski May 19, 2021
b1f1e8e
Removed unnecessary ?
mszylkowski May 20, 2021
8d8da4e
Reverted example html
mszylkowski May 20, 2021
08a50f8
Added anim to bot rendering
mszylkowski May 20, 2021
799bcd8
Remove false test
mszylkowski May 20, 2021
e27b2ff
Fixed linting
mszylkowski May 20, 2021
1a8e4b8
Fix visual diff
mszylkowski May 21, 2021
16a7d8b
Merge branch 'main' of github.com:ampproject/amphtml into animation_r…
mszylkowski May 24, 2021
781f4e1
Fixed paused devAssert on touch pause
mszylkowski May 24, 2021
6f79cc5
Fail gracefully
mszylkowski May 25, 2021
8de825b
StartOrFinishAnimation
mszylkowski May 26, 2021
ef10700
Fixed animation now working
mszylkowski May 26, 2021
1719650
Removed console logs
mszylkowski May 26, 2021
1be1ba0
Added maybePause or maybeResume
mszylkowski May 26, 2021
42debd5
Using maybeInit
mszylkowski May 27, 2021
d127851
Fixed bot rendering visstest
mszylkowski May 27, 2021
aa00ea1
Fixed a pause test and removed unused method
mszylkowski Jun 2, 2021
03c1547
Merge branch 'main' of github.com:ampproject/amphtml into animation_r…
mszylkowski Jun 2, 2021
dc26575
Merge branch 'main' of github.com:ampproject/amphtml into animation_r…
mszylkowski Jun 2, 2021
1e194ed
Removed finishOrPause
mszylkowski Jun 2, 2021
1065721
Merge branch 'animation_reducedmotion' of github.com:mszylkowski/amph…
mszylkowski Jun 3, 2021
9abfb6d
Merge pt2
mszylkowski Jun 3, 2021
655e110
Merge branch 'main' of github.com:ampproject/amphtml into animation_r…
mszylkowski Jun 3, 2021
d32ee51
Fixed animation.js
mszylkowski Jun 3, 2021
9ecdf64
Removed unused property prefersReducedMotion
mszylkowski Jun 3, 2021
208c1e2
Fixed error when switching user prefs
mszylkowski Jun 3, 2021
aa82246
Merge branch 'main' of github.com:ampproject/amphtml into animation_r…
mszylkowski Jun 4, 2021
36c3688
Fixed animation tests
mszylkowski Jun 4, 2021
54703ef
Added delay to bot rendering so the animations are applied
mszylkowski Jun 4, 2021
7e5b84b
removed unused maybe and isinitialized
mszylkowski Jun 18, 2021
d300898
merge branch main of amphtml into animation_reducedmotion
mszylkowski Jun 21, 2021
15f84f2
Merge branch 'main' of github.com:ampproject/amphtml into animation_r…
mszylkowski Jun 23, 2021
864827e
Simplify code
mszylkowski Jun 23, 2021
dd15635
Simplify code
mszylkowski Jun 23, 2021
2c3d045
Simplify a bit more
mszylkowski Jun 23, 2021
69be1fe
Added timer to visual test
mszylkowski Jun 23, 2021
896e8fa
Removed timer
mszylkowski Jun 23, 2021
fabaece
Merge branch 'main' of github.com:ampproject/amphtml into animation_r…
mszylkowski Jun 25, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions examples/amp-story/amp-story-animation.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@
custom-element="amp-story"
src="https://cdn.ampproject.org/v0/amp-story-1.0.js"
></script>
<script
async
custom-element="amp-video"
src="https://cdn.ampproject.org/v0/amp-video-0.1.js"
></script>
<script
async
custom-element="amp-animation"
src="https://cdn.ampproject.org/v0/amp-animation-0.1.js"
></script>
<style amp-custom>
body {
font-family: sans-serif;
Expand Down Expand Up @@ -117,6 +127,39 @@
</script>
</amp-story-animation>
</amp-story-page>

<amp-story-page id="hidden-animations">
<amp-story-animation layout="nodisplay" trigger="visibility">
<script type="application/json">
[
{
"selector": ".anim",
"keyframes": {
"transform": ["translate3d(-100%, 0px, 0)", "translate3d(0px, 0px, 0)"]
},
"delay": 100,
"direction": "normal",
"duration": 1000,
"easing": "cubic-bezier(0.19, 1, 0.22, 1)",
"fill": "both"
}
]
</script>
</amp-story-animation>
<amp-story-grid-layer template="vertical">
<amp-video autoplay="autoplay" layout="fill" loop="loop">
<source type="video/mp4" src="./video/stamp.mp4">
</amp-video>
</amp-story-grid-layer>
<amp-story-grid-layer template="vertical" aspect-ratio="412:618">
<div class="anim" style="width:80%;height:20%;position:absolute;font-size:30px;background:white;left:10%;top:20%;transform:translate3d(-100%, 0px, 0)">
This is the animated text with initial frame in CSS
</div>
<div class="anim" style="width:80%;height:20%;position:absolute;font-size:30px;background:white;left:10%;top:60%">
This is the animated text with final frame in CSS
</div>
</amp-story-grid-layer>
</amp-story-page>
</amp-story>
</body>
</html>
17 changes: 16 additions & 1 deletion examples/visual-tests/amp-story/amp-story-bot-rendering.html
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,26 @@
</amp-story-page>

<amp-story-page id="page-link">
<amp-story-animation layout="nodisplay" trigger="visibility">
<script type="application/json">
[
{
"selector": ".anim",
"keyframes": {
"opacity": ["0", "1"]
},
"direction": "normal",
"fill": "both",
"duration": 1
}
]
</script>
</amp-story-animation>
<amp-story-grid-layer template="fill">
<amp-img layout="fill" src="./img/cat2.jpg"></amp-img>
</amp-story-grid-layer>
<amp-story-grid-layer template="vertical">
<p>
<p class="anim">
<span data-text-background-color="crimson">i can fit in that box but inspect anything brought into the house wack the mini furry mouse intently stare at the same spot. Flee in terror at cucumber discovered on floor shake treat bag</span>
</p>
</amp-story-grid-layer>
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-animation/0.1/runners/animation-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ export class AnimationRunner {
seekToPercent(unusedPercent) {}

/**
* @param {bool} unusedPauseOnError
*/
finish() {}
finish(unusedPauseOnError = false) {}

/**
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ export class NativeWebAnimationRunner extends AnimationRunner {
* @param {time} time
*/
seekTo(time) {
devAssert(this.players_);
if (!this.players_) {
mszylkowski marked this conversation as resolved.
Show resolved Hide resolved
return;
}
this.setPlayState_(WebAnimationPlayState.PAUSED);
this.players_.forEach((player) => {
player.pause();
Expand All @@ -197,15 +199,24 @@ export class NativeWebAnimationRunner extends AnimationRunner {
/**
* @override
*/
finish() {
finish(pauseOnError = false) {
if (!this.players_) {
return;
}
const players = this.players_;
this.players_ = null;
this.setPlayState_(WebAnimationPlayState.FINISHED);
players.forEach((player) => {
player.finish();
if (pauseOnError) {
try {
// Will fail if animation is infinite, in that case we pause it.
player.finish();
} catch (error) {
player.pause();
}
} else {
player.finish();
}
mszylkowski marked this conversation as resolved.
Show resolved Hide resolved
});
}

Expand Down
20 changes: 8 additions & 12 deletions extensions/amp-story/1.0/amp-story-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ import {isPageAttachmentUiV2ExperimentOn} from './amp-story-page-attachment-ui-v
import {isPrerenderActivePage} from './prerender-active-page';
import {listen, listenOnce} from '../../../src/event-helper';
import {CSS as pageAttachmentCSS} from '../../../build/amp-story-open-page-attachment-0.1.css';
import {prefersReducedMotion} from '#core/dom/media-query-props';
import {propagateAttributes} from '#core/dom/propagate-attributes';
import {px, toggle} from '#core/dom/style';
import {renderPageAttachmentUI} from './amp-story-open-page-attachment';
Expand Down Expand Up @@ -318,9 +317,6 @@ export class AmpStoryPage extends AMP.BaseElement {
if (this.animationManager_) {
return;
}
if (prefersReducedMotion(this.win)) {
return;
}
if (!hasAnimations(this.element)) {
return;
}
Expand Down Expand Up @@ -725,7 +721,7 @@ export class AmpStoryPage extends AMP.BaseElement {

/** @return {!Promise} */
beforeVisible() {
return this.maybeApplyFirstAnimationFrame();
return this.maybeApplyFirstAnimationFrameOrFinish();
}

/**
Expand Down Expand Up @@ -1260,7 +1256,10 @@ export class AmpStoryPage extends AMP.BaseElement {
* @private
*/
maybeStartAnimations_() {
this.animationManager_?.animateIn();
if (!this.animationManager_) {
return;
}
this.animationManager_.animateIn();
}

/**
Expand All @@ -1274,17 +1273,14 @@ export class AmpStoryPage extends AMP.BaseElement {
}
this.signals()
.whenSignal(CommonSignals.LOAD_END)
.then(() => this.maybeApplyFirstAnimationFrame())
.then(() => {
this.animationManager_.finishAll();
});
.then(() => this.animationManager_.applyLastFrame());
}

/**
* @return {!Promise}
*/
maybeApplyFirstAnimationFrame() {
return Promise.resolve(this.animationManager_?.applyFirstFrame());
maybeApplyFirstAnimationFrameOrFinish() {
return Promise.resolve(this.animationManager_?.applyFirstFrameOrFinish());
}

/**
Expand Down
54 changes: 42 additions & 12 deletions extensions/amp-story/1.0/animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {dev, devAssert, user, userAssert} from '../../../src/log';
import {escapeCssSelectorIdent} from '#core/dom/css-selectors';
import {getChildJsonConfig} from '../../../src/json';
import {map, omit} from '#core/types/object';
import {prefersReducedMotion} from '#core/dom/media-query-props';
import {scopedQuerySelector, scopedQuerySelectorAll} from '#core/dom/query';
import {setStyles} from '#core/dom/style';
import {timeStrToMillis, unscaledClientRect} from './utils';
Expand Down Expand Up @@ -326,6 +327,20 @@ export class AnimationRunner {
});
}

/**
* Applies the last animation frame.
* @return {!Promise<void>}
*/
applyLastFrame() {
if (this.presetTarget_) {
return Promise.resolve();
}
this.runnerPromise_.then((runner) => {
runner.init();
runner.finish(/* pauseOnError */ true);
});
}

/** Starts or resumes the animation. */
start() {
if (this.hasStarted()) {
Expand Down Expand Up @@ -380,12 +395,7 @@ export class AnimationRunner {
}

if (this.runner_) {
try {
this.runner_.pause();
} catch (e) {
// This fails when the animation is finished explicitly
// (runner.finish()) since this destroys internal players. This is fine.
}
this.runner_.pause();
}
}

Expand All @@ -398,7 +408,7 @@ export class AnimationRunner {
}

if (this.runner_) {
devAssert(this.runner_).resume();
this.runner_.resume();
}
}

Expand Down Expand Up @@ -539,6 +549,9 @@ export class AnimationManager {
/** @private @const */
this.builderPromise_ = this.createAnimationBuilderPromise_();

/** @private @const {bool} */
this.prefersReducedMotion_ = prefersReducedMotion(ampdoc.win);

/** @private {?Array<!AnimationRunner>} */
this.runners_ = null;

Expand All @@ -561,14 +574,31 @@ export class AnimationManager {
* Applies first frame to target element before starting animation.
* @return {!Promise}
*/
applyFirstFrame() {
applyFirstFrameOrFinish() {
return Promise.all(
this.getOrCreateRunners_().map((runner) => runner.applyFirstFrame())
this.getOrCreateRunners_().map((runner) =>
this.prefersReducedMotion_
? runner.applyLastFrame()
: runner.applyFirstFrame()
)
);
}

/**
* Applies last frame to target element before starting animation.
* @return {!Promise}
*/
applyLastFrame() {
return Promise.all(
this.getOrCreateRunners_().map((runner) => runner.applyLastFrame())
);
}

/** Starts all entrance animations for the page. */
animateIn() {
if (this.prefersReducedMotion_) {
return;
}
this.getRunners_().forEach((runner) => runner.start());
}

Expand All @@ -588,15 +618,15 @@ export class AnimationManager {

/** Pauses all animations in the page. */
pauseAll() {
if (!this.runners_) {
if (!this.runners_ || this.prefersReducedMotion_) {
return;
}
this.getRunners_().forEach((runner) => runner.pause());
}

/** Resumes all animations in the page. */
resumeAll() {
if (!this.runners_) {
if (!this.runners_ || this.prefersReducedMotion_) {
return;
}
this.getRunners_().forEach((runner) => runner.resume());
Expand All @@ -615,7 +645,7 @@ export class AnimationManager {
* @private
*/
getRunners_() {
return devAssert(this.runners_, 'Executed before applyFirstFrame');
return devAssert(this.runners_, 'Executed before applyFirstFrameOrFinish');
}

/**
Expand Down
13 changes: 0 additions & 13 deletions extensions/amp-story/1.0/test/test-amp-story-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import * as MediaQueryProps from '#core/dom/media-query-props';
import * as VideoUtils from '#core/dom/video';
import {Action, AmpStoryStoreService} from '../amp-story-store-service';
import {AmpAudio} from '../../../amp-audio/0.1/amp-audio';
Expand Down Expand Up @@ -118,18 +117,6 @@ describes.realWin('amp-story-page', {amp: {extensions}}, (env) => {
expect(page.animationManager_).to.exist;
});

it('should not build the animation manager if `prefers-reduced-motion` is on', async () => {
env.sandbox.stub(MediaQueryProps, 'prefersReducedMotion').returns(true);

const animatedEl = html`<div animate-in="fade-in"></div>`;

element.appendChild(animatedEl);
element.getAmpDoc = () => new AmpDocSingle(win);

page.buildCallback();
expect(page.animationManager_).to.be.null;
});

it('should set an active attribute when state becomes active', async () => {
page.buildCallback();
await page.layoutCallback();
Expand Down
Loading