Add BulkController support for shift+click behaviour

- Allow multiple selections or un-selections with shift+click
- Pull out activeItems getter and instead use this as a method for select all checkboxes and individual checkboxes
- Pull out ToggleAllOptions to a type and support this being passed in via a CustomEvent also, as per pattern being set up in other Controllers
- Adjust unit test structure to be easier to work with different HTML values

client/src/controllers/BulkController.test.js

@@ -2,9 +2,9 @@ import { Application } from '@hotwired/stimulus';
 import { BulkController } from './BulkController';
 
 describe('BulkController', () => {
-  beforeEach(() => {
-    document.body.innerHTML = `
-    <div id="bulk-container" data-controller="w-bulk">
+  const setup = async (
+    html = `
+    <div id="bulk-container" data-controller="w-bulk" data-action="custom:event@document->w-bulk#toggleAll">
       <input id="select-all" type="checkbox" data-w-bulk-target="all" data-action="w-bulk#toggleAll">
       <div id="checkboxes">
         <input type="checkbox" data-w-bulk-target="item" disabled data-action="w-bulk#toggle">
@@ -13,12 +13,17 @@ describe('BulkController', () => {
       </div>
       <button id="clear" data-action="w-bulk#toggleAll" data-w-bulk-force-param="false">Clear all</button>
       <button id="set" data-action="w-bulk#toggleAll" data-w-bulk-force-param="true">Select all</button>
-    </div>`;
+    </div>`,
+  ) => {
+    document.body.innerHTML = `<main>${html}</main>`;
+
     const application = Application.start();
     application.register('w-bulk', BulkController);
-  });
+  };
+
+  it('selects all checkboxes when the select all checkbox is clicked', async () => {
+    await setup();
 
-  it('selects all checkboxes when the select all checkbox is clicked', () => {
     const allCheckbox = document.getElementById('select-all');
 
     allCheckbox.click();
@@ -38,7 +43,9 @@ describe('BulkController', () => {
     ).toEqual(0);
   });
 
-  it('should keep the select all checkbox in sync when individual checkboxes are all ticked', () => {
+  it('should keep the select all checkbox in sync when individual checkboxes are all ticked', async () => {
+    await setup();
+
     const allCheckbox = document.getElementById('select-all');
     expect(allCheckbox.checked).toBe(false);
 
@@ -63,7 +70,9 @@ describe('BulkController', () => {
     expect(allCheckbox.checked).toBe(false);
   });
 
-  it('executes the correct action when the Clear all button is clicked', () => {
+  it('executes the correct action when the Clear all button is clicked', async () => {
+    await setup();
+
     const allCheckbox = document.getElementById('select-all');
     const clearAllButton = document.getElementById('clear');
     expect(allCheckbox.checked).toBe(false);
@@ -88,7 +97,9 @@ describe('BulkController', () => {
     expect(allCheckbox.checked).toBe(false);
   });
 
-  it('executes the correct action when the Set all button is clicked', () => {
+  it('executes the correct action when the Set all button is clicked', async () => {
+    await setup();
+
     const allCheckbox = document.getElementById('select-all');
     const setAllButton = document.getElementById('set');
 
@@ -115,7 +126,50 @@ describe('BulkController', () => {
     });
   });
 
+  it('should support using another method (e.g. CustomEvent) to toggle all', async () => {
+    await setup();
+
+    const allCheckbox = document.getElementById('select-all');
+    expect(allCheckbox.checked).toBe(false);
+
+    document.dispatchEvent(new CustomEvent('custom:event'));
+
+    expect(allCheckbox.checked).toBe(true);
+    expect(document.querySelectorAll(':checked')).toHaveLength(3);
+
+    // calling again, should switch the toggles back
+
+    document.dispatchEvent(new CustomEvent('custom:event'));
+
+    expect(allCheckbox.checked).toBe(false);
+    expect(document.querySelectorAll(':checked')).toHaveLength(0);
+  });
+
+  it('should support a force value in a CustomEvent to override the select all checkbox', async () => {
+    await setup();
+
+    const allCheckbox = document.getElementById('select-all');
+    expect(allCheckbox.checked).toBe(false);
+
+    document.dispatchEvent(
+      new CustomEvent('custom:event', { detail: { force: true } }),
+    );
+
+    expect(allCheckbox.checked).toBe(true);
+    expect(document.querySelectorAll(':checked')).toHaveLength(3);
+
+    // calling again, should not change the state of the checkboxes
+    document.dispatchEvent(
+      new CustomEvent('custom:event', { detail: { force: true } }),
+    );
+
+    expect(allCheckbox.checked).toBe(true);
+    expect(document.querySelectorAll(':checked')).toHaveLength(3);
+  });
+
   it('should allow for action targets to have classes toggled when any checkboxes are clicked', async () => {
+    await setup();
+
     const container = document.getElementById('bulk-container');
 
     // create innerActions container that will be conditionally hidden with test classes
@@ -151,4 +205,105 @@ describe('BulkController', () => {
 
     expect(innerActionsElement.className).toEqual('keep-me hidden w-invisible');
   });
+
+  it('should support shift+click to select a range of checkboxes', async () => {
+    await setup(`
+    <div id="bulk-container-multi" data-controller="w-bulk">
+      <input id="select-all-multi" type="checkbox" data-w-bulk-target="all" data-action="w-bulk#toggleAll">
+      <div id="checkboxes">
+        <input id="0" type="checkbox" data-w-bulk-target="item" data-action="w-bulk#toggle">
+        <input id="1" type="checkbox" data-w-bulk-target="item" data-action="w-bulk#toggle">
+        <input id="x" type="checkbox" data-w-bulk-target="item" data-action="w-bulk#toggle" disabled>
+        <input id="2" type="checkbox" data-w-bulk-target="item" data-action="w-bulk#toggle">
+        <input id="3" type="checkbox" data-w-bulk-target="item" data-action="w-bulk#toggle">
+        <input id="4" type="checkbox" data-w-bulk-target="item" data-action="w-bulk#toggle">
+        <input id="5" type="checkbox" data-w-bulk-target="item" data-action="w-bulk#toggle">
+      </div>
+    </div>`);
+
+    const getClickedIds = () =>
+      Array.from(document.querySelectorAll(':checked')).map(({ id }) => id);
+
+    const shiftClick = async (element) => {
+      document.dispatchEvent(
+        new KeyboardEvent('keydown', {
+          key: 'Shift',
+          shiftKey: true,
+        }),
+      );
+      element.click();
+      document.dispatchEvent(
+        new KeyboardEvent('keyup', {
+          key: 'Shift',
+          shiftKey: true,
+        }),
+      );
+      await Promise.resolve();
+    };
+
+    // initial shift usage should have no impact
+    await shiftClick(document.getElementById('0'));
+    expect(getClickedIds()).toHaveLength(1);
+
+    // shift click should select all checkboxes between the first and last clicked
+    await shiftClick(document.getElementById('2'));
+    expect(getClickedIds()).toEqual(['0', '1', '2']);
+
+    await shiftClick(document.getElementById('5'));
+    expect(getClickedIds()).toEqual([
+      'select-all-multi',
+      '0',
+      '1',
+      '2',
+      '3',
+      '4',
+      '5',
+    ]);
+
+    // it should allow reverse clicks
+    document.getElementById('4').click(); // un-click
+    expect(getClickedIds()).toEqual(['0', '1', '2', '3', '5']);
+
+    // now shift click in reverse, un-clicking those between the last (4) and the new click (1)
+    await shiftClick(document.getElementById('1'));
+    expect(getClickedIds()).toEqual(['0', '5']);
+
+    // reset the clicks, then using shift click should do nothing
+    document.getElementById('select-all-multi').click();
+    document.getElementById('select-all-multi').click();
+    expect(getClickedIds()).toHaveLength(0);
+
+    await shiftClick(document.getElementById('4'));
+    expect(getClickedIds()).toEqual(['4']);
+
+    // finally, do a shift click to the first checkbox, check the select all works after a final click
+    await shiftClick(document.getElementById('0'));
+    expect(getClickedIds()).toEqual(['0', '1', '2', '3', '4']);
+
+    document.getElementById('5').click();
+
+    expect(getClickedIds()).toEqual([
+      'select-all-multi',
+      '0',
+      '1',
+      '2',
+      '3',
+      '4',
+      '5',
+    ]);
+
+    // now ensure that it still works if some element gets changed (not disabled)
+    document.getElementById('x').removeAttribute('disabled');
+    document.getElementById('select-all-multi').click();
+    expect(getClickedIds()).toHaveLength(0);
+
+    await Promise.resolve();
+
+    document.getElementById('3').click(); // click
+
+    await shiftClick(document.getElementById('1'));
+
+    // it should include the previously disabled element, tracking against the DOM, not indexes
+    expect(getClickedIds()).toEqual(['1', 'x', '2', '3']);
+  });
 });

client/src/controllers/BulkController.ts

@@ -1,5 +1,10 @@
 import { Controller } from '@hotwired/stimulus';
 
+type ToggleAllOptions = {
+  /** Override check all behaviour to either force check or uncheck all */
+  force?: boolean;
+};
+
 /**
  * Adds the ability to collectively toggle a set of (non-disabled) checkboxes.
  *
@@ -27,7 +32,7 @@ import { Controller } from '@hotwired/stimulus';
  *     <input data-action="w-bulk#toggle" data-w-bulk-target="item" type="checkbox" />
  *   </div>
  * </div>
-
+ *
  */
 export class BulkController extends Controller<HTMLElement> {
   static classes = ['actionInactive'];
@@ -45,28 +50,91 @@ export class BulkController extends Controller<HTMLElement> {
   /** Classes to remove on the actions target if any actions are checked */
   declare readonly actionInactiveClasses: string[];
 
-  get activeItems() {
-    return this.itemTargets.filter(({ disabled }) => !disabled);
-  }
+  /** Internal tracking of last clicked for shift+click behaviour */
+  lastChanged?: HTMLElement | null;
+
+  /** Internal tracking of whether the shift key is active for multiple selection */
+  shiftActive?: boolean;
 
   /**
    * On creation, ensure that the select all checkboxes are in sync.
+   * Set up the event listeners for shift+click behaviour.
    */
   connect() {
     this.toggle();
+    this.handleShiftKey = this.handleShiftKey.bind(this);
+    document.addEventListener('keydown', this.handleShiftKey);
+    document.addEventListener('keyup', this.handleShiftKey);
+  }
+
+  /**
+   * Returns all valid targets (i.e. not disabled).
+   */
+  getValidTargets(
+    targets: HTMLInputElement[] = this.itemTargets,
+  ): HTMLInputElement[] {
+    return targets.filter(({ disabled }) => !disabled);
+  }
+
+  /**
+   * Event handler to determine if shift key is pressed.
+   */
+  handleShiftKey(event: KeyboardEvent) {
+    if (!event) return;
+
+    const { shiftKey, type } = event;
+
+    if (type === 'keydown' && shiftKey) {
+      this.shiftActive = true;
+    }
+
+    if (type === 'keyup' && this.shiftActive) {
+      this.shiftActive = false;
+    }
   }
 
   /**
-   * When something is toggled, ensure the select all targets are kept in sync.
+   * When an item is toggled, ensure the select all targets are kept in sync.
    * Update the classes on the action targets to reflect the current state.
+   * If the shift key is pressed, toggle all the items between the last clicked
+   * item and the current item.
    */
-  toggle() {
-    const activeItems = this.activeItems;
+  toggle(event?: Event) {
+    const activeItems = this.getValidTargets();
+    const lastChanged = this.lastChanged;
+
+    if (this.shiftActive && lastChanged instanceof HTMLElement) {
+      this.shiftActive = false;
+
+      const lastClickedIndex = activeItems.findIndex(
+        (item) => item === lastChanged,
+      );
+      const currentIndex = activeItems.findIndex(
+        (item) => item === event?.target,
+      );
+
+      const [start, end] = [lastClickedIndex, currentIndex].sort(
+        // eslint-disable-next-line id-length
+        (a, b) => a - b,
+      );
+
+      activeItems.forEach((target, index) => {
+        if (index >= start && index <= end) {
+          // eslint-disable-next-line no-param-reassign
+          target.checked = !!activeItems[lastClickedIndex].checked;
+          this.dispatch('change', { target, bubbles: true });
+        }
+      });
+    }
+
+    this.lastChanged =
+      activeItems.find((item) => item.contains(event?.target as Node)) ?? null;
+
     const totalCheckedItems = activeItems.filter((item) => item.checked).length;
     const isAnyChecked = totalCheckedItems > 0;
     const isAllChecked = totalCheckedItems === activeItems.length;
 
-    this.allTargets.forEach((target) => {
+    this.getValidTargets(this.allTargets).forEach((target) => {
       // eslint-disable-next-line no-param-reassign
       target.checked = isAllChecked;
     });
@@ -82,14 +150,34 @@ export class BulkController extends Controller<HTMLElement> {
   }
 
   /**
-   * Toggles all item checkboxes based on select-all checkbox.
+   * Toggles all item checkboxes, can be used to force check or uncheck all.
+   * If the event used to trigger this method does not have a suitable target,
+   * the first allTarget will be used to determine the current checked value.
    */
-  toggleAll(event: Event & { params?: { force?: boolean } }): void {
-    const force = event?.params?.force;
-    const checkbox = event.target as HTMLInputElement;
-    const isChecked = typeof force === 'boolean' ? force : checkbox.checked;
+  toggleAll(
+    event: CustomEvent<ToggleAllOptions> & { params?: ToggleAllOptions },
+  ) {
+    const { force = null } = {
+      ...event?.detail,
+      ...event?.params,
+    };
+
+    this.lastChanged = null;
+
+    let isChecked = false;
 
-    this.activeItems.forEach((target) => {
+    if (typeof force === 'boolean') {
+      isChecked = force;
+    } else if (event.target instanceof HTMLInputElement) {
+      isChecked = event.target.checked;
+    } else {
+      const checkbox = this.allTargets[0];
+      // use the opposite of the current state
+      // as this is being triggered by an external call
+      isChecked = !checkbox?.checked;
+    }
+
+    this.getValidTargets().forEach((target) => {
       if (target.checked !== isChecked) {
         // eslint-disable-next-line no-param-reassign
         target.checked = isChecked;
@@ -99,4 +187,9 @@ export class BulkController extends Controller<HTMLElement> {
 
     this.toggle();
   }
+
+  disconnect() {
+    document?.removeEventListener('keydown', this.handleShiftKey);
+    document?.removeEventListener('keyup', this.handleShiftKey);
+  }
 }