-
-
Notifications
You must be signed in to change notification settings - Fork 131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add better component example #153
Add better component example #153
Conversation
@Gorzas I dont believe that replacing |
Thanks for checking my PR @offirgolan. I didn't replace |
@Gorzas How about something like this instead: export default Ember.Component.extend({
model: null,
columns: computed(function() {
return [{
label: 'Avatar',
valuePath: 'avatar',
width: '60px',
sortable: false,
cellComponent: 'user-avatar'
}, {
label: 'First Name',
valuePath: 'firstName',
width: '150px'
}, {
label: 'Last Name',
valuePath: 'lastName',
width: '150px'
}, {
label: 'Address',
valuePath: 'address'
}, {
label: 'State',
valuePath: 'state'
}, {
label: 'Country',
valuePath: 'country'
}];
}),
table: computed('model.[]', function() {
/*
Set the table rows when our model changes
*/
this.get('_table').setRows(this.get('model'));
return this.get('_table');
}),
init() {
this._super(...arguments);
/*
Create our table instance
*/
this.set('_table', new Table(this.get('columns')));
}
}); Not sure I like using In the future, we will implement a way of automatically doing this so there wont be a need for any of this. |
…tion without ctrl/cmd (adopted-ember-addons#160) shift-click to select multiple intervening rows works as always
…-light-table into feature/152-better-examples
@@ -90,10 +90,19 @@ export default Ember.Component.extend({ | |||
}]; | |||
}), | |||
|
|||
table: computed('model.[]', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Computed properties are not supposed to have side effects. If you're going to do this, then you might as well use an observer.
@offirgolan I have changed the example again, you can take a look. It seems that we may include two examples instead of only one. What do you think? Note: I have change my commit with a rebase, I don't know if this way is better or if it's better to include an extra commit instead. |
Close #152