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
Datatable sync #516
Datatable sync #516
Conversation
src/components/tables/DataTable.js
Outdated
desc: this.desc | ||
}) | ||
isSelected (item) { | ||
return this.selected && this.selected.find(i => i[this.selectedKey] === item[this.selectedKey]) | ||
}, | ||
sort (index) { |
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 that this method could be cleaned up a bit. The same call is made for every scenario, the only thing that changes is the object that is emitted. Assign the value of that object and only have one this.$emit.
src/components/tables/DataTable.js
Outdated
@@ -115,36 +105,51 @@ export default { | |||
type: Boolean, | |||
default: false | |||
}, | |||
defaultSort: { | |||
selected: { |
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 we should let the selected be controlled under the model and the items be under a prop items, and just sync that. Would be curious to hear others thoughts.
v-model="yourSelectedRows"
:items.sync="tableItems"
src/components/tables/DataTable.js
Outdated
this.sorting = this.defaultSort ? this.defaultSort.field : header.value | ||
const firstSortable = this.headers.find(h => !('sortable' in h) || h.sortable) | ||
|
||
let defaultPagination = { |
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.
should be const as it doesn't change value, could also just put it into the Object.assign arguments.
Why default sort is part of pagination? Let's locate it in headers maybe? |
@avengerweb It's part of the synced prop so that sorting can be controlled outside of the data table. The end result is that it can also be used to set the default sorted column. Placing an option in the headers as well duplicates functionality which already exists. |
src/components/tables/DataTable.js
Outdated
@@ -3,7 +3,7 @@ import Body from './mixins/body' | |||
import Foot from './mixins/foot' | |||
import Progress from './mixins/progress' | |||
import { getObjectValueByPath } from '../../util/helpers' | |||
|
|||
import Vue from 'vue' |
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.
What's this guy for?
All reviewers have until Sunday to provide feedback. |
Made the following changes:
pagination
object prop, and movedpage
,rowsPerPage
,sortBy
anddescending
into it (solves DataTable: changing rows per page mutates a prop, resulting in a warning #498)selectedKey
) (solves Manage selected state in the data-table component instead of in the data itself #400)items
propselected
which can be used inv-checkbox
defaultSort
as pagination prop can be used to same effectThis change allows for external control of pagination, sorting and selection.
Selected example (selectedKey defaults to 'id')
Pagination example (change default sort)