-
Notifications
You must be signed in to change notification settings - Fork 354
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
Header & row action #428
Header & row action #428
Conversation
billy-addepar
commented
Oct 4, 2017
- Adds support for sending action from table header / row.
addon/components/ember-table-row.js
Outdated
const tableObject = this.get('row.api.targetObject'); | ||
tableObject.send('onRowClicked', event, this.get('rowIndex')); | ||
|
||
this.sendAction('onRowClicked', event, this.get('rowIndex'), this.get('rowValue')); |
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.
There are 2 actions sent in this case: one for table for row selection. The other is for outer controller
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.
Overall looks good, just a couple event names should be made simpler but I think this is solid
addon/components/ember-table-row.js
Outdated
const tableObject = this.get('row.api.targetObject'); | ||
tableObject.send('onRowClicked', event, this.get('rowIndex')); | ||
|
||
this.sendAction('onRowClicked', event, this.get('rowIndex'), this.get('rowValue')); |
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.
I think onRowClicked
here is a bit verbose, from the user standpoint they already know they're passing in actions to the row. Let's do onClick
instead
addon/components/ember-table-row.js
Outdated
} | ||
|
||
doubleClick(event) { | ||
this.sendAction('onRowDoubleClicked', event, this.get('rowIndex'), this.get('rowValue')); |
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.
same here, onDoubleClick
addon/components/ember-table-row.js
Outdated
@@ -77,12 +77,12 @@ export default class EmberTableRow extends Component { | |||
|
|||
click(event) { | |||
const tableObject = this.get('row.api.targetObject'); | |||
tableObject.send('onRowClicked', event, this.get('rowIndex')); | |||
tableObject.send('onClick', event, this.get('rowIndex')); |
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.
This is different because we are not sending this action externally. Really, if the table were using the row component directly, it would look like:
{{#ember-table as |row|}}
{{ember-table-row onClick="onRowClicked"}}
{{/ember-table}}
So this action and the action on the table should remain onRowClicked