Skip to content

Commit

Permalink
x-select: use change() to handle changes from DOM
Browse files Browse the repository at this point in the history
Ember views automatically call `.change()` on DOM change events, so
we can use that hook instead of adding a change handler in `didInsertElement`
and tearing it down in `willDestroyElement`.

This is now the only place we need to call `_updateValueSingle` or
`_updateValueMultiple`. We don't need to do that when the content changes
from upstream since (in Glimmer) the component will render the difference
automatically. Calling those in an observer on `content.@each` was causing
upstream writes to happen unnecessarily.
  • Loading branch information
James A. Rosen committed Jun 28, 2015
1 parent 81211fa commit 539a041
Showing 1 changed file with 6 additions and 36 deletions.
42 changes: 6 additions & 36 deletions addon/components/x-select.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,36 +130,18 @@ export default Ember.Component.extend({
}),

/**
* Listen for change events on the native <select> element, which
* indicates that the user has chosen a new option from the
* dropdown. If there is an associated `x-option` component that is
* selected, then the overall value of `x-select` becomes the value
* of that option.
*
* @override
* When the select DOM event fires on the element, trigger the
* component's action with the current value.
*/
didInsertElement: function() {
this._super.apply(this, arguments);

this.$().on('change', Ember.run.bind(this, function() {
this._contentDidChange();
}));
},

/**
* Triggers an update of the selected options.
* Observing `content` ensures that if an element is removed that
* is also part of the selection, selection is cleared.
*
* @private
*/
_contentDidChange: Ember.observer('content.@each', function() {
change() {
if (this.get('multiple')) {
this._updateValueMultiple();
} else {
this._updateValueSingle();
}
}),

this.sendAction('action', this.get('value'), this);
},

/**
* Updates `value` with the object associated with the selected option tag
Expand Down Expand Up @@ -195,7 +177,6 @@ export default Ember.Component.extend({
} else {
this.set('value', newValues);
}
this.raiseAction();
},

/**
Expand All @@ -204,21 +185,10 @@ export default Ember.Component.extend({
willDestroyElement: function() {
this._super.apply(this, arguments);

this.$().off('change');
// might be overkill, but make sure options can get gc'd
this.get('options').clear();
},

/**
* XSelect supports both two-way binding as well as an action. Observe the
* `value` property, and when it changes, raise that as an action.
*
* @private
*/
raiseAction: Ember.observer('value', function() {
this.sendAction('action', this.get('value'), this);
}),

/**
* If this is a multi-select, and the value is not an array, that
* probably indicates a misconfiguration somewhere, so we error out.
Expand Down

0 comments on commit 539a041

Please sign in to comment.