From da8b45452a735bf4426435e79245b79c88bbc95a Mon Sep 17 00:00:00 2001 From: Nicole Chung Date: Mon, 3 Oct 2022 10:07:03 -0400 Subject: [PATCH 1/4] docs: add problem to the examples - add problem example to the demo - conditionally rendered sort-handle renders more often than sortable-item - this creates a this.handleElement that is not attached to the document anymore - keyboard drag and drop will not work --- tests/dummy/app/components/row/component.js | 41 +++++++++++++++++++ tests/dummy/app/components/row/template.hbs | 9 ++++ tests/dummy/app/components/table/template.hbs | 11 +++++ tests/dummy/app/controllers/index.js | 16 ++++++++ tests/dummy/app/templates/index.hbs | 7 +++- 5 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 tests/dummy/app/components/row/component.js create mode 100644 tests/dummy/app/components/row/template.hbs create mode 100644 tests/dummy/app/components/table/template.hbs diff --git a/tests/dummy/app/components/row/component.js b/tests/dummy/app/components/row/component.js new file mode 100644 index 00000000..3b020a19 --- /dev/null +++ b/tests/dummy/app/components/row/component.js @@ -0,0 +1,41 @@ +import Component from '@glimmer/component'; +import { setComponentTemplate } from '@ember/component'; +import templateOnly from '@ember/component/template-only'; +import { hbs } from 'ember-cli-htmlbars'; + +const SortableCell = templateOnly(); + +setComponentTemplate( + hbs`{{log 'rendering cell' @index}} + {{@index}} + `, + SortableCell +); + +const FruitCell = templateOnly(); + +setComponentTemplate( + hbs` + {{@record.fruit}} + `, + FruitCell +); + +const DayOfWeekCell = templateOnly(); + +setComponentTemplate( + hbs` + {{@record.day}} + `, + DayOfWeekCell +); + +export default class Row extends Component { + SortableCell = SortableCell; + FruitCell = FruitCell; + DayOfWeekCell = DayOfWeekCell; + + get isEven() { + return this.args.index % 2 === 0; + } +} diff --git a/tests/dummy/app/components/row/template.hbs b/tests/dummy/app/components/row/template.hbs new file mode 100644 index 00000000..aa3c417e --- /dev/null +++ b/tests/dummy/app/components/row/template.hbs @@ -0,0 +1,9 @@ + + {{#if this.isEven}} + + {{else}} + + {{/if}} + + + diff --git a/tests/dummy/app/components/table/template.hbs b/tests/dummy/app/components/table/template.hbs new file mode 100644 index 00000000..ec7fbcec --- /dev/null +++ b/tests/dummy/app/components/table/template.hbs @@ -0,0 +1,11 @@ + + + + + + + {{#each @records as |record index|}} + + {{/each}} + +
OrderFruitDay of Week
diff --git a/tests/dummy/app/controllers/index.js b/tests/dummy/app/controllers/index.js index f055ae98..b5399ceb 100644 --- a/tests/dummy/app/controllers/index.js +++ b/tests/dummy/app/controllers/index.js @@ -1,9 +1,24 @@ import Controller from '@ember/controller'; import { set, action } from '@ember/object'; +import { tracked } from '@glimmer/tracking'; export default class ModifierController extends Controller { differentSizedModels = ['A', 'B'.repeat(100), 'D'.repeat(50), 'C'.repeat(20)]; + @tracked records = [ + { fruit: 'avocado', day: 'Monday' }, + { fruit: 'banana', day: 'Tuesday' }, + { fruit: 'cashew', day: 'Wednesday' }, + { fruit: 'watermelon', day: 'Thursday' }, + { fruit: 'durian', day: 'Friday' }, + { fruit: 'apple', day: 'Saturday' }, + { fruit: 'lemon', day: 'Sunday' }, + ]; + + @action handleDragChange(reordered) { + this.records = reordered; + } + handleVisualClass = { UP: 'sortable-handle-up', DOWN: 'sortable-handle-down', @@ -43,6 +58,7 @@ export default class ModifierController extends Controller { updateDifferentSizedModels(newOrder) { set(this, 'differentSizedModels', newOrder); } + @action update(newOrder, draggedModel) { set(this, 'model.items', newOrder); diff --git a/tests/dummy/app/templates/index.hbs b/tests/dummy/app/templates/index.hbs index 6cffd141..a8a23f1d 100644 --- a/tests/dummy/app/templates/index.hbs +++ b/tests/dummy/app/templates/index.hbs @@ -90,6 +90,11 @@ +
+

Table with conditional cells

+ + +

Vertical with 15px spacing

@@ -171,4 +176,4 @@
- \ No newline at end of file + From 64975bd1f7332bcde63e3c4c2bec373ec191cc41 Mon Sep 17 00:00:00 2001 From: Nicole Chung Date: Mon, 3 Oct 2022 10:07:03 -0400 Subject: [PATCH 2/4] fix: setup handle element right on keyDown - check again for the correct handle element when key down is pressed - this ensures that the this.handleElement is the one that is still in the document body --- addon/modifiers/sortable-item.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/addon/modifiers/sortable-item.js b/addon/modifiers/sortable-item.js index 00b0819d..3adf3a57 100644 --- a/addon/modifiers/sortable-item.js +++ b/addon/modifiers/sortable-item.js @@ -268,6 +268,8 @@ export default class SortableItemModifier extends Modifier { return; } + this.setupHandleElement(); + // If the event is coming from within the item, we do not want to activate keyboard reorder mode. if (event.target === this.handleElement || event.target === this.element) { this.sortableGroup.activateKeyDown(this); @@ -811,6 +813,16 @@ export default class SortableItemModifier extends Modifier { this.element.removeEventListener('touchstart', this.touchStart); } + setupHandleElement() { + this.handleElement = this.element.querySelector(this.handle); + + if (this.handleElement) { + this.handleElement.style['touch-action'] = 'none'; + } else { + this.element.style['touch-action'] = 'none'; + } + } + element; didSetup = false; @@ -827,13 +839,7 @@ export default class SortableItemModifier extends Modifier { // Instead of using `event.preventDefault()` in the 'primeDrag' event, // (doesn't work in Chrome 56), we set touch-action: none as a workaround. - this.handleElement = this.element.querySelector(this.handle); - - if (this.handleElement) { - this.handleElement.style['touch-action'] = 'none'; - } else { - this.element.style['touch-action'] = 'none'; - } + this.setupHandleElement(); if (!this.didSetup) { this.addEventListener(); From 96492eeb6db3ca1ea6234fd13989f85807b90a9d Mon Sep 17 00:00:00 2001 From: Nicole Chung Date: Mon, 3 Oct 2022 10:07:03 -0400 Subject: [PATCH 3/4] chore: update tests - added a test for a conditional rendering of the sort handle - for this test to fail, drag and drop with keyboard has to be done several times - added displaying the fruits only as this is easier to read in the tests --- tests/acceptance/smoke-modifier-test.js | 56 +++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/acceptance/smoke-modifier-test.js b/tests/acceptance/smoke-modifier-test.js index 5d6e808c..e42f2a04 100644 --- a/tests/acceptance/smoke-modifier-test.js +++ b/tests/acceptance/smoke-modifier-test.js @@ -433,6 +433,51 @@ module('Acceptance | smoke modifier', function (hooks) { await triggerKeyEvent('[data-test-vertical-demo-group]', 'keydown', ENTER_KEY_CODE); assert.equal(justDraggedContents(), 'Zero'); + + assert.equal(tableConditionalCellContents(), 'avocado banana cashew watermelon durian apple lemon '); + await focus('[data-test-table-conditional-cell-handle]'); + + await triggerKeyEvent('[data-test-table-conditional-cell-handle]', 'keydown', ENTER_KEY_CODE); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ARROW_KEY_CODES.DOWN); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ENTER_KEY_CODE); + + assert.equal(tableConditionalCellContents(), 'banana avocado cashew watermelon durian apple lemon '); + }); + + test('Keyboard selection works multiple times for conditionally rendered sort-handle', async function (assert) { + await visit('/'); + + assert.equal(tableConditionalCellContents(), 'avocado banana cashew watermelon durian apple lemon '); + + await focus('[data-test-table-conditional-cell-handle]'); + + await triggerKeyEvent('[data-test-table-conditional-cell-handle]', 'keydown', ENTER_KEY_CODE); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ARROW_KEY_CODES.DOWN); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ENTER_KEY_CODE); + + assert.equal(tableConditionalCellContents(), 'banana avocado cashew watermelon durian apple lemon '); + + const moveHandle = findAll('[data-test-table-conditional-cell-handle]')[4]; + await focus(moveHandle); + + await triggerKeyEvent(moveHandle, 'keydown', ENTER_KEY_CODE); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ARROW_KEY_CODES.UP); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ARROW_KEY_CODES.UP); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ENTER_KEY_CODE); + + assert.equal(tableConditionalCellContents(), 'banana avocado durian cashew watermelon apple lemon '); + + const moveHandle1 = findAll('[data-test-table-conditional-cell-handle]')[0]; + await focus(moveHandle1); + + await triggerKeyEvent(moveHandle1, 'keydown', ENTER_KEY_CODE); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ARROW_KEY_CODES.DOWN); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ARROW_KEY_CODES.DOWN); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ARROW_KEY_CODES.DOWN); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ARROW_KEY_CODES.DOWN); + await triggerKeyEvent('[data-test-table-conditional-cell-demo-group]', 'keydown', ENTER_KEY_CODE); + + assert.equal(tableConditionalCellContents(), 'avocado durian cashew watermelon banana apple lemon '); }); }); @@ -459,4 +504,15 @@ module('Acceptance | smoke modifier', function (hooks) { function contents(selector) { return find(selector).textContent.replace(/⇕/g, '').replace(/\s+/g, ' ').replace(/^\s+/, '').replace(/\s+$/, ''); } + + function tableConditionalCellContents() { + const elements = findAll('[data-test-fruits]'); + let result = ''; + for (const index in elements) { + const element = elements[index]; + result += element.textContent.replace(/⇕/g, '').replace(/\s+/g, ' ').replace(/^\s+/, '').replace(/\s+$/, ''); + result += ' '; + } + return result; + } }); From 886a04c13352148e4d85e19177845e5cef9ea05b Mon Sep 17 00:00:00 2001 From: Nicole Chung Date: Thu, 6 Oct 2022 09:04:36 -0400 Subject: [PATCH 4/4] fix: rewrite components to fix ember-lts-3.24 --- .../components/cells/day-of-week/index.hbs | 1 + .../app/components/cells/fruit/index.hbs | 1 + .../app/components/cells/sortable/index.hbs | 1 + tests/dummy/app/components/row/component.js | 34 ------------------- tests/dummy/app/components/row/template.hbs | 8 ++--- 5 files changed, 7 insertions(+), 38 deletions(-) create mode 100644 tests/dummy/app/components/cells/day-of-week/index.hbs create mode 100644 tests/dummy/app/components/cells/fruit/index.hbs create mode 100644 tests/dummy/app/components/cells/sortable/index.hbs diff --git a/tests/dummy/app/components/cells/day-of-week/index.hbs b/tests/dummy/app/components/cells/day-of-week/index.hbs new file mode 100644 index 00000000..b80f73e2 --- /dev/null +++ b/tests/dummy/app/components/cells/day-of-week/index.hbs @@ -0,0 +1 @@ + diff --git a/tests/dummy/app/components/cells/fruit/index.hbs b/tests/dummy/app/components/cells/fruit/index.hbs new file mode 100644 index 00000000..53e6aef1 --- /dev/null +++ b/tests/dummy/app/components/cells/fruit/index.hbs @@ -0,0 +1 @@ + diff --git a/tests/dummy/app/components/cells/sortable/index.hbs b/tests/dummy/app/components/cells/sortable/index.hbs new file mode 100644 index 00000000..c6d9926e --- /dev/null +++ b/tests/dummy/app/components/cells/sortable/index.hbs @@ -0,0 +1 @@ + diff --git a/tests/dummy/app/components/row/component.js b/tests/dummy/app/components/row/component.js index 3b020a19..29b2e6dc 100644 --- a/tests/dummy/app/components/row/component.js +++ b/tests/dummy/app/components/row/component.js @@ -1,40 +1,6 @@ import Component from '@glimmer/component'; -import { setComponentTemplate } from '@ember/component'; -import templateOnly from '@ember/component/template-only'; -import { hbs } from 'ember-cli-htmlbars'; - -const SortableCell = templateOnly(); - -setComponentTemplate( - hbs`{{log 'rendering cell' @index}} - - `, - SortableCell -); - -const FruitCell = templateOnly(); - -setComponentTemplate( - hbs` - - `, - FruitCell -); - -const DayOfWeekCell = templateOnly(); - -setComponentTemplate( - hbs` - - `, - DayOfWeekCell -); export default class Row extends Component { - SortableCell = SortableCell; - FruitCell = FruitCell; - DayOfWeekCell = DayOfWeekCell; - get isEven() { return this.args.index % 2 === 0; } diff --git a/tests/dummy/app/components/row/template.hbs b/tests/dummy/app/components/row/template.hbs index aa3c417e..4c06514f 100644 --- a/tests/dummy/app/components/row/template.hbs +++ b/tests/dummy/app/components/row/template.hbs @@ -1,9 +1,9 @@ {{#if this.isEven}} - + {{else}} - + {{/if}} - - + +
{{@record.day}}{{@record.fruit}}{{@index}}{{@index}}{{@record.fruit}}{{@record.day}}