Skip to content
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

Table: add setRowsSynced method #451

Merged

Conversation

buschtoens
Copy link
Collaborator

Fixes #414.

The tests assert that:

  • the table is initialized with a synced array
  • the synced array may be replaced
  • the replacement array is correctly synced
  • mutating the replaced array has no effect

Theses tests assert that:

- the table is initialized with a synced array
- the synced array may be replaced
- the replacement array is correctly synced
- mutating the replaced array has no effect
@buschtoens
Copy link
Collaborator Author

While we're at it: do the row mutation methods (addRow, insertRowAt, removeRow, ...) all work as intended when { enableSync: true } or (if not) are they intended to do so? Should they delegate to the proxied array or throw?

#414 (comment)

Fun fact: they delegate to the synced array. It may be noteworthy that setRows does not destroy or replace the binding. Instead the synced array's elements are replaced with the given rows.

setRowsSynced(rows = [], options = {}) {
let _rows = RowSyncArrayProxy.create({
syncArray: rows,
content: emberArray(Table.createRows(rows, options))
Copy link
Collaborator

Choose a reason for hiding this comment

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

slightly tangential question: you wouldn't happen to know if there's any downside to https://github.com/buschtoens/ember-light-table/blob/master/addon/classes/Table.js#L155 doesn't pass in the options to Table.createRows()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Shoot, I forgot to update the @method tag.

Good question... We could add options.rowOptions in the constructor and pass that. If users require options to be passed right now, they should be able to wrap in Table.createRows(rows, options) methinks.

@buschtoens buschtoens merged commit a6fa993 into adopted-ember-addons:master Jul 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants