From 84e7ff3a7943854f6850ec75566b0cc20768a94f Mon Sep 17 00:00:00 2001 From: Eli Flanagan Date: Mon, 29 Jul 2019 10:26:09 -0400 Subject: [PATCH 1/4] begin backwards-compatible closure action usage start to fix #614. The way I safely check for the action feels repetitive so am open to better approaches. --- addon/-private/column-tree.js | 9 ++++++++- addon/components/ember-thead/component.js | 1 + tests/helpers/generate-table.js | 2 +- tests/integration/components/headers/reorder-test.js | 8 ++++---- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/addon/-private/column-tree.js b/addon/-private/column-tree.js index a65917495..d5ce4d7f2 100644 --- a/addon/-private/column-tree.js +++ b/addon/-private/column-tree.js @@ -8,6 +8,7 @@ import { computed } from '@ember/object'; import { gt, readOnly } from '@ember/object/computed'; import { scheduler, Token } from 'ember-raf-scheduler'; +import { SUPPORTS_CLOSURE_ACTIONS } from 'ember-compatibility-helpers'; import { getOrCreate } from './meta-cache'; import { objectAt, move, splice } from './utils/array'; @@ -860,7 +861,13 @@ export default EmberObject.extend({ this.container.classList.remove('is-reordering'); - this.sendAction('onReorder', get(node, 'column'), get(closestColumn, 'column')); + if (SUPPORTS_CLOSURE_ACTIONS) { + if (typeof this.onReorder === 'function') { + this.onReorder(get(node, 'column'), get(closestColumn, 'column')); + } + } else { + this.sendAction('onReorder', get(node, 'column'), get(closestColumn, 'column')); + } }, startResize(node, clientX) { diff --git a/addon/components/ember-thead/component.js b/addon/components/ember-thead/component.js index 77da5e63b..16665fc70 100644 --- a/addon/components/ember-thead/component.js +++ b/addon/components/ember-thead/component.js @@ -205,6 +205,7 @@ export default Component.extend({ this.columnMetaCache = new Map(); this.columnTree = ColumnTree.create({ + onReorder: typeof this.onReorder === 'function' ? this.onReorder.bind(this) : null, sendAction: this.sendAction.bind(this), columnMetaCache: this.columnMetaCache, containerWidthAdjustment: this.containerWidthAdjustment, diff --git a/tests/helpers/generate-table.js b/tests/helpers/generate-table.js index 3187d9100..7a40a3b15 100644 --- a/tests/helpers/generate-table.js +++ b/tests/helpers/generate-table.js @@ -24,7 +24,7 @@ const fullTable = hbs` onUpdateSorts="onUpdateSorts" - onReorder="onReorder" + onReorder=onReorder onResize="onResize" as |h| diff --git a/tests/integration/components/headers/reorder-test.js b/tests/integration/components/headers/reorder-test.js index 4cb21d6ee..0bf1b3b17 100644 --- a/tests/integration/components/headers/reorder-test.js +++ b/tests/integration/components/headers/reorder-test.js @@ -58,13 +58,13 @@ module('Integration | headers | reorder', function() { assert.equal(table.headers.objectAt(0).text.trim(), 'A', 'First column is swapped forward'); }); - test('column reorder action is sent up to controller', async function(assert) { - this.on('onReorder', function(insertedColumn, insertedAfter) { + test('calls the closure action onReorder', async function(assert) { + let onReorder = function(insertedColumn, insertedAfter) { assert.equal(insertedColumn.name, 'A', 'old column index is correct'); assert.equal(insertedAfter.name, 'B', 'new column index is correct'); - }); + }; - await generateTable(this); + await generateTable(this, { onReorder }); await table.headers.objectAt(0).reorderBy(1); }); From d8828e1cfdcc5d48c851fa1942ffe7dc03889c49 Mon Sep 17 00:00:00 2001 From: Eli Flanagan Date: Tue, 30 Jul 2019 09:48:58 -0400 Subject: [PATCH 2/4] account for older ember versions in test --- tests/integration/components/headers/reorder-test.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/integration/components/headers/reorder-test.js b/tests/integration/components/headers/reorder-test.js index 0bf1b3b17..0c25e6779 100644 --- a/tests/integration/components/headers/reorder-test.js +++ b/tests/integration/components/headers/reorder-test.js @@ -10,6 +10,8 @@ import { getScale } from 'ember-table/test-support/helpers/element'; import TablePage from 'ember-table/test-support/pages/ember-table'; import { toBase26 } from 'dummy/utils/base-26'; +import { gte } from 'ember-compatibility-helpers'; + const table = new TablePage(); export async function scrollToEdge(targetElement, edgeOffset, direction) { @@ -58,13 +60,17 @@ module('Integration | headers | reorder', function() { assert.equal(table.headers.objectAt(0).text.trim(), 'A', 'First column is swapped forward'); }); - test('calls the closure action onReorder', async function(assert) { + test('calls the closure action onReorder or sendAction given the ember version', async function(assert) { let onReorder = function(insertedColumn, insertedAfter) { assert.equal(insertedColumn.name, 'A', 'old column index is correct'); assert.equal(insertedAfter.name, 'B', 'new column index is correct'); }; - - await generateTable(this, { onReorder }); + if (gte('1.13')) { + await generateTable(this, { onReorder }); + } else { + this.on('onReorder', onReorder); + await generateTable(this); + } await table.headers.objectAt(0).reorderBy(1); }); From a4a3c91a3dfb69bfb3bfd17017b7ae3feb40adc1 Mon Sep 17 00:00:00 2001 From: Eli Flanagan Date: Tue, 30 Jul 2019 09:55:20 -0400 Subject: [PATCH 3/4] use macros and fix test table invocation --- tests/helpers/generate-table.js | 3 ++- tests/integration/components/headers/reorder-test.js | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/helpers/generate-table.js b/tests/helpers/generate-table.js index 7a40a3b15..45c20ad5d 100644 --- a/tests/helpers/generate-table.js +++ b/tests/helpers/generate-table.js @@ -1,6 +1,7 @@ import hbs from 'htmlbars-inline-precompile'; import wait from 'ember-test-helpers/wait'; import { generateColumns, generateRows } from 'dummy/utils/generators'; +import { SUPPORTS_CLOSURE_ACTIONS } from 'ember-compatibility-helpers'; // reexport for use in tests export { generateColumns, generateRows }; @@ -24,7 +25,7 @@ const fullTable = hbs` onUpdateSorts="onUpdateSorts" - onReorder=onReorder + onReorder=${SUPPORTS_CLOSURE_ACTIONS ? 'onReorder' : '"onReorder"'} onResize="onResize" as |h| diff --git a/tests/integration/components/headers/reorder-test.js b/tests/integration/components/headers/reorder-test.js index 0c25e6779..76cda62ca 100644 --- a/tests/integration/components/headers/reorder-test.js +++ b/tests/integration/components/headers/reorder-test.js @@ -10,7 +10,7 @@ import { getScale } from 'ember-table/test-support/helpers/element'; import TablePage from 'ember-table/test-support/pages/ember-table'; import { toBase26 } from 'dummy/utils/base-26'; -import { gte } from 'ember-compatibility-helpers'; +import { SUPPORTS_CLOSURE_ACTIONS } from 'ember-compatibility-helpers'; const table = new TablePage(); @@ -65,7 +65,7 @@ module('Integration | headers | reorder', function() { assert.equal(insertedColumn.name, 'A', 'old column index is correct'); assert.equal(insertedAfter.name, 'B', 'new column index is correct'); }; - if (gte('1.13')) { + if (SUPPORTS_CLOSURE_ACTIONS) { await generateTable(this, { onReorder }); } else { this.on('onReorder', onReorder); From 780af5ce3c5a3e017784ac2a9cd156d2d2a64916 Mon Sep 17 00:00:00 2001 From: Eli Flanagan Date: Tue, 30 Jul 2019 10:05:48 -0400 Subject: [PATCH 4/4] use ember inline conditional instead --- tests/helpers/generate-table.js | 3 +-- tests/integration/components/headers/reorder-test.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/helpers/generate-table.js b/tests/helpers/generate-table.js index 45c20ad5d..e30eeaf8f 100644 --- a/tests/helpers/generate-table.js +++ b/tests/helpers/generate-table.js @@ -1,7 +1,6 @@ import hbs from 'htmlbars-inline-precompile'; import wait from 'ember-test-helpers/wait'; import { generateColumns, generateRows } from 'dummy/utils/generators'; -import { SUPPORTS_CLOSURE_ACTIONS } from 'ember-compatibility-helpers'; // reexport for use in tests export { generateColumns, generateRows }; @@ -25,7 +24,7 @@ const fullTable = hbs` onUpdateSorts="onUpdateSorts" - onReorder=${SUPPORTS_CLOSURE_ACTIONS ? 'onReorder' : '"onReorder"'} + onReorder=(if supportsClosureActions onReorder "onReorder") onResize="onResize" as |h| diff --git a/tests/integration/components/headers/reorder-test.js b/tests/integration/components/headers/reorder-test.js index 76cda62ca..666707745 100644 --- a/tests/integration/components/headers/reorder-test.js +++ b/tests/integration/components/headers/reorder-test.js @@ -66,7 +66,7 @@ module('Integration | headers | reorder', function() { assert.equal(insertedAfter.name, 'B', 'new column index is correct'); }; if (SUPPORTS_CLOSURE_ACTIONS) { - await generateTable(this, { onReorder }); + await generateTable(this, { onReorder, supportsClosureActions: SUPPORTS_CLOSURE_ACTIONS }); } else { this.on('onReorder', onReorder); await generateTable(this);