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.

Sign up for GitHub

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

Jump to bottom

fix: save table column order on setAll #200

Merged
merged 4 commits into from
Jun 29, 2023
Merged

fix: save table column order on setAll #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35480ee
fix: save table column order on setAll
joelamb Jun 20, 2023
ca3255b
test: set all persists column order to preferences
joelamb Jun 21, 2023
9d07304
chore: update coloumn reordering docs with preferences example
joelamb Jun 23, 2023
ced9ca3
Create wet-melons-hunt.md
joelamb Jun 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/wet-melons-hunt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"ember-headless-table": patch
---

fix: save table column order on setAll
14 changes: 14 additions & 0 deletions docs/plugins/column-reordering/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ None

The order of columns will be represented in the preferences.

```js
"ColumnReordering": {
"columns": {},
"table": {
"order": {
"A": 1,
"B": 2,
"C": 3,
"D": 4
}
}
}
```

### Accessibility

It's recommended to use `<button>`s for changing the order of columns.
Expand Down
2 changes: 2 additions & 0 deletions ember-headless-table/src/plugins/column-reordering/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,8 @@ export class ColumnOrder {
for (let [key, value] of map.entries()) {
this.map.set(key, value);
}

this.args.save?.(map);
joelamb marked this conversation as resolved.
Show resolved Hide resolved
};

/**
Expand Down
56 changes: 52 additions & 4 deletions test-app/tests/plugins/column-reordering/rendering-test.gts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';

import { headlessTable } from 'ember-headless-table';
import { meta, columns } from 'ember-headless-table/plugins';
import { ColumnReordering, moveLeft, moveRight } from 'ember-headless-table/plugins/column-reordering';
import { columns } from 'ember-headless-table/plugins';
import { ColumnOrder, ColumnReordering, moveLeft, moveRight, setColumnOrder } from 'ember-headless-table/plugins/column-reordering';
import { ColumnVisibility, hide, show } from 'ember-headless-table/plugins/column-visibility';
import { DATA } from 'test-app/data';

Expand Down Expand Up @@ -449,13 +449,13 @@ module('Plugins | columnReordering', function (hooks) {
});
});

test('changing column order updates preferences', async function (assert) {
test('changing column order with `move left` updates preferences', async function (assert) {
assert.strictEqual(getColumnOrder(), 'B C A D', 'pre-test setup');

await click('th.C .left');
await click('th.A .left');

assert.strictEqual(getColumnOrder(), 'C A B D', 'pre-test setup');
assert.strictEqual(getColumnOrder(), 'C A B D', 'reordered');

assert.deepEqual(preferences, {
"plugins": {
Expand All @@ -482,6 +482,54 @@ module('Plugins | columnReordering', function (hooks) {
}
});
});

test('changing column order with `set all` updates preferences', async function (assert) {
assert.strictEqual(getColumnOrder(), 'B C A D', 'pre-test setup');

let order = new ColumnOrder({
columns: () =>
[
{ key: 'D' },
{ key: 'C' },
{ key: 'B' },
{ key: 'A' },
] as Column[],
existingOrder: new Map([
['A', 3],
['B', 2],
['C', 1],
['D', 0],
]),
});

// @ts-expect-error
setColumnOrder(ctx.table, order);
Comment on lines +505 to +506
Copy link
Contributor

@ynotdraw ynotdraw Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Not being in this repo much:

Out of curiosity - why is this @ts-expect-error required? Are we not supposed to be calling this directly? Adding a comment here for future travelers may be helpful.

Copy link
Contributor Author

@joelamb joelamb Jun 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Without it I get a TS error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setColumnOrder is not usually called directly (it's in the helpers.ts) so we're OK to ignore the TS error here.


assert.deepEqual(preferences, {
"plugins": {
"ColumnReordering": {
"columns": {},
"table": {
"order": {
"A": 3,
"B": 2,
"C": 1,
"D": 0
}
}
},
"ColumnVisibility": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: why is ColumnVisibility here? Was that set up from a previous test?

Copy link
Contributor Author

@joelamb joelamb Jun 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's because the tests in this module are set up with the Column Visibility plugin on line 376, and including that module adds the property to the preferences object

"columns": {
"A": {},
"B": {},
"C": {},
"D": {}
},
"table": {}
}
}
});
});
});

});