Skip to content

Commit

Permalink
trying to replace observers with lifecycle hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakesjews
jakesjews committed Apr 5, 2016
1 parent 5c36aaa commit 54bcb0f
Showing 1 changed file with 27 additions and 43 deletions.
70 changes: 27 additions & 43 deletions addon/components/x-toggle/component.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Ember from 'ember';
import layout from './template';

const { on, run, computed, observer } = Ember;
const { computed } = Ember;

export default Ember.Component.extend({
layout: layout,
Expand Down Expand Up @@ -37,57 +37,41 @@ export default Ember.Component.extend({
return this.get('elementId') + '-x-toggle';
}),

wasToggled: on('init', observer('toggled', function () {
var toggled = this.get('toggled');
didUpdateAttrs: function({newAttrs, oldAttrs}) {

This comment has been minimized.

Sign in to view

Copy link
@knownasilya

knownasilya Apr 5, 2016

👍
if (!oldAttrs) {
oldAttrs = {};
}

var offIndex = this.get('offLabel').indexOf(':');
var onIndex = this.get('onLabel').indexOf(':');
var offState = offIndex > -1 ? this.get('offLabel').substr(offIndex + 1) : false;
var onState = onIndex > -1 ? this.get('onLabel').substr(onIndex + 1) : true;

this.sendAction('toggle', toggled, this.get('name'));

if (toggled === false) {
this.set('value', offState);
} else {
this.set('value', onState);
}
})),

valueObserver: on('init', observer('value', function() {
var debounce = this.get('debounce');

if (!debounce) {
debounce = run.debounce(this, function () {
var value = this.get('value');
var offIndex = this.get('offLabel').indexOf(':');
var onIndex = this.get('onLabel').indexOf(':');
var offState = offIndex > -1 ? this.get('offLabel').substr(offIndex + 1) : false;
var onState = onIndex > -1 ? this.get('onLabel').substr(onIndex + 1) : true;

if (value === onState) {
this.set('toggled', true);
} else {
this.set('toggled', false);
this.set('value', offState);
}

this.set('debounce', null);
}, 500);

this.set('debounce', debounce);
if (newAttrs.toggled !== oldAttrs.toggled) {
if (newAttrs.toggled === false) {
newAttrs.value = offState;
} else {
newAttrs.value = onState;
}
} else if (newAttrs.value !== oldAttrs.value) {
if (newAttrs.value === onState) {
newAttrs.toggled = true;
} else {
newAttrs.toggled = false;
newAttrs.value = offState;
}
}
})),

clearDebounce: on('willDestroyElement', function () {
var debounce = this.get('debounce');

if (debounce) {
run.cancel(debounce);
this.set('debounce', null);
}
}),
this._super(...arguments);
},

click(event) {
event.stopPropagation();
},

actions: {
checkedChanged: function(e) {
this.sendAction('toggle', e);
}
}
});

3 comments on commit 54bcb0f

@jakesjews
Copy link
Member Author

Choose a reason for hiding this comment

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

@knownasilya ha that was quick! I'm mostly experimenting right now with this in our app but if it goes well there should be a PR pretty soon.

@knownasilya
Copy link

Choose a reason for hiding this comment

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

Looking forward to that PR! Really appreciate you taking on some of these changes.

@jakesjews
Copy link
Member Author

Choose a reason for hiding this comment

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

@knownasilya How would you feel about converting to use the data down action up paradigm? I'm finding that using the lifeCycleHooks is causing a lot of trouble with the older style mutable bindings.

The downside to this is that it would be a breaking change since it would require using the mut helper {{x-toggle toggle=(action (mut "toggledProp"))}} to maintain two way bindings.

The other issue is non-boolean value bindings. Two options could be a separate event like valueChanged or just allowing users to handle it in an action like

toggle: function(val) {
  var newVal = val ? 'on' : 'off'; 
  this.set('somethingElse', newVal);
}

I am also open to trying to keep the existing API working but this issue is making it difficult.

Please sign in to comment.