Skip to content

Commit

Permalink
fix(animation): avoid partial keyframes (#19169)
Browse files Browse the repository at this point in the history
* ensure custom props not part of final keyframes

* fix clear

* clean up
  • Loading branch information
liamdebeasi committed Aug 23, 2019
1 parent fb70980 commit fa958a5
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 112 deletions.
4 changes: 2 additions & 2 deletions core/src/utils/animation/animation-interface.ts
Expand Up @@ -16,8 +16,8 @@ export interface Animation {
progressEnd(shouldComplete: boolean, step: number, dur: number | undefined): Animation;

from(property: string, value: any): Animation;
to(property: string, value: any, clearAfterAnimation?: boolean): Animation;
fromTo(property: string, fromValue: any, toValue: any, clearAfterAnimation?: boolean): Animation;
to(property: string, value: any): Animation;
fromTo(property: string, fromValue: any, toValue: any): Animation;
keyframes(keyframes: any[]): Animation;

addAnimation(animationToADd: Animation | Animation[] | undefined | null): Animation;
Expand Down
2 changes: 1 addition & 1 deletion core/src/utils/animation/animation-utils.ts
Expand Up @@ -43,7 +43,7 @@ export const generateKeyframeRules = (keyframes: any[] = []) => {

const frameString = [];
for (const property in keyframe) {
if (keyframe.hasOwnProperty(property) && property !== 'offset' && property !== 'clear') {
if (keyframe.hasOwnProperty(property) && property !== 'offset') {
frameString.push(`${property}: ${keyframe[property]};`);
}
}
Expand Down
79 changes: 7 additions & 72 deletions core/src/utils/animation/animation.ts
Expand Up @@ -64,6 +64,8 @@ export const createAnimation = () => {

elements.length = 0;
childAnimations.length = 0;
_keyframes.length = 0;

clearOnFinish();

initialized = false;
Expand Down Expand Up @@ -1033,98 +1035,31 @@ export const createAnimation = () => {
if (firstFrame != null && (firstFrame.offset === undefined || firstFrame.offset === 0)) {
firstFrame[property] = value;
} else {
const object: any = {
offset: 0
};
object[property] = value;

_keyframes = [
object,
{ offset: 0, [property]: value },
..._keyframes
];
}

return ani;
};

const to = (property: string, value: any, clearAfterAnimation?: boolean) => {
const to = (property: string, value: any) => {
const lastFrame = _keyframes[_keyframes.length - 1];

if (lastFrame != null && (lastFrame.offset === undefined || lastFrame.offset === 1)) {

/**
* If last frame is not the clear frame
* set the value as usual
*/
if (!lastFrame.clear) {
lastFrame[property] = value;
} else {
/**
* If last frame is the keyframe then we need to
* set the value on the previous frame
*/
const secondToLastFrame = _keyframes[_keyframes.length - 2];
secondToLastFrame[property] = value;
}

/**
* If not on the last possible keyframe, add
* the last keyframe
*/
lastFrame[property] = value;
} else {
_keyframes = [
..._keyframes,
{ offset: 1, [property]: value }
];
}

if (clearAfterAnimation) {
const lastFrame = _keyframes[_keyframes.length - 1];
if (lastFrame != null) {

/**
* If we are on the clear frame, just set the property
*/
if (lastFrame.clear) {
lastFrame[property] = '';
return ani;
}

/**
* If we are already setup for a clear frame, just mark it
* as such and set the property
*/
const secondToLastFrame = _keyframes[_keyframes.length - 2];
if (lastFrame.offset === 1 && secondToLastFrame.offset === 0.99) {
lastFrame.clear = true;
lastFrame[property] = '';
secondToLastFrame[property] = value;
return ani;
}

/**
* If the last frame is not the clear frame
* and has an offset of 1, we need to move it
* back by a frame to account for the clear frame
*/
if (lastFrame.offset === 1 && secondToLastFrame.offset !== 0.99) {
lastFrame.offset = 0.99;
}

/**
* Add a clear frame that runs immediately after
* the last frame that the user has set. This will
* allow users to clear certain properties from elements
*/
_keyframes.push({ offset: lastFrame.offset + 0.01, [property]: '', clear: true });
}
}

return ani;
};

const fromTo = (property: string, fromValue: any, toValue: any, clearAfterAnimation?: boolean) => {
return from(property, fromValue).to(property, toValue, clearAfterAnimation);
const fromTo = (property: string, fromValue: any, toValue: any) => {
return from(property, fromValue).to(property, toValue);
};

return ani = {
Expand Down
39 changes: 3 additions & 36 deletions core/src/utils/animation/test/animation.spec.ts
Expand Up @@ -114,37 +114,6 @@ describe('Animation Class', () => {
offset: 1
});
});

it('should clear properties at the end of an animation', () => {
animation
.fromTo('opacity', 0, 1, true)
.fromTo('background', 'red', 'blue')
.fromTo('color', 'purple', 'green', true);

const keyframes = animation.getKeyframes();

expect(keyframes.length).toEqual(3);
expect(keyframes[0]).toEqual({
opacity: 0,
color: 'purple',
background: 'red',
offset: 0
});

expect(keyframes[1]).toEqual({
opacity: 1,
color: 'green',
offset: 0.99,
background: 'blue'
});

expect(keyframes[2]).toEqual({
clear: true,
opacity: '',
color: '',
offset: 1
})
});

it('should mix keyframes and fromTo properly', () => {
animation
Expand All @@ -153,7 +122,7 @@ describe('Animation Class', () => {
{ offset: 0.99, background: 'blue' },
{ offset: 1, background: 'green' }
])
.fromTo('opacity', 0, 1, true)
.fromTo('opacity', 0, 1)

const keyframes = animation.getKeyframes();
expect(keyframes.length).toEqual(3);
Expand All @@ -164,16 +133,14 @@ describe('Animation Class', () => {
});

expect(keyframes[1]).toEqual({
opacity: 1,
background: 'blue',
offset: 0.99
});

expect(keyframes[2]).toEqual({
opacity: '',
opacity: 1,
background: 'green',
offset: 1,
clear: true
offset: 1
});
});
});
Expand Down
7 changes: 6 additions & 1 deletion core/src/utils/transition/ios.transition.ts
Expand Up @@ -138,7 +138,12 @@ export const iosTransitionAnimation = (navEl: HTMLElement, opts: TransitionOptio

enteringToolBarBg
.beforeClearStyles([OPACITY])
.fromTo(OPACITY, 0.01, 1, true);
.keyframes([
{ offset: 0, opacity: 0.01 },
{ offset: 0.99, opacity: 1 },
{ offset: 1, opacity: 'var(--opacity)' }
// TODO: Find a way to support clearing properties from Web Animations
]);

// forward direction, entering page has a back button
enteringBackButton.fromTo(OPACITY, 0.01, 1);
Expand Down

0 comments on commit fa958a5

Please sign in to comment.