WebAnimations Meta by tomdye · Pull Request #713 · dojo/widget-core

tomdye · 2017-10-09T15:28:55Z

Type: feature

The following has been addressed in the PR:

There is a related issue
All code matches the style guide
Unit or Functional tests are included in the PR

Description:
Adds a WebAnimations meta which allows programatic web animations to be applied to a VNode.

import WebAnimation from '@dojo/widget-core/meta/WebAnimation';

export default class AnimatedWidget extends WidgetBase {
    protected render() {
        const animate = {
            id: 'rootAnimation',
            effects: [
                { height: '10px' },
                { height: '100px' }
            ],
            controls: {
                play: true
            }
        };

        this.meta(WebAnimation).animate('root', animate);

        return v('div', {
            key: 'root'
        });
    }
}

Resolves #657

agubler

couple of very minor nits and questions

agubler · 2017-10-31T17:54:07Z

+        const { scroll } = this.meta(Dimensions).get('content');
+
+        return [
+            { height: `0px` },


nit: we don't need ` here, just '

agubler · 2017-10-31T17:54:27Z

+import Map from '@dojo/shim/Map';
+import MetaBase from '../meta/Base';
+
+declare const KeyframeEffect: any;


Can we not type these?

agubler · 2017-10-31T17:59:19Z

+				(node: HNode) => {
+					const { animate, key } = node.properties;
+					if (animate && key) {
+						this.meta(AnimationPlayer).add(key as string, animate, this);


any reason we can't bind the controls in here rather than in the meta?

we can but it's potentially an array of arrays so wanted to avoid the extra loops. Can do though if you feel it's better?

agubler · 2017-10-31T17:59:59Z

+			decorate(result,
+				(node: HNode) => {
+					const { animate, key } = node.properties;
+					if (animate && key) {


Do you think we should warn if animate is passed without a key, might be surprising if it doesn't work?

tomdye · 2017-11-03T15:28:16Z

@agubler ready for another review

agubler · 2017-11-06T15:44:35Z

 export interface WidgetMetaProperties {
 	invalidate: () => void;
 	nodeHandler: NodeHandlerInterface;
+	bind: any;


Is this really any?

the bind scope is any in widgetBase I believe? I guess it should probably be WidgetBaseConstructor ?

It wouldn't be the constructor, it would just be WidgetBase if it is able to be typed.

agubler · 2017-11-06T15:47:35Z

+	}
+
+	afterRender() {
+		super.afterRender();


why super.afterRender()?

Is this not how you call the super function?

agubler · 2017-11-06T18:55:12Z

package-lock.json conflict now

tomdye · 2017-11-07T13:35:54Z

@agubler all done, please re-review

kitsonk requested review from agubler and matt-gadd October 9, 2017 15:33

kitsonk added the beta4 label Oct 9, 2017

dylans added this to the 2017.10 milestone Oct 10, 2017

tomdye force-pushed the animations branch from 9a63998 to 861040e Compare October 19, 2017 15:27

kitsonk modified the milestones: 2017.10, beta.4 Oct 30, 2017

tomdye force-pushed the animations branch 3 times, most recently from afcaa60 to 59867fb Compare October 31, 2017 16:46

tomdye changed the title ~~AnimatableMixin~~ AnimatedMixin Oct 31, 2017

agubler reviewed Oct 31, 2017

View reviewed changes

tomdye force-pushed the animations branch 2 times, most recently from 8ffa0b6 to 26ecd13 Compare November 2, 2017 14:16

tomdye changed the title ~~AnimatedMixin~~ WebAnimations Meta Nov 2, 2017

tomdye force-pushed the animations branch from a4d075d to 76713d2 Compare November 3, 2017 15:08

agubler reviewed Nov 6, 2017

View reviewed changes

tomdye added 10 commits November 7, 2017 12:01

add animatable mixin

4685961

startign to add tests

f85b652

starting tests

71f1b26

added tests

39d41b3

coverage

b52717a

bumped test coverage

787c418

reinstated tests

577f895

added typings for animate

17abf43

changed to bdd style

0842ec8

adding animations readme info

8c0d3da

tomdye added 21 commits November 7, 2017 12:01

fix headings

1a1ab07

add index

da7d027

add index

4ce0f74

auto bind onFinish and onCancel

bfac7b7

fix animatable

75e2b7b

add package lock again

7bf0993

changed to animated

d9b3cae

fixed test

2b734c6

pr comments

be94fc5

moved animated to meta

c0566c5

reinstated tests

24041c8

fix readme

9197e86

fix functional test json

fdfea28

rename add to animate

eb06216

adding info test

13c5136

added tests for get info

47aac32

change type

e156912

update readme, change interface

97d0e96

remove super call

57a721c

updated readme and bind type

48bef93

updated package lock

f28f82b

tomdye force-pushed the animations branch from 6330753 to f28f82b Compare November 7, 2017 12:03

tomdye added 2 commits November 7, 2017 12:22

make bind optional

aa37df9

reinstate type, add instances to tests

3f98375

agubler approved these changes Nov 7, 2017

View reviewed changes

tomdye merged commit 7267224 into dojo:master Nov 7, 2017

Uh oh!

Conversation

tomdye commented Oct 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agubler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomdye commented Nov 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agubler commented Nov 6, 2017

Uh oh!

tomdye commented Nov 7, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tomdye commented Oct 9, 2017 •

edited

Loading