Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Add reaction callback to custom diffProperty #586

Merged
merged 6 commits into from
Jun 23, 2017

Conversation

agubler
Copy link
Member

@agubler agubler commented Jun 22, 2017

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:

This PR changes the diffProperty decorator to add the concept of a reaction callback that is registered by using the decorator at a class method level.

Diff functions are now passed via the second argument of the decorator and are required to pure functions with no side affects.

@diffProperty('foo', diffFunction)
class MyWidget extends WidgetBase {}

When the decorator is used at a method level it will register the function as the reaction callback that is called when one or more of the defined properties are considered changed. The function receives the previous and new properties for the registered keys.

class MyWidget extends WidgetBase {
    @diffProperty('foo', diffFunction)
    protected onFooChanged(previousProperties: any, newProperties: any): void {
        // do something when foo is considered changed by the function passed i.e. `diffFunction`
        // the previous and new foo value is available on the function arguments.
    }
}

This is the function that should be used to apply custom logic based on property changes and replaced the existing onPropertiesChanged catch all decorator.

Note: onPropertiesChanged decorator has been removed as part of these changes.

Resolves #579

Copy link
Member

@kitsonk kitsonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good and I like the whole change. Seems worth taking the breaking change on.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants