fix: Handle element in sortable item should be the most recent

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();

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;
+  }
 });

tests/dummy/app/components/cells/day-of-week/index.hbs

@@ -0,0 +1 @@
+<td>{{@record.day}}</td>

tests/dummy/app/components/cells/fruit/index.hbs

@@ -0,0 +1 @@
+<td data-test-fruits>{{@record.fruit}}</td>

tests/dummy/app/components/cells/sortable/index.hbs

@@ -0,0 +1 @@
+<td data-test-table-conditional-cell-handle {{sortable-handle index=@index}} ...attributes>{{@index}}</td>

tests/dummy/app/components/row/component.js

@@ -0,0 +1,7 @@
+import Component from '@glimmer/component';
+
+export default class Row extends Component {
+  get isEven() {
+    return this.args.index % 2 === 0;
+  }
+}

tests/dummy/app/components/row/template.hbs

@@ -0,0 +1,9 @@
+<tr ...attributes>
+  {{#if this.isEven}}
+    <Cells::Sortable @index={{@index}} />
+  {{else}}
+    <Cells::Sortable @index={{@index}} class="odd-class"/>
+  {{/if}}
+  <Cells::Fruit @record={{@record}} />
+  <Cells::DayOfWeek @record={{@record}}/>
+</tr>

tests/dummy/app/components/table/template.hbs

@@ -0,0 +1,11 @@
+<table data-test-table-conditional-cell-demo-group {{sortable-group onChange=@handleDragChange groupName="table-tracked" }} >
+  <thead> 
+    <td>Order</td>
+    <td>Fruit</td>
+    <td>Day of Week</td> </thead>
+  <tbody>
+    {{#each @records as |record index|}}
+      <Row @record={{record}} @index={{index}} {{sortable-item model=record groupName="table-tracked"}}  />
+    {{/each}}
+  </tbody>
+</table>

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);

tests/dummy/app/templates/index.hbs

@@ -90,6 +90,11 @@
       </table>
     </section>
 
+    <section class="table-cell-changes-demo">
+      <h3>Table with conditional cells</h3>
+      <Table @records={{this.records}} @handleDragChange={{this.handleDragChange}} />
+    </section>
+
     <section class='vertical-spacing-demo'>
       <h3>Vertical with 15px spacing</h3>
 
@@ -171,4 +176,4 @@
     </ol>
 
   </section>
-</article>
+</article>