Skip to content

Commit

Permalink
fix: Ensure column cell values update when columns are reordered (#878)
Browse files Browse the repository at this point in the history
Fixes #808
Fixes #820

The idea behind this is basically the same as #567.

The value of `cellValue` is computed by grabbing the `columnValue.valuePath`
and then grabbing the value of the corresponding key on `rowValue`.
Basically something like `rowValue.${columnValue.valuePath}`, which is
not possible using any of the built in computed macros.

This basically uses an observer to watch for changes to
`columnValue.valuePath`, and defines/redefines `cellValue` as a computed
alias for the property at that path on `rowValue`. This ensures the
`cellValue` is correct if `columnValue.valuePath` or the actual value at
that path on `rowValue` changes. Observers aren't recommended but this
was already using one.

Previously this was solved by creating a more generic `dynamicAlias`
computed macro in #567. To be
honest, I was having a little trouble wrapping my head around all that
was happening in there but I think the changes in this PR accomplish the
same idea.

I'm not totally sure what the issue was with the other implementation
but, since the `dynamicAlias` macro was only used in this one place, it
feels OK to have a more simple implementation that is hard coded
specifically for this case.

Co-authored-by: Eric Kelly <HeroicEric@users.noreply.github.com>
  • Loading branch information
HeroicEric and HeroicEric authored Mar 17, 2021
1 parent eec0594 commit 7a05456
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 127 deletions.
120 changes: 0 additions & 120 deletions addon/-private/utils/computed.js

This file was deleted.

15 changes: 12 additions & 3 deletions addon/components/-private/row-wrapper.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
import Component from '@ember/component';
import hbs from 'htmlbars-inline-precompile';

import EmberObject, { get, setProperties, computed } from '@ember/object';
import EmberObject, { get, setProperties, computed, defineProperty } from '@ember/object';
import { alias } from '@ember/object/computed';
import { A as emberA } from '@ember/array';

import { notifyPropertyChange } from '../../-private/utils/ember';
import { objectAt } from '../../-private/utils/array';
import { dynamicAlias } from '../../-private/utils/computed';
import { observer } from '../../-private/utils/observer';

const CellWrapper = EmberObject.extend({
cellValue: dynamicAlias('rowValue', 'columnValue.valuePath'),
/* eslint-disable-next-line ember/no-observers, ember-best-practices/no-observers */
columnValueValuePathDidChange: observer('columnValue.valuePath', function() {
let columnValuePath = get(this, 'columnValue.valuePath');
let cellValue = columnValuePath ? alias(`rowValue.${columnValuePath}`) : null;

defineProperty(this, 'cellValue', cellValue);
notifyPropertyChange(this, 'cellValue');
}),

cellMeta: computed('rowMeta', 'columnValue', function() {
let rowMeta = get(this, 'rowMeta');
Expand Down
36 changes: 32 additions & 4 deletions tests/integration/components/headers/reorder-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,40 @@ module('Integration | headers | reorder', function() {
await generateTable(this);

await table.headers.objectAt(0).reorderBy(1);
assert.equal(table.headers.objectAt(0).text.trim(), 'B', 'First column is swapped forward');
assert.equal(table.headers.objectAt(1).text.trim(), 'A', 'Second column is swapped backward');
assert.equal(
table.headers.objectAt(0).text.trim(),
'B',
'First column header is swapped forward'
);
assert.equal(table.getCell(0, 0).text.trim(), '0B', 'First column cells are swapped forward');
assert.equal(
table.headers.objectAt(1).text.trim(),
'A',
'Second column header is swapped backward'
);
assert.equal(
table.getCell(0, 1).text.trim(),
'0A',
'Second column cells are swapped forward'
);

await table.headers.objectAt(1).reorderBy(-1);
assert.equal(table.headers.objectAt(1).text.trim(), 'B', 'Second column is swapped backward');
assert.equal(table.headers.objectAt(0).text.trim(), 'A', 'First column is swapped forward');
assert.equal(
table.headers.objectAt(1).text.trim(),
'B',
'Second column header is swapped backward'
);
assert.equal(
table.getCell(0, 1).text.trim(),
'0B',
'Second column cells are swapped forward'
);
assert.equal(
table.headers.objectAt(0).text.trim(),
'A',
'First column header is swapped forward'
);
assert.equal(table.getCell(0, 0).text.trim(), '0A', 'First column cells are swapped forward');
});

test('column reorder action is sent up to controller', async function(assert) {
Expand Down

0 comments on commit 7a05456

Please sign in to comment.