Persist column width preferences #211

joelamb · 2023-08-03T16:19:46Z

When columns are resized, the column widths are persisted to the preferences

so that it can be run independently of the other apps

changeset-bot · 2023-08-03T16:19:49Z

🦋 Changeset detected

Latest commit: 26bf7bc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
ember-headless-table	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-08-03T16:24:58Z

Preview URLs

GH Env: preview
docs: https://360b5d5c.ember-headless-table.pages.dev
api docs: https://360b5d5c.ember-headless-table.pages.dev/api/modules.html

joelamb · 2023-08-03T16:29:22Z

ember-headless-table/src/test-support/index.ts

@@ -27,7 +27,11 @@ export function createHelpers(selectors: Selectors) {
    let targetX = startX + delta;

    triggerEvent(element, 'mousedown', { clientX: startX, button: 0 });
+    await new Promise((resolve) => setTimeout(resolve, 100));


this ensures that the mouse down, drag and release events don't all happen in the same runloop

isn't that what happens IRL, tho?

without these pauses the mouseup trigger event fire before the new column-width data is available to be saved to preferences. IRL no-one drags a column that quickly.

200ms is also quite long, and a big delay. most tests on my machine would take way longer with this change.

Does a settled or something similar work or no?

@ynotdraw settled didn't work. That's what I tried first.

@NullVoxPopuli I can get tests to pass with a single 50ms delay (tests fail even with a 10ms delay).

Interesting! Do you know what's goin on within those 50ms?
Like, what are we waiting for?

The inline style width property of the table header cells of the two affected columns are being calculated and added to the DOM

I bet we're missing some test waiters.

await settled should work, if possible.

Anywho, thanks for appeasing my questions!
I'm happy with things, given that this is a separate task to do

Logged the issue here: #215

NullVoxPopuli · 2023-08-03T17:01:49Z

ember-headless-table/src/plugins/-private/base.ts

@@ -262,6 +262,7 @@ function columnsFor<DataType = any>(



This method upsets me. lol (I wrote it)

It was meant to be temporary.

I need to see how TanStack Table handles their column dependency calculations

NullVoxPopuli · 2023-08-03T17:06:55Z

ember-headless-table/src/plugins/column-resizing/plugin.ts

+
+      let colPreferences = preferences.forColumn(columnToUpdate, ColumnResizing);
+
+      colPreferences.set('width', column.width.toString());


should this width be a percentage so that when the table is loaded on a different sized browser, the columns have the same way of being sized?

I could see the argument either way.
Though, with fixed-column sizes across dynamic-sized browsers / devices, I'd think we'd only want to save the width of the columns that have specifically been adjusted by the user, so that dynamic widths can still be flexible.

like,

| set width | set width | dynamic width | dynamic width | dynamic width | set width |

NullVoxPopuli

Conditionally approved~ish

my only concern is the arbitrary delay in test-support, as this could more than double the test time of every test wanting to resize columns.

When resizing drag ends, the updated col widths for all visible columns are saved to preferences. If preferences exist they are used to set initial column widths.

joelamb · 2023-08-04T13:19:31Z

Conditionally approved~ish

my only concern is the arbitrary delay in test-support, as this could more than double the test time of every test wanting to resize columns.

Tests reliably pass with just a single 50ms delay between the drag and mouse up - but will not pass with a shorter delay (or no delay)

ynotdraw

Similar to @NullVoxPopuli, going to approve, but it would be nice if we could get to the bottom of the delays in a follow up at some point.

nicolechung · 2023-08-09T13:44:45Z

ember-headless-table/src/test-support/index.ts

@@ -28,6 +28,9 @@ export function createHelpers(selectors: Selectors) {

    triggerEvent(element, 'mousedown', { clientX: startX, button: 0 });
    triggerEvent(element, 'mousemove', { clientX: targetX, button: 0 });
+
+    await new Promise((resolve) => setTimeout(resolve, 50));


What does this do?

Just curious as it seems like something I could reuse in the future...

It's a workaround that ensures that the calculation of the column width styles are complete and the DOM is updated before the test continues.

It's tracked to fix in #215

but is a necessary workaround for now.

nicolechung · 2023-08-09T13:50:06Z

ember-headless-table/src/plugins/-private/base.ts

@@ -309,6 +314,15 @@ function columnsFor<DataType = any>(
      return visibility.columns;
    }

+    if (sizing) {
+      assert(


Nit: do we need to test for this new assert?

I don't think we currently test for any of the other asserts in this function

nicolechung · 2023-08-09T13:50:31Z

ember-headless-table/src/plugins/column-resizing/plugin.ts

+      return this.options.width;
+    }
+
+    assert('saved width must be a string', typeof savedWidth === 'string');


Nit (non-blocking): do we need to test for this assert?

nicolechung · 2023-08-09T13:50:45Z

ember-headless-table/src/plugins/column-resizing/plugin.ts

+
+  @action
+  save() {
+    this.tableMeta.saveColWidths(this.tableMeta.visibleColumnMetas);


Nit: do we need to test for this save action?

nicolechung · 2023-08-09T13:51:39Z

.changeset/hot-bats-drum.md

+"ember-headless-table": minor
+---
+
+Persists resized column widths to preferences


Nit: should mention the newly added save action and related asserts, i.e. will error if the column width is not a string.

joelamb added 2 commits August 3, 2023 17:00

chore: add volta to docs app package.json

4dec67f

so that it can be run independently of the other apps

test: add column resizing preferences tests

55875d7

github-actions bot deployed to ember-headless-table (Preview) August 3, 2023 16:23 View deployment

joelamb force-pushed the jl-col-width-preferences branch from 6b17586 to ecefa70 Compare August 3, 2023 16:25

github-actions bot deployed to ember-headless-table (Preview) August 3, 2023 16:29 View deployment

joelamb commented Aug 3, 2023

View reviewed changes

NullVoxPopuli reviewed Aug 3, 2023

View reviewed changes

github-actions bot deployed to ember-headless-table (Preview) August 3, 2023 17:06 View deployment

NullVoxPopuli reviewed Aug 3, 2023

View reviewed changes

NullVoxPopuli approved these changes Aug 3, 2023

View reviewed changes

joelamb requested review from ynotdraw and nicolechung August 3, 2023 17:17

joelamb force-pushed the jl-col-width-preferences branch from e02db2a to d83734f Compare August 4, 2023 13:13

feat: persist resized col widths in preferences

7a09878

When resizing drag ends, the updated col widths for all visible columns are saved to preferences. If preferences exist they are used to set initial column widths.

joelamb force-pushed the jl-col-width-preferences branch from d83734f to 7a09878 Compare August 4, 2023 13:15

github-actions bot deployed to ember-headless-table (Preview) August 4, 2023 13:19 View deployment

chore: add changeset

0a9415c

github-actions bot deployed to ember-headless-table (Preview) August 4, 2023 14:32 View deployment

ynotdraw approved these changes Aug 4, 2023

View reviewed changes

joelamb requested a review from clintcs August 7, 2023 08:12

joelamb mentioned this pull request Aug 8, 2023

Minimise delay required in column resizing tests #215

Open

nicolechung reviewed Aug 9, 2023

View reviewed changes

nicolechung approved these changes Aug 9, 2023

View reviewed changes

chore: update changelog

26bf7bc

github-actions bot deployed to ember-headless-table (Preview) August 10, 2023 08:52 View deployment

joelamb merged commit c3a3a5c into main Aug 10, 2023
25 checks passed

joelamb deleted the jl-col-width-preferences branch August 10, 2023 08:58

github-actions bot mentioned this pull request Aug 10, 2023

Version Packages #216

Merged

joelamb mentioned this pull request Sep 19, 2023

Resize preference save sequence #233

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persist column width preferences #211

Persist column width preferences #211

joelamb commented Aug 3, 2023

changeset-bot bot commented Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 3, 2023 •

edited

Loading

joelamb Aug 3, 2023

NullVoxPopuli Aug 3, 2023

joelamb Aug 3, 2023

NullVoxPopuli Aug 3, 2023 •

edited

Loading

ynotdraw Aug 3, 2023

joelamb Aug 4, 2023

NullVoxPopuli Aug 4, 2023

joelamb Aug 4, 2023

NullVoxPopuli Aug 4, 2023

joelamb Aug 8, 2023

NullVoxPopuli Aug 3, 2023

NullVoxPopuli Aug 3, 2023

NullVoxPopuli left a comment

joelamb commented Aug 4, 2023

ynotdraw left a comment

nicolechung Aug 9, 2023

nicolechung Aug 9, 2023

joelamb Aug 9, 2023

nicolechung Aug 9, 2023

joelamb Aug 9, 2023

nicolechung Aug 9, 2023

nicolechung Aug 9, 2023

nicolechung Aug 9, 2023


		let colPreferences = preferences.forColumn(columnToUpdate, ColumnResizing);

		colPreferences.set('width', column.width.toString());

Persist column width preferences #211

Persist column width preferences #211

Conversation

joelamb commented Aug 3, 2023

changeset-bot bot commented Aug 3, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Aug 3, 2023 • edited Loading

Preview URLs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NullVoxPopuli Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NullVoxPopuli left a comment

Choose a reason for hiding this comment

joelamb commented Aug 4, 2023

ynotdraw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 3, 2023 •

edited

Loading

NullVoxPopuli Aug 3, 2023 •

edited

Loading