From 6ac95ef47b02bf191103d8cca9d19b350e2a1342 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Thu, 20 Oct 2022 19:12:31 -0400 Subject: [PATCH 1/2] fix(resizing): resizing depends on column order, not just visibility --- docs/plugins/column-resizing/demo/demo-a.md | 2 + .../src/plugins/column-reordering/plugin.ts | 53 +++++++++++++++++++ .../src/plugins/column-resizing/plugin.ts | 14 ++--- 3 files changed, 62 insertions(+), 7 deletions(-) diff --git a/docs/plugins/column-resizing/demo/demo-a.md b/docs/plugins/column-resizing/demo/demo-a.md index 61f4d113..f6edb86e 100644 --- a/docs/plugins/column-resizing/demo/demo-a.md +++ b/docs/plugins/column-resizing/demo/demo-a.md @@ -40,6 +40,7 @@ import { htmlSafe } from '@ember/template'; import { headlessTable } from 'ember-headless-table'; import { meta } from 'ember-headless-table/plugins'; import { ColumnVisibility } from 'ember-headless-table/plugins/column-visibility'; +import { ColumnReordering } from 'ember-headless-table/plugins/column-reordering'; import { ColumnResizing, resizeHandle, isResizing } from 'ember-headless-table/plugins/column-resizing'; import { DATA } from 'docs-app/sample-data'; @@ -62,6 +63,7 @@ export default class extends Component { data: () => DATA, plugins: [ ColumnVisibility, + ColumnReordering, ColumnResizing, ], }); diff --git a/ember-headless-table/src/plugins/column-reordering/plugin.ts b/ember-headless-table/src/plugins/column-reordering/plugin.ts index 6f739ff6..d0d6d849 100644 --- a/ember-headless-table/src/plugins/column-reordering/plugin.ts +++ b/ember-headless-table/src/plugins/column-reordering/plugin.ts @@ -160,6 +160,59 @@ export class TableMeta { return this.columnOrder.orderedColumns; } + previousColumn = (referenceColumn: Column) => { + let visible = this.columns; + let referenceIndex = visible.indexOf(referenceColumn); + + assert( + `index of reference column must be >= 0. column likely not a part of the table`, + referenceIndex >= 0 + ); + + /** + * There can be nothing before the first column + */ + if (referenceIndex === 0) { + return null; + } + + return visible[referenceIndex - 1]; + } + + nextColumn = (referenceColumn: Column) => { + let visible = this.columns; + let referenceIndex = visible.indexOf(referenceColumn); + + assert( + `index of reference column must be >= 0. column likely not a part of the table`, + referenceIndex >= 0 + ); + + /** + * There can be nothing after the last column + */ + if (referenceIndex > visible.length - 1) { + return null; + } + + return visible[referenceIndex + 1]; + } + + columnsAfter = (referenceColumn: Column) => { + let visible = this.columns; + let referenceIndex = visible.indexOf(referenceColumn); + + return visible.slice(referenceIndex + 1); + } + + columnsBefore = (referenceColumn: Column) => { + let visible = this.columns; + let referenceIndex = visible.indexOf(referenceColumn); + + return visible.slice(0, referenceIndex); + } + + /** * @private * This isn't our data to expose, but it is useful to alias diff --git a/ember-headless-table/src/plugins/column-resizing/plugin.ts b/ember-headless-table/src/plugins/column-resizing/plugin.ts index 64321f44..675068bb 100644 --- a/ember-headless-table/src/plugins/column-resizing/plugin.ts +++ b/ember-headless-table/src/plugins/column-resizing/plugin.ts @@ -154,7 +154,7 @@ export class ColumnMeta { } get hasResizeHandle() { - let visiblility = meta.withFeature.forTable(this.column.table, 'columnVisibility'); + let visiblility = meta.withFeature.forTable(this.column.table, 'columnOrder'); let previous = visiblility.previousColumn(this.column); if (!previous) return false; @@ -249,14 +249,14 @@ export class TableMeta { ); } - get visibleColumns() { - let visiblility = meta.withFeature.forTable(this.table, 'columnVisibility'); + get #availableColumns() { + let otherTableMeta = meta.withFeature.forTable(this.table, 'columnOrder'); - return visiblility.visibleColumns; + return otherTableMeta.columns; } get visibleColumnMetas() { - return this.visibleColumns.map((column) => meta.forColumn(column, ColumnResizing)); + return this.#availableColumns.map((column) => meta.forColumn(column, ColumnResizing)); } get totalInitialColumnWidths() { @@ -293,7 +293,7 @@ export class TableMeta { let totalGap = totalGapOf(entry.target.querySelector('[role="row"]')); let diff = this.scrollContainerWidth - this.totalVisibleColumnsWidth - totalGap; - distributeDelta(diff, this.visibleColumns); + distributeDelta(diff, this.#availableColumns); } @action @@ -320,7 +320,7 @@ export class TableMeta { let position = this.options?.handlePosition ?? 'left'; let growingColumn: Column | null | undefined; - let visiblility = meta.withFeature.forTable(this.table, 'columnVisibility'); + let visiblility = meta.withFeature.forTable(this.table, 'columnOrder'); if (position === 'right') { growingColumn = isDraggingRight ? visiblility.nextColumn(column) : column; From bd66b7a7f113ee0863050f887a46b8dabf4b1ed2 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Fri, 21 Oct 2022 15:02:28 -0400 Subject: [PATCH 2/2] chore(tests): add test for preventing regression of the bug --- .../src/plugins/column-reordering/plugin.ts | 9 +- .../column-resizing/rendering-test.gts | 115 ++++++++++++++++-- .../plugins/sticky-columns/rendering-test.gts | 3 +- 3 files changed, 110 insertions(+), 17 deletions(-) diff --git a/ember-headless-table/src/plugins/column-reordering/plugin.ts b/ember-headless-table/src/plugins/column-reordering/plugin.ts index d0d6d849..66d2fb08 100644 --- a/ember-headless-table/src/plugins/column-reordering/plugin.ts +++ b/ember-headless-table/src/plugins/column-reordering/plugin.ts @@ -177,7 +177,7 @@ export class TableMeta { } return visible[referenceIndex - 1]; - } + }; nextColumn = (referenceColumn: Column) => { let visible = this.columns; @@ -196,22 +196,21 @@ export class TableMeta { } return visible[referenceIndex + 1]; - } + }; columnsAfter = (referenceColumn: Column) => { let visible = this.columns; let referenceIndex = visible.indexOf(referenceColumn); return visible.slice(referenceIndex + 1); - } + }; columnsBefore = (referenceColumn: Column) => { let visible = this.columns; let referenceIndex = visible.indexOf(referenceColumn); return visible.slice(0, referenceIndex); - } - + }; /** * @private diff --git a/test-app/tests/plugins/column-resizing/rendering-test.gts b/test-app/tests/plugins/column-resizing/rendering-test.gts index 0b66f13e..6cd188cf 100644 --- a/test-app/tests/plugins/column-resizing/rendering-test.gts +++ b/test-app/tests/plugins/column-resizing/rendering-test.gts @@ -1,17 +1,22 @@ import Component from '@glimmer/component'; import { tracked } from '@glimmer/tracking'; -import { assert } from '@ember/debug'; +import { assert, assert as debugAssert } from '@ember/debug'; import { htmlSafe } from '@ember/template'; -import { findAll, render, settled } from '@ember/test-helpers'; +import { click, findAll, render, settled } from '@ember/test-helpers'; import * as QUnit from 'qunit'; import { module, test, skip } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; +import { setOwner } from '@ember/application'; +// @ts-ignore +import { on } from '@ember/modifier'; +// @ts-ignore +import { fn } from '@ember/helper'; import { headlessTable, type ColumnConfig } from 'ember-headless-table'; import { ColumnResizing, resizeHandle } from 'ember-headless-table/plugins/column-resizing'; import { ColumnVisibility } from 'ember-headless-table/plugins/column-visibility'; +import { ColumnReordering, moveLeft, moveRight } from 'ember-headless-table/plugins/column-reordering'; import { createHelpers, requestAnimationFrameSettled } from 'ember-headless-table/test-support'; -import { setOwner } from '@ember/application'; type Changes = Array<{ value: () => number; by: number; msg?: string }>; @@ -214,7 +219,7 @@ module('Plugins | resizing', function (hooks) { table = headlessTable(this, { columns: () => this.columns, data: () => [] as unknown[], - plugins: [ColumnResizing, ColumnVisibility], + plugins: [ColumnResizing, ColumnReordering, ColumnVisibility], }); } @@ -226,7 +231,6 @@ module('Plugins | resizing', function (hooks) { test('it resizes each column', async function () { ctx.setContainerWidth(1000); await render( - // @ts-ignore @@ -268,7 +272,7 @@ module('Plugins | resizing', function (hooks) { table = headlessTable(this, { columns: () => this.columns, data: () => [] as unknown[], - plugins: [ColumnResizing, ColumnVisibility], + plugins: [ColumnResizing, ColumnReordering, ColumnVisibility], }); } @@ -281,7 +285,6 @@ module('Plugins | resizing', function (hooks) { test('it works', async function () { ctx.setContainerWidth(1000); await render( - // @ts-ignore @@ -327,7 +330,6 @@ module('Plugins | resizing', function (hooks) { ctx.setContainerWidth(1000); await settled(); await render( - // @ts-ignore @@ -369,7 +371,6 @@ module('Plugins | resizing', function (hooks) { test('table & columns resize to fit containing element', async function () { ctx.setContainerWidth(1000); await render( - // @ts-ignore @@ -414,7 +415,6 @@ module('Plugins | resizing', function (hooks) { test('table resizing respects resized columns', async function () { ctx.setContainerWidth(1000); await render( - // @ts-ignore @@ -481,7 +481,6 @@ module('Plugins | resizing', function (hooks) { skip('it works', async function () { ctx.setContainerWidth(1000); await render( - // @ts-ignore @@ -518,4 +517,98 @@ module('Plugins | resizing', function (hooks) { }); }); }); + + module('interaction with other plugins', function () { + module('ColumnReordering', function(hooks) { + class DefaultOptions extends Context { + table = headlessTable(this, { + columns: () => this.columns, + data: () => [] as unknown[], + plugins: [ColumnResizing, ColumnReordering, ColumnVisibility], + }); + } + + hooks.beforeEach(function () { + ctx = new DefaultOptions(); + setOwner(ctx, this.owner); + }); + + test('resizing makes sense regardless of column order', async function (assert) { + ctx.setContainerWidth(1000); + await render( + + ) + + const [columnA, columnB, columnC, columnD] = getColumns(); + + debugAssert(`columnA doesn't exist`, columnA); + debugAssert(`columnB doesn't exist`, columnB); + debugAssert(`columnC doesn't exist`, columnC); + debugAssert(`columnD doesn't exist`, columnD); + + await requestAnimationFrameSettled(); + + const assertSizes = (sizes: Array<[HTMLTableCellElement, number]>) => { + for (let pair of sizes) { + let actual = width(pair[0]); + + assert.strictEqual(actual, pair[1]); + } + } + + assertSizes([ + [columnA, 250], + [columnB, 250], + [columnC, 250], + [columnD, 250], + ]); + + await assertChanges( + () => dragRight(columnB, 50), + [ + { value: () => width(columnA), by: 50, msg: 'width of A increased by 50' }, + { value: () => width(columnB), by: -50, msg: 'width of B decreased by 50' }, + { value: () => width(columnC), by: 0, msg: 'width of C unchanged' }, + { value: () => width(columnD), by: 0, msg: 'width of D unchanged' }, + ] + ); + + assertSizes([ + [columnA, 300], + [columnB, 200], + [columnC, 250], + [columnD, 250], + ]); + + await click('#B-right'); + await requestAnimationFrameSettled(); + + // Sizes don't change + assertSizes([ + [columnA, 300], + [columnB, 200], + [columnC, 250], + [columnD, 250], + ]); + + await assertChanges( + () => dragLeft(columnB, 10), + [ + { value: () => width(columnA), by: 0, msg: 'width of A unchanged' }, + { value: () => width(columnC), by: -10, msg: 'width of C decreased by 10' }, + { value: () => width(columnB), by: 10, msg: 'width of B increased by 10' }, + { value: () => width(columnD), by: 0, msg: 'width of D unchanged' }, + ] + ); + }); + }); + }); }); diff --git a/test-app/tests/plugins/sticky-columns/rendering-test.gts b/test-app/tests/plugins/sticky-columns/rendering-test.gts index 333c3c27..96094a97 100644 --- a/test-app/tests/plugins/sticky-columns/rendering-test.gts +++ b/test-app/tests/plugins/sticky-columns/rendering-test.gts @@ -8,6 +8,7 @@ import { setupRenderingTest } from 'ember-qunit'; import { headlessTable, type ColumnConfig } from 'ember-headless-table'; import { ColumnVisibility } from 'ember-headless-table/plugins/column-visibility'; import { StickyColumns, isSticky } from 'ember-headless-table/plugins/sticky-columns'; +import { ColumnReordering } from 'ember-headless-table/plugins/column-reordering'; import { ColumnResizing } from 'ember-headless-table/plugins/column-resizing'; import { createHelpers } from 'ember-headless-table/test-support'; import { DATA } from 'test-app/data'; @@ -82,7 +83,7 @@ module('Plugins | StickyColumns', function (hooks) { table = headlessTable(this, { columns: () => this.columns, data: () => DATA, - plugins: [ColumnVisibility, ColumnResizing, StickyColumns], + plugins: [ColumnVisibility, ColumnReordering, ColumnResizing, StickyColumns], }); }