diff --git a/app/components/open_project/common/border_box_list_component.sass b/app/components/open_project/common/border_box_list_component.sass index b6ebd825a0ca..b9bef95c1eff 100644 --- a/app/components/open_project/common/border_box_list_component.sass +++ b/app/components/open_project/common/border_box_list_component.sass @@ -61,7 +61,7 @@ > ul > .Box-row:first-of-type border-top-color: transparent - &:has(> .Box-card[data-selected]) + &[data-batch-selected] border-top-color: var(--box-list-item-selected-border-color) &:has(> .Box-card[aria-current="true"]) @@ -193,10 +193,11 @@ &:has(> .Box-card:focus-visible) background-color: var(--bgColor-muted) -// Selected state: applied synchronously on activation by the -// backlogs--work-package controller (single selection), so the list reacts -// before the split screen has loaded. -.Box-row:has(> .Box-card[data-selected]) +// Batch selection is written on the `.Box-row` itself, while the +// current-work-package rule below matches `aria-current` on the `.Box-card` +// nested inside it. The two selectors are asymmetric because the attributes +// are. +.Box-row[data-batch-selected] background-color: var(--box-list-item-selected-bg-color) @include op-box-list-item-edge-borders(var(--box-list-item-selected-border-color)) @@ -206,7 +207,7 @@ // Current state: the card is open in the split screen. aria-current is // maintained by the backlogs--work-package controller once the URL reflects -// the details pane; the strong border wins over a simultaneous selected state. +// the details pane; the strong border wins over a simultaneous batch selection. .Box-row:has(> .Box-card[aria-current="true"]) @include op-box-list-item-edge-borders(var(--box-list-item-pressed-border-color)) diff --git a/config/locales/js-en.yml b/config/locales/js-en.yml index dc2a37bfd2c2..7c54ca744684 100644 --- a/config/locales/js-en.yml +++ b/config/locales/js-en.yml @@ -867,6 +867,17 @@ en: move_failed_rolled_back: "Move failed. %{label} returned to its previous position." moved: "%{label} moved to position %{position} of %{total}" moved_to_list: "%{label} moved to %{list}, position %{position} of %{total}" + selection: + cleared: "Selection cleared." + not_selectable: "Selection unchanged. This item takes no part in this list's ordering." + range_blocked: "Selection unchanged. That range contains an item that takes no part in this list's ordering." + range_restarted: + one: "Could not extend the range. 1 item selected." + other: "Could not extend the range. %{count} items selected." + range_unavailable: "Selection unchanged. Expand this list to select that range." + selected: + one: "1 item selected." + other: "%{count} items selected." spot: drop_modal: diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index 6fccc2fec73c..202d9b389869 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -7,6 +7,7 @@ - `./src/common/` - Framework-agnostic modules (the `core-common` alias), importable from both Angular and Stimulus. Code belongs here when it depends on neither framework and both sides need it; a helper only Stimulus controllers use belongs in `./src/stimulus/helpers/` instead. - `./src/stimulus/` - Stimulus controllers - `./src/turbo/` - Turbo integration +- [Sortable lists selection contract](src/stimulus/controllers/dynamic/sortable-lists/README.md) - Consumer wiring, item identity, selection gestures, and presentation. ## Configuration Files diff --git a/frontend/src/common/batch-selection.spec.ts b/frontend/src/common/batch-selection.spec.ts new file mode 100644 index 000000000000..54d785fbc865 --- /dev/null +++ b/frontend/src/common/batch-selection.spec.ts @@ -0,0 +1,253 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { BatchSelection, selectionKey } from './batch-selection'; + +describe('BatchSelection', () => { + let selection:BatchSelection; + + beforeEach(() => { + selection = new BatchSelection(); + }); + + it('starts empty with no anchor', () => { + expect(selection.size).toBe(0); + expect(selection.anchor).toBeNull(); + }); + + it('replaces the batch and establishes the anchor', () => { + selection.replace({ type: 'work_package', id: '1' }, 'sprint:7'); + selection.replace({ type: 'work_package', id: '2' }, 'sprint:7'); + + expect(selection.items().map((entry) => entry.id)).toEqual(['2']); + expect(selection.anchor).toEqual({ type: 'work_package', id: '2', listKey: 'sprint:7' }); + }); + + it('re-bases the anchor when toggling on', () => { + selection.replace({ type: 'work_package', id: '1' }, 'sprint:7'); + selection.toggle({ type: 'work_package', id: '2' }, 'sprint:7'); + + expect(selection.items().map((entry) => entry.id)).toEqual(['1', '2']); + expect(selection.anchor).toEqual({ type: 'work_package', id: '2', listKey: 'sprint:7' }); + }); + + // The anchor deliberately survives its own deselection: the next Shift + // gesture still measures its range from the card the user last touched. + it('re-bases the anchor when toggling off', () => { + selection.replace({ type: 'work_package', id: '1' }, 'sprint:7'); + selection.toggle({ type: 'work_package', id: '1' }, 'sprint:7'); + + expect(selection.size).toBe(0); + expect(selection.anchor).toEqual({ type: 'work_package', id: '1', listKey: 'sprint:7' }); + }); + + it('resizes a range and preserves the anchor', () => { + selection.replace({ type: 'work_package', id: '1' }, 'sprint:7'); + selection.range([{ type: 'work_package', id: '1' }, { type: 'work_package', id: '2' }, { type: 'work_package', id: '3' }]); + selection.range([{ type: 'work_package', id: '1' }, { type: 'work_package', id: '2' }]); + + expect(selection.items().map((entry) => entry.id)).toEqual(['1', '2']); + expect(selection.anchor).toEqual({ type: 'work_package', id: '1', listKey: 'sprint:7' }); + }); + + describe('range sessions', () => { + const wp = (id:string) => ({ type: 'work_package', id }); + const ids = () => selection.items().map((entry) => entry.id).sort(); + + beforeEach(() => { + selection.replace(wp('1'), 'sprint:7'); + selection.toggle(wp('3'), 'sprint:7'); + }); + + it('preserves independent selections while extending, shrinking and reversing', () => { + selection.range(['3', '4', '5'].map(wp)); + expect(ids()).toEqual(['1', '3', '4', '5']); + selection.range(['3', '4'].map(wp)); + expect(ids()).toEqual(['1', '3', '4']); + selection.range(['2', '3'].map(wp)); + expect(ids()).toEqual(['1', '2', '3']); + }); + + it('preserves baseline members even when they overlap a previous range', () => { + selection.range(['1', '2', '3'].map(wp)); + selection.range(['3', '4'].map(wp)); + expect(ids()).toEqual(['1', '3', '4']); + }); + + it('starts a new baseline after toggling off the anchor', () => { + selection.range(['3', '4'].map(wp)); + selection.toggle(wp('3'), 'sprint:7'); + selection.range(['2', '3'].map(wp)); + expect(ids()).toEqual(['1', '2', '3', '4']); + expect(selection.anchor?.id).toBe('3'); + }); + + it.each(['replace', 'clear', 'selectAll'] as const)('ends the session on %s', (operation) => { + selection.range(['3', '4'].map(wp)); + if (operation === 'replace') { + selection.replace(wp('5'), 'sprint:7'); + } else if (operation === 'selectAll') { + selection.selectAll([wp('5')], { ...wp('5'), listKey: 'sprint:7' }); + } else { + selection.clear(); + } + selection.range(['5', '6'].map(wp)); + expect(ids()).toEqual(['5', '6']); + }); + + it('narrows and reverses a selection made with select-all', () => { + selection.selectAll(['1', '2', '3', '4', '5'].map(wp), { ...wp('3'), listKey: 'sprint:7' }); + selection.range(['3', '4'].map(wp)); + expect(ids()).toEqual(['3', '4']); + selection.range(['2', '3'].map(wp)); + expect(ids()).toEqual(['2', '3']); + }); + + it('starts a fresh baseline when toggling after select-all', () => { + selection.selectAll(['1', '2', '3', '4'].map(wp), { ...wp('3'), listKey: 'sprint:7' }); + selection.toggle(wp('2'), 'sprint:7'); + selection.range(['2', '3'].map(wp)); + expect(ids()).toEqual(['1', '2', '3', '4']); + }); + + it('does not restore pruned baseline members', () => { + selection.range(['3', '4'].map(wp)); + selection.prune(new Set(['3', '4', '5'].map((id) => selectionKey(wp(id))))); + selection.range(['3', '5'].map(wp)); + expect(ids()).toEqual(['3', '5']); + }); + + it('keeps the session through unchanged reconciliation', () => { + selection.range(['3', '4', '5'].map(wp)); + selection.prune(new Set(['1', '3', '4', '5'].map((id) => selectionKey(wp(id))))); + selection.rebindAnchor('sprint:7'); + selection.range(['3', '4'].map(wp)); + expect(ids()).toEqual(['1', '3', '4']); + }); + + it('ends the session when the anchor moves to another list', () => { + selection.range(['3', '4'].map(wp)); + selection.rebindAnchor('sprint:8'); + selection.range(['3', '5'].map(wp)); + expect(ids()).toEqual(['1', '3', '4', '5']); + }); + }); + + it('selects all with an explicit anchor', () => { + selection.selectAll([{ type: 'work_package', id: '3' }, { type: 'work_package', id: '1' }, { type: 'work_package', id: '2' }], { type: 'work_package', id: '2', listKey: 'sprint:7' }); + + expect(selection.items().map((entry) => entry.id)).toEqual(['3', '1', '2']); + expect(selection.anchor).toEqual({ type: 'work_package', id: '2', listKey: 'sprint:7' }); + }); + + it('clears the batch and the anchor', () => { + selection.replace({ type: 'work_package', id: '1' }, 'sprint:7'); + selection.clear(); + + expect(selection.size).toBe(0); + expect(selection.anchor).toBeNull(); + }); + + it('prunes ids that are no longer live and reports the change', () => { + selection.replace({ type: 'work_package', id: '1' }, 'sprint:7'); + selection.toggle({ type: 'work_package', id: '2' }, 'sprint:7'); + + expect(selection.prune(new Set([selectionKey({ type: 'work_package', id: '1' })]))).toBe(true); + expect(selection.items().map((entry) => entry.id)).toEqual(['1']); + expect(selection.prune(new Set([selectionKey({ type: 'work_package', id: '1' })]))).toBe(false); + }); + + it('drops an anchor whose card is gone', () => { + selection.replace({ type: 'work_package', id: '1' }, 'sprint:7'); + selection.prune(new Set()); + + expect(selection.anchor).toBeNull(); + }); + + it('reports membership without exposing mutable state', () => { + selection.replace({ type: 'work_package', id: '1' }, 'sprint:7'); + + expect(selection.has({ type: 'work_package', id: '1' })).toBe(true); + expect(selection.has({ type: 'work_package', id: '2' })).toBe(false); + }); + + describe('composite identity', () => { + // Ids are unique per table, not per root: a section and a custom field + // can both be id 5, and a nested topology puts them under one root. + it('keeps items of different types that share an id apart', () => { + selection.toggle({ type: 'section', id: '5' }, 'sections'); + selection.toggle({ type: 'custom_field', id: '5' }, 'custom_field:5'); + + expect(selection.size).toBe(2); + expect(selection.has({ type: 'section', id: '5' })).toBe(true); + expect(selection.has({ type: 'custom_field', id: '5' })).toBe(true); + expect(selection.has({ type: 'work_package', id: '5' })).toBe(false); + }); + + it('prunes by composite key, not by bare id', () => { + selection.toggle({ type: 'custom_field', id: '5' }, 'custom_field:5'); + + // The id survives — under a different type, which is a different item. + selection.prune(new Set([selectionKey({ type: 'section', id: '5' })])); + + expect(selection.size).toBe(0); + }); + + it('drops an anchor whose type no longer exists even when the id survives', () => { + selection.replace({ type: 'custom_field', id: '5' }, 'custom_field:5'); + + selection.prune(new Set([selectionKey({ type: 'section', id: '5' })])); + + expect(selection.anchor).toBeNull(); + }); + + it('builds a key that no type or id can forge a collision in', () => { + expect(selectionKey({ type: 'a', id: 'b' })) + .not.toEqual(selectionKey({ type: 'a\u001Fb', id: '' })); + }); + }); + + describe('#rebindAnchor', () => { + // A card can be moved to another list while staying the anchor, so the + // caller re-derives the key and hands it back. + it('points the anchor at a different list without disturbing membership', () => { + selection.replace({ type: 'work_package', id: '7' }, 'sprint:1'); + + selection.rebindAnchor('sprint:2'); + + expect(selection.anchor).toEqual({ type: 'work_package', id: '7', listKey: 'sprint:2' }); + expect(selection.items().map((entry) => entry.id)).toEqual(['7']); + }); + + it('does nothing when there is no anchor', () => { + selection.rebindAnchor('sprint:2'); + + expect(selection.anchor).toBeNull(); + }); + }); +}); diff --git a/frontend/src/common/batch-selection.ts b/frontend/src/common/batch-selection.ts new file mode 100644 index 000000000000..a24165679839 --- /dev/null +++ b/frontend/src/common/batch-selection.ts @@ -0,0 +1,180 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +/** + * One selectable item's identity. + * + * Type as well as id, because ids are unique per source table rather than per + * root: a section and a custom field can both be id 5, and a nested list puts + * them under one root. Keying on the id alone would let one paint the other. + */ +export interface SelectionItem { + type:string; + id:string; +} + +/** An opaque key for one item. Build it with {@link selectionKey}. */ +export type SelectionKey = string; + +// U+001F (unit separator) cannot appear in an HTML attribute value that +// reached us through the DOM, so no type or id can forge a collision. Written +// as an escape rather than a literal control character in source. +const KEY_SEPARATOR = '\u001F'; + +export function selectionKey({ type, id }:SelectionItem):SelectionKey { + return `${type}${KEY_SEPARATOR}${id}`; +} + +/** + * The item from which a contiguous batch range is measured. + * + * `listKey` is opaque here: the model records which list a range may span + * without learning what a list is. Its holder decides how to derive it, and + * rebinds it through {@link BatchSelection#rebindAnchor} when the item moves. + */ +export interface SelectionAnchor extends SelectionItem { + listKey:string; +} + +/** + * Visual order belongs to the live document. Callers resolve ranges and + * determine whether their members may belong to the same batch. + */ +export class BatchSelection { + private selectedItems = new Map(); + private selectionAnchor:SelectionAnchor|null = null; + private rangeBaseline:Map|null = null; + + // Keys, for membership tests and presentation matching. + get keys():ReadonlySet { + return new Set(this.selectedItems.keys()); + } + + // The pairs themselves, for callers that need the type back. + items():SelectionItem[] { + return [...this.selectedItems.values()]; + } + + get anchor():SelectionAnchor|null { + return this.selectionAnchor; + } + + get size():number { + return this.selectedItems.size; + } + + has(item:SelectionItem):boolean { + return this.selectedItems.has(selectionKey(item)); + } + + replace(item:SelectionItem, listKey:string):void { + this.rangeBaseline = null; + this.selectedItems = new Map([[selectionKey(item), item]]); + this.selectionAnchor = { ...item, listKey }; + } + + // Re-bases the anchor even when the toggle deselects: the user's last + // touched card is where they expect the next range to start from. + toggle(item:SelectionItem, listKey:string):void { + this.rangeBaseline = null; + const key = selectionKey(item); + + if (this.selectedItems.has(key)) { + this.selectedItems.delete(key); + } else { + this.selectedItems.set(key, item); + } + + this.selectionAnchor = { ...item, listKey }; + } + + // The anchor stays put so repeated Shift gestures resize one range rather + // than walking it across the list. + range(rangeItems:readonly SelectionItem[]):void { + this.rangeBaseline ??= new Map(this.selectedItems); + this.selectedItems = new Map(this.rangeBaseline); + for (const item of rangeItems) { + this.selectedItems.set(selectionKey(item), item); + } + } + + selectAll(items:readonly SelectionItem[], anchor:SelectionAnchor|null):void { + this.rangeBaseline = new Map(); + this.selectedItems = new Map(items.map((item) => [selectionKey(item), item])); + this.selectionAnchor = anchor; + } + + clear():void { + this.rangeBaseline = null; + this.selectedItems = new Map(); + this.selectionAnchor = null; + } + + /** + * Points the existing anchor at a different list. + * + * A card can be moved to another list while remaining the anchor — a + * cross-list drag does exactly that. The caller re-derives the key from the + * live document and hands it back. + */ + rebindAnchor(listKey:string):void { + if (this.selectionAnchor && this.selectionAnchor.listKey !== listKey) { + this.rangeBaseline = null; + this.selectionAnchor = { ...this.selectionAnchor, listKey }; + } + } + + /** + * Drops members and an anchor that no longer exist in the document. + * + * `liveKeys` is keyed the same way membership is, so an id that survives + * under a *different* type does not keep a stale member alive. + * + * @return whether membership or the anchor changed, for a caller that can + * skip redundant work when nothing did. + */ + prune(liveKeys:ReadonlySet):boolean { + let changed = false; + + for (const key of [...this.selectedItems.keys()]) { + if (!liveKeys.has(key)) { + this.selectedItems.delete(key); + this.rangeBaseline?.delete(key); + changed = true; + } + } + + if (this.selectionAnchor && !liveKeys.has(selectionKey(this.selectionAnchor))) { + this.selectionAnchor = null; + this.rangeBaseline = null; + changed = true; + } + + return changed; + } +} diff --git a/frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.spec.ts index 9e096f2aed93..dfed73434f28 100644 --- a/frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.spec.ts @@ -68,18 +68,26 @@ describe('Backlogs work package controller', () => { vi.restoreAllMocks(); }); + // The card is wrapped in its row, matching production: `data-batch-selected` + // is written on the row (the
  • ), never on the card itself (see + // sortable-lists/selection.ts). This controller only ever touches the card + // (`this.element`), so wrapping it here is what makes "leaves batch + // membership alone" below a meaningful assertion instead of one that would + // pass unchanged even if the two elements were confused. function renderWorkPackage() { fixture.innerHTML = ` -
    - Work package -
    +
  • +
    + Work package +
    +
    • `; return fixture.querySelector('[data-controller="backlogs--work-package"]')!; @@ -113,6 +121,7 @@ describe('Backlogs work package controller', () => { expect(event.defaultPrevented).toBe(true); expect(navigation.openSplitPane).toHaveBeenCalledTimes(1); expect(navigation.openFullPane).not.toHaveBeenCalled(); + expect(workPackage.hasAttribute('aria-current')).toBe(false); }); it('opens the full pane when Shift+Enter is pressed', async () => { @@ -139,13 +148,12 @@ describe('Backlogs work package controller', () => { expect(navigation.openFullPane).not.toHaveBeenCalled(); }); - it('marks the card as selected immediately on click, before the pane opens', async () => { + it('does not mark a card as current merely because it was clicked', async () => { const workPackage = renderWorkPackage(); await nextFrame(); workPackage.dispatchEvent(new MouseEvent('click', { bubbles: true })); - expect(workPackage.hasAttribute('data-selected')).toBe(true); expect(workPackage.hasAttribute('aria-current')).toBe(false); expect(navigation.openSplitPane).not.toHaveBeenCalled(); }); @@ -169,46 +177,46 @@ describe('Backlogs work package controller', () => { } }); - it('marks the card as selected when Enter opens the split pane', async () => { + it('marks the card as current when the URL points at it', async () => { const workPackage = renderWorkPackage(); await nextFrame(); - keydown(workPackage, 'Enter'); + document.dispatchEvent(new CustomEvent('turbo:visit', { + detail: { url: '/projects/demo/backlogs/details/SP-42' }, + })); - expect(workPackage.hasAttribute('data-selected')).toBe(true); - expect(navigation.openSplitPane).toHaveBeenCalledTimes(1); + expect(workPackage.getAttribute('aria-current')).toBe('true'); }); - it('clears the selection of other work packages when a card is selected', async () => { + it('unmarks the card as current once the URL moves elsewhere', async () => { const workPackage = renderWorkPackage(); - const other = document.createElement('article'); - other.setAttribute('data-controller', 'backlogs--work-package'); - fixture.appendChild(other); await nextFrame(); - other.setAttribute('data-selected', ''); - workPackage.dispatchEvent(new MouseEvent('click', { bubbles: true })); + document.dispatchEvent(new CustomEvent('turbo:visit', { + detail: { url: '/projects/demo/backlogs/details/SP-42' }, + })); + document.dispatchEvent(new CustomEvent('turbo:visit', { + detail: { url: '/projects/demo/backlogs' }, + })); - expect(other.hasAttribute('data-selected')).toBe(false); - expect(workPackage.hasAttribute('data-selected')).toBe(true); + expect(workPackage.hasAttribute('aria-current')).toBe(false); }); - it('syncs selection and aria-current from the visited URL', async () => { + it('leaves batch membership alone when the URL changes', async () => { const workPackage = renderWorkPackage(); + const row = workPackage.parentElement!; await nextFrame(); + row.setAttribute('data-batch-selected', ''); + document.dispatchEvent(new CustomEvent('turbo:visit', { detail: { url: '/projects/demo/backlogs/details/SP-42' }, })); - - expect(workPackage.getAttribute('aria-current')).toBe('true'); - expect(workPackage.hasAttribute('data-selected')).toBe(true); + expect(row.hasAttribute('data-batch-selected')).toBe(true); document.dispatchEvent(new CustomEvent('turbo:visit', { detail: { url: '/projects/demo/backlogs' }, })); - - expect(workPackage.hasAttribute('aria-current')).toBe(false); - expect(workPackage.hasAttribute('data-selected')).toBe(false); + expect(row.hasAttribute('data-batch-selected')).toBe(true); }); }); diff --git a/frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.ts b/frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.ts index 27f037fdc55e..7821095d7000 100644 --- a/frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.ts @@ -30,6 +30,7 @@ import { Controller } from '@hotwired/stimulus'; import * as Turbo from '@hotwired/turbo'; import type { TurboVisitEvent } from '@hotwired/turbo'; import { WP_ID_URL_PATTERN } from 'core-app/shared/helpers/work-package-id-pattern'; +import { closestInteractiveElement } from 'core-common/interactive-element-helper'; const DETAILS_URL_PATTERN = new RegExp(`/details/(${WP_ID_URL_PATTERN})(?:/|$)`); @@ -57,10 +58,10 @@ export default class WorkPackageController extends Controller imple this.element.addEventListener('dblclick', this, { signal }); this.element.addEventListener('keydown', this, { signal }); document.addEventListener('turbo:visit', (event:TurboVisitEvent) => { - this.syncSelectionFromUrl(event.detail.url); + this.syncCurrentFromUrl(event.detail.url); }, { signal }); - this.syncSelectionFromUrl(window.location.href); + this.syncCurrentFromUrl(window.location.href); } disconnect():void { @@ -73,40 +74,21 @@ export default class WorkPackageController extends Controller imple } } - private syncSelectionFromUrl(locationUrl:string):void { + private syncCurrentFromUrl(locationUrl:string):void { const { pathname } = new URL(locationUrl, window.location.origin); const [, id] = DETAILS_URL_PATTERN.exec(pathname) ?? []; // Bookmarks and external links may still carry a numeric ID after the // switch to semantic mode, so accept either form here. if (id !== undefined && (id === this.idValue.toString() || id === this.displayIdValue)) { - this.markAsSelected(); this.markAsCurrent(); } else { - this.unmarkAsSelected(); this.unmarkAsCurrent(); } } - // Selection is applied synchronously on activation, before the split screen - // has loaded, so the list reacts immediately. With single selection any - // previously selected work package is cleared right away instead of waiting - // for its own URL sync. - markAsSelected():void { - document - .querySelectorAll('[data-controller~="backlogs--work-package"][data-selected]') - .forEach((other) => { - if (other !== this.element) { - other.removeAttribute('data-selected'); - } - }); - - this.element.setAttribute('data-selected', ''); - } - - unmarkAsSelected():void { - this.element.removeAttribute('data-selected'); - } - + // Not set optimistically: activation waits out the double-click delay below + // and may resolve to the full view instead, so asserting a current work + // package here would announce a navigation that may never happen. markAsCurrent():void { this.element.setAttribute('aria-current', 'true'); } @@ -133,12 +115,10 @@ export default class WorkPackageController extends Controller imple const target = event.target; if (!(target instanceof HTMLElement)) return; - if (this.shouldIgnoreMouseTarget(target)) return; + if (this.shouldIgnoreTarget(target)) return; if (this.clickTimeout !== null) return; - this.markAsSelected(); - this.clickTimeout = window.setTimeout(() => { this.clickTimeout = null; this.openSplitPane(); @@ -149,7 +129,7 @@ export default class WorkPackageController extends Controller imple const target = event.target; if (!(target instanceof HTMLElement)) return; - if (this.shouldIgnoreMouseTarget(target)) return; + if (this.shouldIgnoreTarget(target)) return; if (this.clickTimeout !== null) { clearTimeout(this.clickTimeout); @@ -165,14 +145,13 @@ export default class WorkPackageController extends Controller imple const target = event.target; if (!(target instanceof HTMLElement)) return; - if (this.shouldIgnoreKeyboardTarget(target)) return; + if (this.shouldIgnoreTarget(target)) return; event.preventDefault(); if (event.shiftKey) { this.openFullPane(); } else { - this.markAsSelected(); this.openSplitPane(); } } @@ -185,20 +164,9 @@ export default class WorkPackageController extends Controller imple Turbo.visit(this.fullUrlValue, { frame: '_top' }); } - private shouldIgnoreMouseTarget(target:HTMLElement):boolean { - return [ - 'a', - 'button', - 'clipboard-copy', - ].some((selector) => target.closest(selector) !== null); - } - - private shouldIgnoreKeyboardTarget(target:HTMLElement):boolean { - return this.shouldIgnoreMouseTarget(target) || [ - 'input', - 'textarea', - 'select', - "[contenteditable='true']", - ].some((selector) => target.closest(selector) !== null); + // The same rule the selection orchestrator applies: a control inside the + // card is that control first. The card itself is the boundary, not a hit. + private shouldIgnoreTarget(target:HTMLElement):boolean { + return closestInteractiveElement(target, this.element) !== null; } } diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts index 76c264e3aa49..814d026f2a71 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts @@ -60,6 +60,7 @@ import { type Mock, type MockInstance } from 'vitest'; import { LiveRegionElement } from '@primer/live-region-element'; import { setupStimulusTest, type StimulusTestContext } from 'core-stimulus/test-helpers'; import type SortableListsControllerType from './sortable-lists.controller'; +import { selectionTranslations } from './sortable-lists/testing/selection-translations'; import type { sortableItemData as sortableItemDataFn, sortableListData as sortableListDataFn, @@ -81,6 +82,7 @@ describe('Sortable lists controller', () => { // cannot resolve `.mock.calls`'s element type from; pin the spied method's // own signature instead so calls stay typed. let announceSpy:MockInstance; + let userAgentDataDescriptor:PropertyDescriptor|undefined; beforeAll(async () => { ({ monitorForElements } = await import('@atlaskit/pragmatic-drag-and-drop/element/adapter')); @@ -119,13 +121,15 @@ describe('Sortable lists controller', () => { function renderFixture({ moveUrlTemplate = '/move/{id}', optimistic = false, - }:{ moveUrlTemplate?:string|null; optimistic?:boolean } = {}) { + selectionEnabled = false, + }:{ moveUrlTemplate?:string|null; optimistic?:boolean; selectionEnabled?:boolean } = {}) { fixture.innerHTML = `
    { sourceList.append(itemRow('1'), itemRow('2'), itemRow('3')); targetList.append(itemRow('4'), itemRow('5')); return { - root, sourceList, targetList, scrollable, firstSourceItem: sourceList.querySelector('[data-sortable-lists--item-id-value="1"]')!, + root, + sourceList, + targetList, + scrollable, + firstSourceItem: sourceList.querySelector('[data-sortable-lists--item-id-value="1"]')!, + items: Array.from(root.querySelectorAll('[data-sortable-lists--item-id-value]')), }; } @@ -316,9 +325,32 @@ describe('Sortable lists controller', () => { }; } + // Layered on renderFixture, plus selection and a tabindex per item. The + // value is set at creation time: Stimulus picks up an attribute mutated + // after connect through its async MutationObserver channel, too late for a + // synchronous test. The tabindex mirrors Backlogs cards, whose item element + // is itself focusable. + function renderSelectableRoot({ + moveUrlTemplate = '/move/{id}', + }:{ moveUrlTemplate?:string|null } = {}) { + const fixtureElements = renderFixture({ moveUrlTemplate, selectionEnabled: true }); + fixtureElements.items.forEach((item) => item.setAttribute('tabindex', '0')); + + return fixtureElements; + } + beforeEach(async () => { vi.clearAllMocks(); + // Multi-select gestures below use Ctrl, which is only the multi-select + // modifier off Apple platforms — pin the platform so the suite behaves + // the same on a macOS workstation and on Linux CI. + userAgentDataDescriptor = Object.getOwnPropertyDescriptor(navigator, 'userAgentData'); + Object.defineProperty(navigator, 'userAgentData', { + value: { platform: 'Windows' }, + configurable: true, + }); + // The synthetic drop input below carries fixed coordinates that bear no // relation to where the fixture's rows actually lay out, so a real // hit-test would report the source's own row for every drop and the @@ -353,6 +385,7 @@ describe('Sortable lists controller', () => { moved: '%{label} moved to position %{position} of %{total}', moved_to_list: '%{label} moved to %{list}, position %{position} of %{total}', }, + selection: selectionTranslations, }, }, }, @@ -374,6 +407,11 @@ describe('Sortable lists controller', () => { document.body.querySelector('live-region')?.remove(); vi.restoreAllMocks(); vi.unstubAllGlobals(); + if (userAgentDataDescriptor) { + Object.defineProperty(navigator, 'userAgentData', userAgentDataDescriptor); + } else { + delete (navigator as { userAgentData?:unknown }).userAgentData; + } }); it('moves a list-only drop onto the source list to its configured position', async () => { @@ -1101,6 +1139,19 @@ describe('Sortable lists controller', () => { expect(body.body.get('prev_id')).toBe('2'); }); + it('refuses a directional move for a non-movable item', async () => { + const { root, sourceList, firstSourceItem } = renderFixture(); + firstSourceItem.setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + await ctx.nextFrame(); + + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + controller.moveInDirection(firstSourceItem, 'down'); + await flushPromises(); + + expect(itemIds(sourceList)).toEqual(['1', '2', '3']); + expect(fetchMock).not.toHaveBeenCalled(); + }); + it('reports per-direction move availability for gating', async () => { const { root, firstSourceItem } = renderFixture(); await ctx.nextFrame(); @@ -1112,6 +1163,19 @@ describe('Sortable lists controller', () => { }); }); + // A card that takes no part in ordering must not be offered a move the + // click path would then refuse. + it('reports no available direction for a non-movable item', async () => { + const { root, firstSourceItem } = renderFixture(); + firstSourceItem.setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + await ctx.nextFrame(); + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + + expect(controller.moveAvailability(firstSourceItem)).toEqual({ + top: false, up: false, down: false, bottom: false, + }); + }); + it('reports null availability for an item outside any owned list', async () => { const { root } = renderFixture(); await ctx.nextFrame(); @@ -1227,4 +1291,963 @@ describe('Sortable lists controller', () => { ); }); }); + + // Collapsing a batch and selecting the dragged card are different things: + // with nothing selected, a drag must not manufacture a one-card batch. + it('leaves an empty selection empty when a drag starts with nothing selected', async () => { + const { root, firstSourceItem } = renderSelectableRoot(); + await ctx.nextFrame(); + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + + controller.collapseSelectionForDrag(firstSourceItem); + + expect(document.querySelector('[data-batch-selected]')).toBeNull(); + }); + + // A drag that narrows a larger batch to one card is a count change a + // screen-reader user has to hear. + it('announces the new count when a drag collapses a multi-card batch', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + items[0].dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + items[1].dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true, ctrlKey: true })); + items[2].dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true, ctrlKey: true })); + announceSpy.mockClear(); + + controller.collapseSelectionForDrag(items[0]); + + expect(items.filter((item) => item.hasAttribute('data-batch-selected'))).toEqual([items[0]]); + expect(announceSpy.mock.calls.map((call) => [call[0], call[1]])).toEqual([ + ['[selected:1]', { politeness: 'polite' }], + ]); + }); + + const click = (element:HTMLElement, init:MouseEventInit = {}) => { + element.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true, ...init })); + }; + + // A gesture the root leaves alone keeps its native default, so a real + // anchor would navigate the test page away. Cancelled in the bubble phase, + // after the root's capture-phase listener has had its turn. + const clickWithoutNavigating = (element:HTMLElement, init:MouseEventInit = {}) => { + const blockNavigation = (event:Event) => event.preventDefault(); + window.addEventListener('click', blockNavigation, { once: true }); + + click(element, init); + + window.removeEventListener('click', blockNavigation); + }; + + const isSelected = (element:HTMLElement) => element.hasAttribute('data-batch-selected'); + + // A restored page brings its markup back but not the model. Not driven + // through turbo:before-cache: that also fires for the details-pane + // navigation, where clearing would strip a live batch. + it('clears stale batch presentation left in the DOM when it connects', async () => { + fixture.innerHTML = ` +
    +
      +
    • +
    +
    + `; + const stale = fixture.querySelector('#stale-root')!; + const row = stale.querySelector('[data-sortable-lists--item-id-value="1"]')!; + expect(row.hasAttribute('data-batch-selected')).toBe(true); + + await ctx.nextFrame(); + + expect(row.hasAttribute('data-batch-selected')).toBe(false); + }); + + it('selects only the clicked card on a plain click', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + + click(items[0]); + + expect(isSelected(items[0])).toBe(true); + expect(isSelected(items[1])).toBe(false); + }); + + it('lets a plain click continue to the navigation handler', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + const event = new MouseEvent('click', { bubbles: true, cancelable: true }); + + items[0].dispatchEvent(event); + + expect(event.defaultPrevented).toBe(false); + }); + + it('lets a plain click on a non-movable card continue to navigate without selecting', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + const event = new MouseEvent('click', { bubbles: true, cancelable: true }); + + items[0].dispatchEvent(event); + + expect(isSelected(items[0])).toBe(false); + expect(event.defaultPrevented).toBe(false); + }); + + // A fixed card does not join the batch, but the click still opens its + // details pane, so it must not leave an unrelated batch behind. + it('clears an existing batch on a plain click that lands on a non-movable card', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + click(items[1], { ctrlKey: true }); + items[2].setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + const event = new MouseEvent('click', { bubbles: true, cancelable: true }); + + items[2].dispatchEvent(event); + + expect(items.some(isSelected)).toBe(false); + expect(event.defaultPrevented).toBe(false); + }); + + it('announces the new count when a plain click collapses a multi-card batch', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + click(items[1], { ctrlKey: true }); + click(items[2], { ctrlKey: true }); + announceSpy.mockClear(); + + click(items[0]); + + expect(items.filter(isSelected)).toEqual([items[0]]); + expect(announcedMessages()).toEqual([ + ['[selected:1]', { politeness: 'polite' }], + ]); + }); + + // The click also opens the details pane, which is its own feedback. + it('stays silent on a plain click that does not change the selected count', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + announceSpy.mockClear(); + + click(items[1]); + + expect(items.filter(isSelected)).toEqual([items[1]]); + expect(announceSpy).not.toHaveBeenCalled(); + }); + + // An unconsumed modified gesture would reach the card's own click handler + // and open the details pane on a selection toggle. + it('consumes a modified click during a busy move without changing the selection', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + root.setAttribute('data-sortable-lists-busy', 'true'); + const event = new MouseEvent('click', { bubbles: true, cancelable: true, ctrlKey: true }); + + items[0].dispatchEvent(event); + + expect(event.defaultPrevented).toBe(true); + expect(items.some(isSelected)).toBe(false); + }); + + it('toggles a card without navigating on a modified click', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + const event = new MouseEvent('click', { bubbles: true, cancelable: true, ctrlKey: true }); + + click(items[0]); + items[1].dispatchEvent(event); + + expect(isSelected(items[0])).toBe(true); + expect(isSelected(items[1])).toBe(true); + expect(event.defaultPrevented).toBe(true); + }); + + it('selects a range from the anchor on a shift click', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + + click(items[0]); + click(items[2], { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[0], items[1], items[2]]); + }); + + it('keeps the anchor fixed so a range can be resized', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + + click(items[0]); + click(items[2], { shiftKey: true }); + click(items[1], { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[0], items[1]]); + }); + + it('starts a single selection when shift is pressed without an anchor', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + + click(items[1], { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[1]]); + }); + + // The count rule alone would fall silent whenever the prior selection was + // already a single card, leaving the failed range with no feedback. + it('announces a distinct message when a cross-list Shift-click restarts the range', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + announceSpy.mockClear(); + + click(items[3], { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[3]]); + expect(announcedMessages()).toEqual([ + ['[range_restarted:1]', { politeness: 'polite' }], + ]); + }); + + // Same restart, reached the other way: an anchor pruned after a morph + // leaves a one-card selection with nothing to range from. + it('announces a distinct message when a Shift-click restarts the range after its anchor was pruned', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + click(items[1], { ctrlKey: true }); + items[1].remove(); + morphRoot(root); + await ctx.nextFrame(); + announceSpy.mockClear(); + + click(items[2], { shiftKey: true }); + + // items[1] was removed from the document above; a stale reference to it + // would still carry the attribute it had before removal, so membership + // is checked on the two live cards rather than filtering the whole + // `items` array. + expect(isSelected(items[0])).toBe(false); + expect(isSelected(items[2])).toBe(true); + expect(announcedMessages()).toEqual([ + ['[range_restarted:1]', { politeness: 'polite' }], + ]); + }); + + it('tells the user to expand the list when a range crosses a truncation marker', async () => { + const { sourceList, items } = renderSelectableRoot(); + await ctx.nextFrame(); + const marker = document.createElement('li'); + marker.setAttribute('data-sortable-lists-prev-item-id', '1'); + marker.setAttribute('data-sortable-lists-omitted-count', '9'); + sourceList.insertBefore(marker, items[1]); + + click(items[0]); + announceSpy.mockClear(); + click(items[2], { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[0]]); + expect(announcedMessages()).toEqual([ + ['[range_unavailable]', { politeness: 'polite' }], + ]); + }); + + // Expanding the list can never resolve a non-movable card in the span. + it('tells the user a locked card blocks the range rather than to expand the list', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[1].setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + + click(items[0]); + announceSpy.mockClear(); + click(items[2], { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[0]]); + expect(announcedMessages()).toEqual([ + ['[range_blocked]', { politeness: 'polite' }], + ]); + }); + + it('preserves the batch and announces when a non-movable card is modifier-clicked', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[1].setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + const event = new MouseEvent('click', { bubbles: true, cancelable: true, ctrlKey: true }); + + click(items[0]); + announceSpy.mockClear(); + items[1].dispatchEvent(event); + + expect(items.filter(isSelected)).toEqual([items[0]]); + expect(event.defaultPrevented).toBe(true); + expect(announcedMessages()).toEqual([ + ['[not_selectable]', { politeness: 'polite' }], + ]); + }); + + it('ignores gestures that start on an interactive descendant', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + const link = document.createElement('a'); + link.href = '/somewhere'; + items[0].appendChild(link); + + clickWithoutNavigating(link, { ctrlKey: true }); + + expect(items.some(isSelected)).toBe(false); + }); + + // Stopping the walk at the item would classify every gesture on a Backlogs + // card as interactive, since the card itself carries tabindex. Only + // observable once the focus host is a distinct, nested element. + it('selects when a gesture lands on a non-interactive descendant of a nested focus host', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + const focusHost = document.createElement('div'); + focusHost.setAttribute('data-sortable-lists--item-target', 'focus'); + focusHost.tabIndex = 0; + const label = document.createElement('span'); + focusHost.appendChild(label); + items[0].appendChild(focusHost); + + click(label); + + expect(isSelected(items[0])).toBe(true); + }); + + // A nested focus host sits below the item, so a gesture landing elsewhere + // on the row never reaches it walking up: stopping there unconditionally + // would leave the walk unbounded past the item. + it('still selects when a gesture lands on the row outside a nested focus host', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + const focusHost = document.createElement('div'); + focusHost.setAttribute('data-sortable-lists--item-target', 'focus'); + focusHost.tabIndex = 0; + items[0].appendChild(focusHost); + + click(items[0]); + + expect(isSelected(items[0])).toBe(true); + }); + + it('does not select at all when selection is not enabled', async () => { + const { items } = renderFixture(); + await ctx.nextFrame(); + + click(items[0]); + + expect(items.some(isSelected)).toBe(false); + }); + + // A Turbo morph can flip the permission-gated value on a live root without + // reconnecting the controller, so the gestures follow the flip. + it('stops selecting when selection is disabled on the live root', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + expect(isSelected(items[0])).toBe(true); + + root.setAttribute('data-sortable-lists-selection-enabled-value', 'false'); + await ctx.nextFrame(); + + expect(items.some(isSelected)).toBe(false); + click(items[1]); + expect(items.some(isSelected)).toBe(false); + }); + + it('starts selecting when selection is enabled on the live root', async () => { + const { root, items } = renderFixture(); + await ctx.nextFrame(); + click(items[0]); + expect(items.some(isSelected)).toBe(false); + + items.forEach((item) => item.setAttribute('tabindex', '0')); + root.setAttribute('data-sortable-lists-selection-enabled-value', 'true'); + await ctx.nextFrame(); + + click(items[0]); + expect(isSelected(items[0])).toBe(true); + }); + + // A cached root re-attaching reconnects the same controller instance, and + // Stimulus fires no value callback for an unchanged attribute. + it('restores selection when the same root reconnects', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + expect(isSelected(items[0])).toBe(true); + + root.remove(); + await ctx.nextFrame(); + fixture.append(root); + await ctx.nextFrame(); + + click(items[1]); + expect(isSelected(items[1])).toBe(true); + }); + + const keydown = (target:HTMLElement, key:string, init:KeyboardEventInit = {}) => { + const event = new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...init }); + target.dispatchEvent(event); + return event; + }; + + it('toggles the focused card on Space', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + + const event = keydown(items[0], ' '); + + expect(isSelected(items[0])).toBe(true); + expect(event.defaultPrevented).toBe(true); + }); + + it('extends the range on Shift+Space', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + items[2].focus(); + + keydown(items[2], ' ', { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[0], items[1], items[2]]); + }); + + it('moves focus within the list on ArrowDown', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + + keydown(items[0], 'ArrowDown'); + + expect(document.activeElement).toBe(items[1]); + }); + + // An unconsumed arrow falls through to native scrolling, moving the page + // while focus stays put. + it('consumes ArrowUp at the first card even though focus cannot move', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + + const event = keydown(items[0], 'ArrowUp'); + + expect(document.activeElement).toBe(items[0]); + expect(event.defaultPrevented).toBe(true); + }); + + it('consumes ArrowDown at the last card of a list even though focus cannot move', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[2].focus(); + + const event = keydown(items[2], 'ArrowDown'); + + expect(document.activeElement).toBe(items[2]); + expect(event.defaultPrevented).toBe(true); + }); + + it('extends the range while moving focus on Shift+ArrowDown', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + items[0].focus(); + + keydown(items[0], 'ArrowDown', { shiftKey: true }); + + expect(document.activeElement).toBe(items[1]); + expect(items.filter(isSelected)).toEqual([items[0], items[1]]); + }); + + it('moves focus to the list boundaries on Home and End', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[1].focus(); + + keydown(items[1], 'End'); + expect(document.activeElement).toBe(items[2]); + + keydown(items[2], 'Home'); + expect(document.activeElement).toBe(items[0]); + }); + + // Same boundary-scroll problem as the plain arrows above. + it('consumes Home at the first card even though focus cannot move', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + + const event = keydown(items[0], 'Home'); + + expect(document.activeElement).toBe(items[0]); + expect(event.defaultPrevented).toBe(true); + }); + + it('consumes End at the last card even though focus cannot move', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[2].focus(); + + const event = keydown(items[2], 'End'); + + expect(document.activeElement).toBe(items[2]); + expect(event.defaultPrevented).toBe(true); + }); + + // The design's keyboard table specifies the first/last *movable* card for + // Home/End (unlike plain arrow steps, which are not qualified that way). + it('skips a non-movable card when moving to the list boundary on End', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[2].setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + items[0].focus(); + + keydown(items[0], 'End'); + + expect(document.activeElement).toBe(items[1]); + }); + + // Focus cannot move further, but the range still resizes to the boundary. + it('resizes the range to the list boundary on Shift+Home even when focus cannot move', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[2]); + items[2].focus(); + keydown(items[2], 'ArrowUp'); + keydown(items[1], 'ArrowUp'); + + keydown(items[0], 'Home', { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[0], items[1], items[2]]); + }); + + // The fixture spans two lists: items[0..2] in the source, items[3..4] in + // the target. + it('selects every movable card in the focused list on Ctrl+A', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[1].setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + items[0].focus(); + + keydown(items[0], 'a', { ctrlKey: true }); + + const selected = items.filter(isSelected); + expect(selected).toEqual([items[0], items[2]]); + expect(selected).not.toContain(items[1]); + }); + + // Would pass under a root-wide select-all. + it('leaves movable cards of other lists unselected on Ctrl+A', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + + keydown(items[0], 'a', { ctrlKey: true }); + + expect(isSelected(items[3])).toBe(false); + expect(isSelected(items[4])).toBe(false); + }); + + it('selects every movable card of the focused list on ctrl A', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + + keydown(items[0], 'a', { ctrlKey: true }); + + expect(items.filter(isSelected)).toEqual([items[0], items[1], items[2]]); + }); + + it('does nothing on a bare "a" without a modifier', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + + const event = keydown(items[0], 'a'); + + expect(items.some(isSelected)).toBe(false); + expect(event.defaultPrevented).toBe(false); + }); + + // Focusing something other than the first card rules out an anchor that + // merely coincides with a document-order fallback. + it('anchors the batch on the focused card after Ctrl+A when it is movable', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[2].focus(); + + keydown(items[2], 'a', { ctrlKey: true }); + keydown(items[0], ' ', { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[0], items[1], items[2]]); + }); + + // A subsequent Shift gesture proves the fallback anchor is set: no anchor + // at all would collapse the range to a single card. + it('anchors the batch on the first movable card after Ctrl+A when the focused card is not movable', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + items[0].focus(); + + keydown(items[0], 'a', { ctrlKey: true }); + keydown(items[2], ' ', { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[1], items[2]]); + }); + + it('clears the batch on Escape', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + items[0].focus(); + + keydown(items[0], 'Escape'); + + expect(items.some(isSelected)).toBe(false); + }); + + // Escape clears local state only, so the busy gate must not swallow it. + it('clears the batch on Escape even during a busy move', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + items[0].focus(); + root.setAttribute('data-sortable-lists-busy', 'true'); + + keydown(items[0], 'Escape'); + + expect(items.some(isSelected)).toBe(false); + }); + + // An unconsumed Escape still reaches dialogs and menus. + it('leaves Escape unconsumed when there is nothing to clear', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + + const event = keydown(items[0], 'Escape'); + + expect(event.defaultPrevented).toBe(false); + }); + + // BatchSelection#toggle re-bases the anchor even on a deselect, so an + // emptied selection leaves one behind. The Shift gesture below would range + // from it had Escape not dropped it. + it('clears the stale anchor left by a deselect, not just a visible selection', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + keydown(items[0], ' '); + keydown(items[0], ' '); + + keydown(items[0], 'Escape'); + keydown(items[2], ' ', { shiftKey: true }); + + expect(items.filter(isSelected)).toEqual([items[2]]); + }); + + // A click on empty column space focuses nothing, and Escape there still + // means "drop the selection". + it('clears the batch on Escape when focus has left the root entirely', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + + keydown(document.body, 'Escape'); + + expect(items.some(isSelected)).toBe(false); + }); + + // An interactive descendant keeps its own clicks and Space, but Escape + // still belongs to the selection. + it('clears the batch on Escape from a link inside a card', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + const link = document.createElement('a'); + link.href = '#'; + items[1].append(link); + + keydown(link, 'Escape'); + + expect(items.some(isSelected)).toBe(false); + }); + + it('leaves Escape inside an open menu to the menu', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + const menu = document.createElement('div'); + menu.setAttribute('role', 'menu'); + const menuButton = document.createElement('button'); + menu.append(menuButton); + root.append(menu); + + const event = keydown(menuButton, 'Escape'); + + expect(isSelected(items[0])).toBe(true); + expect(event.defaultPrevented).toBe(false); + }); + + it('ignores an Escape another handler already consumed', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + items[1].addEventListener('keydown', (event) => event.preventDefault(), { once: true }); + + keydown(items[1], 'Escape'); + + expect(isSelected(items[0])).toBe(true); + }); + + // Focus routinely rests in the details pane or the page header after a + // selection; Escape there still means "drop the selection". + it('clears the batch on Escape from a control outside the root', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + const button = document.createElement('button'); + document.body.appendChild(button); + button.focus(); + + try { + keydown(button, 'Escape'); + } finally { + button.remove(); + } + + expect(items.some(isSelected)).toBe(false); + }); + + it('leaves Escape to a text field outside the root', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + const input = document.createElement('input'); + document.body.appendChild(input); + input.focus(); + + try { + const event = keydown(input, 'Escape'); + expect(event.defaultPrevented).toBe(false); + } finally { + input.remove(); + } + + expect(isSelected(items[0])).toBe(true); + }); + + it('leaves Escape to an open dialog', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + const dialog = document.createElement('dialog'); + const button = document.createElement('button'); + dialog.appendChild(button); + document.body.appendChild(dialog); + dialog.showModal(); + + try { + const event = keydown(button, 'Escape'); + expect(event.defaultPrevented).toBe(false); + } finally { + dialog.close(); + dialog.remove(); + } + + expect(isSelected(items[0])).toBe(true); + }); + + // A menu opens its popover before moving focus into it on the next frame, + // so an Escape in between still targets the invoker inside the card. + it('leaves Escape to an open popover while focus is still on its invoker', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + const invoker = document.createElement('button'); + const popover = document.createElement('div'); + popover.setAttribute('popover', ''); + items[1].append(invoker, popover); + popover.showPopover(); + invoker.focus(); + + try { + const event = keydown(invoker, 'Escape'); + expect(event.defaultPrevented).toBe(false); + } finally { + popover.hidePopover(); + } + + expect(isSelected(items[0])).toBe(true); + }); + + it('clears the batch on Escape once the popover has closed', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + const popover = document.createElement('div'); + popover.setAttribute('popover', ''); + items[1].appendChild(popover); + popover.showPopover(); + popover.hidePopover(); + + keydown(items[1], 'Escape'); + + expect(isSelected(items[0])).toBe(false); + }); + + it('leaves Enter to the navigation handler', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[0].focus(); + + const event = keydown(items[0], 'Enter'); + + expect(event.defaultPrevented).toBe(false); + }); + + it('ignores keys from an interactive descendant', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + const input = document.createElement('input'); + items[0].appendChild(input); + input.focus(); + + keydown(input, ' '); + + expect(items.some(isSelected)).toBe(false); + }); + + function morphRoot(root:HTMLElement) { + root.dispatchEvent(new CustomEvent('turbo:morph-element', { bubbles: true })); + } + + // A frame navigation, such as the "show more" expander, swaps the rows + // wholesale without a morph event; the fresh rows arrive unmarked. + describe('reconciling the batch after rows are replaced', () => { + const replaceRows = (items:HTMLElement[], keep:(item:HTMLElement) => boolean) => { + const rowsContainer = items[0].parentElement!; + const fresh = items.filter(keep).map((item) => { + const clone = item.cloneNode(true) as HTMLElement; + clone.removeAttribute('data-batch-selected'); + return clone; + }); + rowsContainer.replaceChildren(...fresh); + return fresh; + }; + + it('repaints the surviving members on the fresh rows without announcing', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + click(items[1], { ctrlKey: true }); + announceSpy.mockClear(); + + const fresh = replaceRows(items, () => true); + await ctx.nextFrame(); + + expect(fresh.filter(isSelected)).toEqual([fresh[0], fresh[1]]); + expect(announceSpy).not.toHaveBeenCalled(); + }); + + it('prunes a member the replacement dropped and announces the new count', async () => { + const { items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + click(items[1], { ctrlKey: true }); + announceSpy.mockClear(); + + const fresh = replaceRows(items, (item) => item !== items[1]); + await ctx.nextFrame(); + + expect(fresh.filter(isSelected)).toEqual([fresh[0]]); + expect(announcedMessages()).toEqual([ + ['[selected:1]', { politeness: 'polite' }], + ]); + }); + }); + + describe('reconciling the batch after a morph', () => { + it('reapplies the batch presentation after a morph strips it', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + items[0].removeAttribute('data-batch-selected'); + + morphRoot(root); + await ctx.nextFrame(); + + expect(isSelected(items[0])).toBe(true); + }); + + it('clears a stale marker the morph preserved', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + items[2].setAttribute('data-batch-selected', ''); + + morphRoot(root); + await ctx.nextFrame(); + + expect(isSelected(items[2])).toBe(false); + }); + + it('announces the new count when a morph prune removes a selected card', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + click(items[1], { ctrlKey: true }); + click(items[2], { ctrlKey: true }); + items[1].remove(); + announceSpy.mockClear(); + + morphRoot(root); + await ctx.nextFrame(); + + expect(announcedMessages()).toEqual([ + ['[selected:2]', { politeness: 'polite' }], + ]); + }); + + // renderSelection runs on every morph and decides from membership alone, + // so a prune that drops nothing has to stay silent by itself. + it('stays silent when a morph prunes nothing', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + click(items[1], { ctrlKey: true }); + announceSpy.mockClear(); + + morphRoot(root); + await ctx.nextFrame(); + + expect(announceSpy).not.toHaveBeenCalled(); + }); + + // selectedIds() filters to elements still in the document, so it would + // pass even with the model unpruned. The anchor is the one place an + // unpruned model is observable: a dangling one makes the Shift+click + // below report an unavailable range instead of restarting the selection. + it('drops a removed member and its stale anchor from the model', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + click(items[0]); + click(items[1], { ctrlKey: true }); + items[1].remove(); + + morphRoot(root); + await ctx.nextFrame(); + click(items[2], { shiftKey: true }); + + expect(isSelected(items[0])).toBe(false); + expect(isSelected(items[2])).toBe(true); + }); + }); }); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts index 2c5b1df1d9d2..6458d539bbe9 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts @@ -47,6 +47,7 @@ import { } from './sortable-lists/drag-and-drop'; import { captureRowPositions, + isOrderableItem, reorderRows, resolveDirectionalPreviousItemId, resolveItemId, @@ -61,23 +62,27 @@ import { type MoveAvailability, type MoveDirection, } from './sortable-lists/list-dom'; +import { SelectionOrchestrator, type SelectionHost } from './sortable-lists/selection-orchestrator'; type CleanupFn = () => void; type ElementDropPayload = ElementEventPayloadMap['onDrop']; type MoveResult = { ok:true }|{ ok:false; showToast:boolean }; interface MoveAnnouncementContext { label:string|null; listName:string|null; crossList:boolean } -export default class SortableListsController extends Controller implements SortableListsRoot { +export default class SortableListsController extends Controller implements SortableListsRoot, SelectionHost { static outlets = ['sortable-lists--list', 'sortable-lists--item', 'sortable-lists--scrollable']; static values = { moveUrlTemplate: String, moveUrlTemplates: Object, optimistic: { type: Boolean, default: false }, + selectionEnabled: { type: Boolean, default: false }, + announcementScope: { type: String, default: 'js.sortable_lists.selection' }, + selectionDescriptionId: { type: String, default: '' }, }; declare readonly sortableListsListOutlets:import('./sortable-lists/list.controller').default[]; - declare readonly sortableListsItemOutlets:RootAwareChild[]; + declare readonly sortableListsItemOutlets:(RootAwareChild & { focusItem():void })[]; declare readonly sortableListsScrollableOutlets:RootAwareChild[]; declare readonly moveUrlTemplateValue:string; @@ -85,9 +90,15 @@ export default class SortableListsController extends Controller imp declare readonly moveUrlTemplatesValue:Record; declare readonly hasMoveUrlTemplatesValue:boolean; declare readonly optimisticValue:boolean; + declare readonly selectionEnabledValue:boolean; + declare readonly announcementScopeValue:string; + declare readonly selectionDescriptionIdValue:string; + + private selection?:SelectionOrchestrator; private monitorCleanupFn?:CleanupFn; private healScheduled = false; + private reconcileScheduled = false; connect():void { this.monitorCleanupFn = monitorForElements({ @@ -99,14 +110,115 @@ export default class SortableListsController extends Controller imp }, }); this.element.addEventListener('turbo:morph-element', this.scheduleRegistrationHeal); + // The value callback misses a reconnect of the same controller instance: + // Stimulus fires it only when the attribute string changes, and a cached + // root re-attaching still carries the old one. setupSelection is guarded + // against the overlap. + if (this.selectionEnabledValue) { + this.setupSelection(); + } } disconnect():void { this.element.removeEventListener('turbo:morph-element', this.scheduleRegistrationHeal); + this.teardownSelection(); this.monitorCleanupFn?.(); this.monitorCleanupFn = undefined; } + // A Turbo morph can toggle the permission-gated value on a live root + // without reconnecting the controller, so the gestures follow the flip + // rather than waiting for one. + selectionEnabledValueChanged():void { + if (this.selectionEnabledValue) { + this.setupSelection(); + } else { + this.teardownSelection(); + } + } + + // Constructed only for a consumer that opted in, listeners and all: a + // root that never opted in must not swallow Space or the arrows. + private setupSelection():void { + // The callback also fires for a later write of the same value, which + // must not stack listeners or replace the live model. + if (this.selection) { + return; + } + + this.selection = new SelectionOrchestrator(this); + // Capture phase: a modified gesture has to be consumed before the + // card's own navigation listener sees it, whichever connected first. + this.element.addEventListener('click', this.onSelectionClick, true); + this.element.addEventListener('keydown', this.onSelectionKeydown, true); + // At the document, in the bubble phase: clearing must not depend on + // focus sitting on a card row, and an overlay's Escape runs first. + document.addEventListener('keydown', this.onSelectionEscape); + // A restored page brings its markup back but not the model, so batch + // presentation present at connect time claims a selection nothing + // holds. Not `turbo:before-cache`: that also fires for the details-pane + // navigation that morphs this page in place, where the controller never + // went away and the highlight has to survive. + this.selection.clearPresentation(); + } + + private teardownSelection():void { + if (!this.selection) { + return; + } + + this.element.removeEventListener('click', this.onSelectionClick, true); + this.element.removeEventListener('keydown', this.onSelectionKeydown, true); + document.removeEventListener('keydown', this.onSelectionEscape); + this.selection.teardown(); + this.selection = undefined; + } + + private readonly onSelectionClick = (event:MouseEvent):void => { + this.selection?.handleClick(event); + }; + + private readonly onSelectionKeydown = (event:KeyboardEvent):void => { + this.selection?.handleKeydown(event); + }; + + private readonly onSelectionEscape = (event:KeyboardEvent):void => { + this.selection?.handleEscape(event); + }; + + get rootElement():HTMLElement { + return this.element; + } + + get announcementScope():string { + return this.announcementScopeValue; + } + + get descriptionId():string { + return this.selectionDescriptionIdValue; + } + + // Through the item's own outlet, so the consumer decides which element + // inside the row holds the tab stop. + focusItem(target:HTMLElement):void { + const outlet = this.sortableListsItemOutlets.find((item) => item.element === target); + + if (outlet) { + outlet.focusItem(); + } else { + target.focus(); + } + } + + // Live ordered membership, for AGILE-278's batch move. + selectedIds():string[] { + return this.selection?.selectedIds() ?? []; + } + + collapseSelectionForDrag(itemElement:HTMLElement):void { + this.selection?.collapseForDrag(itemElement); + } + // A morph desyncs the children's drag-and-drop state in two ways. Stimulus // outlet-connected callbacks do not fire reliably for elements a morph // replaces, so those children never receive the root reference and refuse @@ -144,6 +256,14 @@ export default class SortableListsController extends Controller imp child.connectRoot(this); child.reregister(); }); + + // Once per morph batch, not per disconnect: a morph can replace a row + // with a fresh element for the same work package, and the disconnect + // alone would drop a member about to come straight back. + // Re-synced on every morph whether or not prune dropped anything: a + // morph can strip or preserve the marker attribute independently of + // the model. + this.selection?.reconcile(); }); }; @@ -157,10 +277,29 @@ export default class SortableListsController extends Controller imp sortableListsItemOutletConnected(item:RootAwareChild):void { item.connectRoot(this); + this.scheduleSelectionReconcile(); } sortableListsItemOutletDisconnected(item:RootAwareChild):void { item.disconnectRoot(); + this.scheduleSelectionReconcile(); + } + + // Rows can be replaced without a morph — a frame navigation such as the + // "show more" expander swaps them wholesale — and the fresh rows carry no + // marker. Outlet callbacks arrive after the whole mutation batch, so one + // microtask later the members that survived are present to be repainted + // and the ones that vanished can be pruned. + private scheduleSelectionReconcile():void { + if (this.reconcileScheduled) { + return; + } + + this.reconcileScheduled = true; + queueMicrotask(() => { + this.reconcileScheduled = false; + this.selection?.reconcile(); + }); } sortableListsScrollableOutletConnected(scrollable:RootAwareChild):void { @@ -175,19 +314,27 @@ export default class SortableListsController extends Controller imp return this.element.hasAttribute(sortableListsBusyAttribute); } - // Availability mirrors executability: a direction is offered exactly when the - // move resolver can produce a target for it. This keeps the menu honest about - // truncated lists, where a one-step move across the hidden block is not - // addressable. Null means the item is not in an owned list (yet). The result - // is a snapshot for menu gating; the click path re-resolves the live DOM. + // A direction is offered exactly when the move resolver can produce a + // target for it, and never for an item moveInDirection would refuse below. + // Null means the item is not in an owned list yet. A snapshot for menu + // gating; the click path re-resolves the live DOM. moveAvailability(itemElement:HTMLElement):MoveAvailability|null { + if (!isOrderableItem(itemElement)) { + return { + top: false, up: false, down: false, bottom: false, + }; + } + const list = this.ownerListOf(itemElement); return list ? resolveMoveAvailability({ itemElement, rowsContainer: list.rowsContainer }) : null; } moveInDirection(itemElement:HTMLElement, direction:MoveDirection):void { - if (this.busy) { + // The menu is rendered server-side from a permission check that does not + // know about per-work-package movability, so a stale or over-permissive + // menu must not execute a move the server will refuse. + if (this.busy || !isOrderableItem(itemElement)) { return; } @@ -212,6 +359,10 @@ export default class SortableListsController extends Controller imp return; } + // Last, after every resolution above: several of them bail, and + // collapsing earlier would destroy the batch for a move that never runs. + this.selection?.collapseForMove(itemElement); + void this.performMove({ sourceRow, rowsContainer: list.rowsContainer, @@ -483,4 +634,5 @@ export default class SortableListsController extends Controller imp void announce(message, { politeness: 'assertive' }); } + } diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/README.md b/frontend/src/stimulus/controllers/dynamic/sortable-lists/README.md new file mode 100644 index 000000000000..713c58b2432f --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/README.md @@ -0,0 +1,73 @@ +# Sortable lists selection + +## Consumer wiring + +Batch selection is opt-in per sortable root through `selectionEnabled`. The root +sets `announcementScope` for its translation vocabulary and +`selectionDescriptionId` for one shared description element. Selected items +reference that description through `aria-describedby` on their focus host. + +Items declare `mobility`: `fixed`, `confined`, or `free`. Mobility gates dragging, +selection eligibility, and positional moves. A missing value defaults to `free`; +an unrecognised value resolves to `fixed`. Structural rows such as “Show more” +are not sortable items and do not participate in selection. + +## Model and ownership + +- [BatchSelection](../../../../common/batch-selection.ts) stores membership, + the anchor, and the baseline preserved while resizing a range. It has no + framework or DOM dependency. +- [selection.ts](selection.ts) resolves candidates, ranges, live document order, + and presentation. +- [selection-orchestrator.ts](selection-orchestrator.ts) interprets gestures + through a narrow host interface and imports no Stimulus. + +Selection identity is `(type, id)`. Each root must render exactly one instance +of each pair, and an item without a type is not a selection candidate. The +orchestrator permits one item type per batch; the model itself does not impose +that compatibility policy. + +An item belongs to its nearest ancestor sortable root. Independently nested +roots are ownership boundaries. Item lookup, focus targets, and range resolution +must respect the owning item and list. + +## Gestures and range sessions + +Ctrl/Cmd-click toggles individual items and can build a sparse batch across +lists. The toggled item becomes the anchor even when deselected. Plain click +replaces the selection with the movable item while allowing its activation. + +Shift-click, Shift+Space, and Shift+Arrow resize a range within the anchor's +list. Independent selections present when the range starts remain selected, +including selections in other lists. Repeated Shift gestures extend, shrink, +or reverse only the range's contribution. Without an anchor, or when Shift +crosses into another list, selection restarts at the acted-on item. Ranges +crossing unloaded or non-movable items are rejected without changing selection. + +Ctrl/Cmd+A replaces the batch with loaded movable items of the focused item's +type in its list. A subsequent Shift gesture can narrow or reverse that +selection. An individual toggle starts a fresh range baseline. + +Space toggles the focused item. Arrows move focus between items, including fixed +items; Home/End target the first/last movable item. Focus movement remains +available during a move, but selection mutations are blocked. Escape clears +selection and the anchor at document level, while respecting fields and overlays +that own the key. + +Reconciliation prunes unavailable items and rebinds the anchor to its live list. +Losing the anchor or moving it to another list ends the range session; unchanged +reconciliation preserves it. + +## Presentation and feedback + +`data-batch-selected` belongs on the sortable item element. In Backlogs this is +the row, while `aria-current` belongs on the card inside it and represents the +work package open in the details pane. Styles must account for those distinct +elements and states. Updating the current work package does not change batch +membership. + +Selection gestures announce membership changes, including changes that retain +the same count. Plain navigation clicks announce only when a wider batch +collapses. Rejected ranges and range restarts have distinct translation keys. +Tests use [keyed translation fixtures](testing/selection-translations.ts) to +assert the selected key and plural form without duplicating production copy. diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts index 2cae90ec252e..812fdb8972e1 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts @@ -101,6 +101,9 @@ export interface SortableListsRoot { // The rows container of the item's innermost owning list, or null when the // item is not (yet) inside a list the root knows about. ownerRowsContainer(itemElement:HTMLElement):HTMLElement|null; + // A drag moves exactly one item until AGILE-278 lands, so it collapses any + // wider batch onto the dragged card. + collapseSelectionForDrag(itemElement:HTMLElement):void; } // Implemented by the list, item and scrollable controllers so the root can diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts index 8bc94713becc..e75a2bf7f9a4 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts @@ -105,6 +105,7 @@ describe('Sortable lists item controller', () => { moveAvailability: vi.fn(() => null), ownerListElementOf: vi.fn(() => ownerListElement), ownerRowsContainer: vi.fn(ownerRowsContainer), + collapseSelectionForDrag: vi.fn(), }; } @@ -113,15 +114,15 @@ describe('Sortable lists item controller', () => { { handle = null, root = fakeRoot(), - confinedValue = false, externalUrl = null, label = null, + mobility = 'free', }:{ handle?:HTMLElement|null; root?:SortableListsRoot|null; - confinedValue?:boolean; externalUrl?:string|null; label?:string|null; + mobility?:string; } = {}, ) { const controller = Object.create(ItemController.prototype) as InstanceType; @@ -131,11 +132,14 @@ describe('Sortable lists item controller', () => { Object.defineProperty(controller, 'hasIdValue', { value: true }); Object.defineProperty(controller, 'typeValue', { value: 'item' }); Object.defineProperty(controller, 'hasTypeValue', { value: true }); - Object.defineProperty(controller, 'confinedValue', { value: confinedValue }); Object.defineProperty(controller, 'externalUrlValue', { value: externalUrl ?? '' }); Object.defineProperty(controller, 'hasExternalUrlValue', { value: externalUrl !== null }); Object.defineProperty(controller, 'labelValue', { value: label ?? '' }); Object.defineProperty(controller, 'hasLabelValue', { value: label !== null }); + // Written to the element, not stubbed as a controller property: the + // controller reads mobility through list-dom's parser, which stubbing + // would bypass. + element.setAttribute('data-sortable-lists--item-mobility-value', mobility); Object.defineProperty(controller, 'hasHandleTarget', { value: handle !== null }); if (handle) { Object.defineProperty(controller, 'handleTarget', { value: handle }); @@ -177,6 +181,15 @@ describe('Sortable lists item controller', () => { element.removeAttribute('data-drop-target-for-element'); }); }); + // Mirrors the real adapter's addAttribute side effect, so movability tests + // can assert on the same `draggable` attribute Pragmatic marks in production. + vi.mocked(draggable).mockImplementation(({ element }) => { + element.setAttribute('draggable', 'true'); + + return vi.fn(() => { + element.removeAttribute('draggable'); + }); + }); }); function connectItem({ id, type }:{ id:string; type:string }) { @@ -188,7 +201,7 @@ describe('Sortable lists item controller', () => { Object.defineProperty(controller, 'hasIdValue', { value: id !== '' }); Object.defineProperty(controller, 'typeValue', { value: type }); Object.defineProperty(controller, 'hasTypeValue', { value: type !== '' }); - Object.defineProperty(controller, 'confinedValue', { value: false }); + Object.defineProperty(controller, 'hasHandleTarget', { value: false }); controller.connect(); @@ -498,7 +511,7 @@ describe('Sortable lists item controller', () => { it('still registers a drag source', () => { const element = document.createElement('li'); - connectedControllerFor(element, { confinedValue: true }); + connectedControllerFor(element, { mobility: 'confined' }); expect(draggable).toHaveBeenCalledWith(expect.objectContaining({ element })); }); @@ -740,7 +753,7 @@ describe('Sortable lists item controller', () => { const element = document.createElement('article'); connectedControllerFor(element, { root: fakeRoot(root, { ownerListElement: sourceList }), - confinedValue: true, + mobility: 'confined', }); expect(vi.mocked(draggable).mock.lastCall?.[0].getInitialData?.(draggableArgs(element))) @@ -1227,4 +1240,112 @@ describe('Sortable lists item controller', () => { expect(moveInDirection).not.toHaveBeenCalled(); }); }); + + describe('movability and focus', () => { + let ctx:StimulusTestContext; + + afterEach(() => { + ctx?.dispose(); + }); + + // Mounts a minimal item row through the real Stimulus lifecycle, so + // mobility and the focus target come from real value/target wiring. + // Omitting `mobility` renders no attribute at all, exercising the free + // default; the root's tabindex stands in for a consumer that makes the + // row itself focusable. + async function renderItem( + { mobility, withFocusTarget = false }:{ mobility?:string; withFocusTarget?:boolean } = {}, + ):Promise { + ctx = await setupStimulusTest({ + controllers: { + 'sortable-lists--item': ItemController, + }, + }); + + const mobilityAttr = mobility === undefined ? '' : ` data-sortable-lists--item-mobility-value="${mobility}"`; + const focusTargetHtml = withFocusTarget + ? '' + : ''; + + await ctx.mount(` +
  • ${focusTargetHtml}
    • + `); + + return ctx.container.querySelector('[data-controller="sortable-lists--item"]')!; + } + + function controllerFor(element:HTMLElement) { + return ctx.getController>('sortable-lists--item', element); + } + + it('registers a draggable when the item takes part in ordering', async () => { + const item = await renderItem({ mobility: 'free' }); + + expect(item.hasAttribute('draggable')).toBe(true); + }); + + it('does not register a draggable when the item is fixed', async () => { + const item = await renderItem({ mobility: 'fixed' }); + + expect(item.hasAttribute('draggable')).toBe(false); + }); + + // A fixed row is still an ordered participant: it anchors drops for its + // orderable neighbours, so its drop target must stay registered. + it('still registers a drop target when the item is fixed', async () => { + const item = await renderItem({ mobility: 'fixed' }); + + expect(item.hasAttribute('data-drop-target-for-element')).toBe(true); + }); + + it('treats a missing mobility attribute as free', async () => { + const item = await renderItem({}); + + expect(item.hasAttribute('draggable')).toBe(true); + }); + + it('focuses its focus target', async () => { + const item = await renderItem({ mobility: 'free', withFocusTarget: true }); + const controller = controllerFor(item); + + controller.focusItem(); + + expect(document.activeElement).toBe(item.querySelector('[data-sortable-lists--item-target~="focus"]')); + }); + + it('focuses itself when it has no focus target', async () => { + const item = await renderItem({ mobility: 'free' }); + const controller = controllerFor(item); + + controller.focusItem(); + + expect(document.activeElement).toBe(item); + }); + + it('collapses the batch onto the dragged item when a drag starts', async () => { + const item = await renderItem({ mobility: 'free' }); + const controller = controllerFor(item); + const collapseSelectionForDrag = vi.fn(); + const root:SortableListsRoot = { + element: item, + busy: false, + moveInDirection: vi.fn(), + moveAvailability: vi.fn(() => null), + ownerListElementOf: vi.fn(() => null), + ownerRowsContainer: vi.fn(() => null), + collapseSelectionForDrag, + }; + + controller.connectRoot(root); + + vi.mocked(draggable).mock.lastCall?.[0].onDragStart?.(dragEventPayload(item)); + + expect(collapseSelectionForDrag).toHaveBeenCalledWith(item); + }); + }); }); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts index 409483e43657..9ee39dd476b4 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts @@ -48,13 +48,13 @@ import { type SortableItemData, type SortableListsRoot, } from './drag-and-drop'; -import { isMoveDirection, sortableItemSelector } from './list-dom'; +import { isConfinedItem, isMoveDirection, isOrderableItem, sortableItemSelector } from './list-dom'; import { renderDragPreview } from './preview'; type CleanupFn = () => void; export default class ItemController extends Controller implements RootAwareChild { - static targets = ['handle', 'preview', 'moveItem', 'moveMenu', 'moveDivider']; + static targets = ['handle', 'preview', 'moveItem', 'moveMenu', 'moveDivider', 'focus']; static elements = { menu: 'action-menu' }; static values = { @@ -62,13 +62,11 @@ export default class ItemController extends Controller implements R type: String, externalUrl: String, hideUnavailable: { type: Boolean, default: true }, - // A confined item is still a full drag source, but only its own list and - // that list's rows accept it as a drop target; foreign containers refuse - // it, so a release there lands nowhere and the item stays put. Consumers - // use this for items the server allows to reorder in place but refuses to - // relocate to another container. - confined: { type: Boolean, default: false }, label: String, + // See ItemMobility in list-dom. A `confined` item is still a full drag + // source; only its own list accepts it as a drop target, so a release + // anywhere else lands nowhere and the item stays put. + mobility: { type: String, default: 'free' }, }; declare readonly idValue:string; @@ -78,7 +76,6 @@ export default class ItemController extends Controller implements R declare readonly externalUrlValue:string; declare readonly hasExternalUrlValue:boolean; declare readonly hideUnavailableValue:boolean; - declare readonly confinedValue:boolean; declare readonly labelValue:string; declare readonly hasLabelValue:boolean; @@ -91,6 +88,8 @@ export default class ItemController extends Controller implements R declare readonly hasMoveMenuTarget:boolean; declare readonly moveDividerTarget:HTMLElement; declare readonly hasMoveDividerTarget:boolean; + declare readonly focusTarget:HTMLElement; + declare readonly hasFocusTarget:boolean; // Provided by the stimulus-elements blessing; absent when the item is not // inside a Primer action-menu (a drag-only consumer), in which case the move @@ -143,7 +142,7 @@ export default class ItemController extends Controller implements R move(event:ActionEvent):void { const item = event.currentTarget; - if (!this.hasMenuElement || !(item instanceof HTMLElement)) { + if (!isOrderableItem(this.element) || !this.hasMenuElement || !(item instanceof HTMLElement)) { return; } @@ -157,6 +156,12 @@ export default class ItemController extends Controller implements R } } + // The focus host is the consumer's business: Backlogs puts the tab stop on + // the card inside the row, another consumer may focus the row itself. + focusItem():void { + (this.hasFocusTarget ? this.focusTarget : this.element).focus(); + } + // Called by the root controller's outlet-connected callback. connectRoot(root:SortableListsRoot):void { this.root = root; @@ -178,14 +183,16 @@ export default class ItemController extends Controller implements R private register():void { this.cleanupFn = combine( - this.registerDraggable(), + // A non-movable item registers no draggable but stays a drop target: + // its movable neighbours still anchor on it. + isOrderableItem(this.element) ? this.registerDraggable() : () => undefined, this.registerDropTarget(), ); } // Both values are required: an item with an empty id can never be persisted, - // and an empty type never matches a list's accepted type, so the item would - // appear draggable yet silently refuse every drop. Surface that wiring mistake. + // and an empty type would never match a list's accepted type, so it could + // neither be dropped nor anchor a drop. Surface that wiring mistake. private warnOnMissingValues():void { if (!this.hasIdValue) { console.warn( @@ -221,6 +228,9 @@ export default class ItemController extends Controller implements R }, getInitialData: () => this.getItemData(), onDragStart: () => { + // One drag moves one item until AGILE-278 lands, so a wider batch + // collapses onto it rather than appearing to come along. + this.root?.collapseSelectionForDrag(this.element); // Cancels drops landing outside registered drop targets. This also // guards the external data channel: a misdropped card carrying // text/uri-list would otherwise navigate the current tab to that URL. @@ -357,7 +367,7 @@ export default class ItemController extends Controller implements R type: this.typeValue, rootElement: this.root?.element ?? null, sourceListElement: this.root?.ownerListElementOf(this.element) ?? null, - confined: this.confinedValue, + confined: isConfinedItem(this.element), }); } diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts index 55677dcbd0aa..da677c3b14f9 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts @@ -28,13 +28,18 @@ import { captureRowPositions, + isConfinedItem, + isOrderableItem, + itemMobility, reorderRows, + sortableItemMobilityAttribute, resolveDirectionalPreviousItemId, resolveMoveAvailability, resolveListAppendPreviousItemId, resolveItemPosition, resolveItemLabel, resolveItemType, + resolveItemElement, restoreRowPositions, rowOf, rowsRemainAt, @@ -112,6 +117,29 @@ describe('sortable lists DOM helpers', () => { }); }); + describe('resolveItemElement', () => { + it('does not descend into a nested list for a wrapper row', () => { + const list = listElement(); + const wrapper = document.createElement('li'); + const nested = listElement(); + nested.append(itemRow('9')); + wrapper.append(nested); + list.append(wrapper); + + expect(resolveItemElement(wrapper, list)).toBeNull(); + }); + + it('resolves the item a row wraps directly', () => { + const list = listElement(); + const wrapper = document.createElement('li'); + const item = itemRow('1'); + wrapper.append(item); + list.append(wrapper); + + expect(resolveItemElement(wrapper, list)).toBe(item); + }); + }); + describe('reorderRows', () => { it('moves a row to sit immediately after the previous item anchor', () => { const list = listElement(); @@ -358,6 +386,47 @@ describe('sortable lists DOM helpers', () => { }); }); +describe('itemMobility', () => { + function itemWith(mobility?:string):HTMLElement { + const item = document.createElement('li'); + item.setAttribute('data-sortable-lists--item-id-value', '1'); + + if (mobility !== undefined) { + item.setAttribute(sortableItemMobilityAttribute, mobility); + } + + return item; + } + + it('defaults a missing attribute to free', () => { + expect(itemMobility(itemWith())).toBe('free'); + }); + + it.each(['fixed', 'confined', 'free'])('reads the recognised value %s', (value) => { + expect(itemMobility(itemWith(value))).toBe(value); + }); + + // A typo must not hand the user a draggable, selectable card that only + // fails once the request comes back. + it('falls back to fixed for an unrecognised value', () => { + expect(itemMobility(itemWith('movable'))).toBe('fixed'); + }); + + it('treats an empty attribute as unrecognised', () => { + expect(itemMobility(itemWith(''))).toBe('fixed'); + }); + + it('derives orderable and confined from the union', () => { + expect(isOrderableItem(itemWith('free'))).toBe(true); + expect(isOrderableItem(itemWith('confined'))).toBe(true); + expect(isOrderableItem(itemWith('fixed'))).toBe(false); + + expect(isConfinedItem(itemWith('confined'))).toBe(true); + expect(isConfinedItem(itemWith('free'))).toBe(false); + expect(isConfinedItem(itemWith('fixed'))).toBe(false); + }); +}); + describe('directional move helpers', () => { function container(ids:string[]):HTMLElement { const ul = document.createElement('ul'); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts index c4ae78c451b4..1ef732f7e504 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts @@ -26,9 +26,13 @@ // See COPYRIGHT and LICENSE files for more details. //++ +import { debugLog } from 'core-app/shared/helpers/debug_output'; + // Sortable lists use a DOM contract shared by the root and item controllers: // the root has data-controller~="sortable-lists"; lists are sortable-lists--list -// controllers wired to the root via outlets; items expose sortable-lists--item values; +// controllers wired to the root via outlets; items expose sortable-lists--item +// values and a mobility, which says what ordering the item takes part in; an +// item that takes none still participates in list order and accepts drops; // sparse non-item rows may expose data-sortable-lists-prev-item-id. // // This module holds the drag-and-drop-agnostic half of that contract: reading @@ -41,6 +45,18 @@ export const sortableItemSelector = '[data-sortable-lists--item-id-value]'; export const sortableListSelector = '[data-controller~="sortable-lists--list"]'; export const sortablePreviousItemIdAttribute = 'data-sortable-lists-prev-item-id'; export const sortableOmittedCountAttribute = 'data-sortable-lists-omitted-count'; +export const sortableItemMobilityAttribute = 'data-sortable-lists--item-mobility-value'; + +/** + * What ordering an item takes part in. + * + * `fixed` takes no part at all: no drag, no positional move, no selection. + * `confined` reorders within its own list but is refused by every other + * container. `free` may move to any list that accepts its type. + */ +export type ItemMobility = 'fixed'|'confined'|'free'; + +const recognisedMobilities = new Set(['fixed', 'confined', 'free']); // Rows are the direct children of the list's resolved rows container. The // rows container itself (a nested
      , the list element, ...) is decided by the list @@ -71,6 +87,37 @@ export function resolveItemId(element:Element):string|null { export const sortableItemTypeAttribute = 'data-sortable-lists--item-type-value'; +/** + * The item's mobility. + * + * An absent attribute means `free`, so a consumer that renders no mobility + * keeps working. A present but unrecognised value fails closed to `fixed`: a + * typo must not hand the user a draggable card, live move actions and a + * selectable row that only fail once the request comes back. + */ +export function itemMobility(itemElement:Element):ItemMobility { + const value = itemElement.getAttribute(sortableItemMobilityAttribute); + + if (value === null) { + return 'free'; + } + + if (!recognisedMobilities.has(value)) { + debugLog(`sortable-lists: unrecognised mobility "${value}", treating the item as fixed`); + return 'fixed'; + } + + return value as ItemMobility; +} + +export function isOrderableItem(itemElement:Element):boolean { + return itemMobility(itemElement) !== 'fixed'; +} + +export function isConfinedItem(itemElement:Element):boolean { + return itemMobility(itemElement) === 'confined'; +} + export function resolveItemType(element:Element):string|null { const type = element.getAttribute(sortableItemTypeAttribute); @@ -85,9 +132,7 @@ export function resolveItemType(element:Element):string|null { // wrong list entirely — which is exactly what a non-item marker row (e.g. an // empty list's placeholder) would otherwise resolve to instead of "no item // here". `boundary.contains(match)` accepts a self-or-ancestor match found -// inside the rows container and rejects one outside it. Only this ancestor -// climb is bounded — resolveItemElement's querySelector fallback below -// descends unbounded and can match an item belonging to a nested inner list. +// inside the rows container and rejects one outside it. export function resolveClosestItemElement(element:Element, boundary:Element):HTMLElement|null { if (!(element instanceof HTMLElement)) { return null; @@ -97,9 +142,14 @@ export function resolveClosestItemElement(element:Element, boundary:Element):HTM return match && boundary.contains(match) ? match : null; } +// The upward climb is bounded by `boundary`; the downward fallback only +// looks at the row's own direct children, so a wrapper row cannot resolve +// to an item of a list nested inside it. export function resolveItemElement(element:Element, boundary:Element):HTMLElement|null { - return resolveClosestItemElement(element, boundary) ?? - element.querySelector(sortableItemSelector); + return resolveClosestItemElement(element, boundary) + ?? Array.from(element.children).find((child):child is HTMLElement => ( + child instanceof HTMLElement && child.matches(sortableItemSelector) + )) ?? null; } export function resolvePreviousItemId(element:Element, boundary:Element):string|null { diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts index 018b9a38d8f4..e1eaa25599f0 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts @@ -78,6 +78,7 @@ describe('Sortable lists list controller', () => { moveAvailability: vi.fn(() => null), ownerListElementOf: vi.fn(() => null), ownerRowsContainer: vi.fn(() => null), + collapseSelectionForDrag: vi.fn(), }; } diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.spec.ts index 69b4697601d7..2595cc54dff1 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.spec.ts @@ -37,7 +37,6 @@ describe('sortable lists drag preview', () => { article.setAttribute('data-dragging', 'source'); article.setAttribute('data-drop-position', 'top'); article.setAttribute('data-drop-position-owner', '123'); - article.setAttribute('data-selected', ''); article.setAttribute('aria-current', 'true'); article.setAttribute('aria-roledescription', 'draggable'); @@ -65,7 +64,6 @@ describe('sortable lists drag preview', () => { expect(element.hasAttribute('data-dragging')).toBe(false); expect(element.hasAttribute('data-drop-position')).toBe(false); expect(element.hasAttribute('data-drop-position-owner')).toBe(false); - expect(element.hasAttribute('data-selected')).toBe(false); expect(element.hasAttribute('aria-current')).toBe(false); expect(element.hasAttribute('aria-roledescription')).toBe(false); }); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.ts index f92bd1a9c41e..df42edadfa61 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.ts @@ -33,13 +33,16 @@ // Attributes stripped from the cloned drag preview so it carries no behaviour or // stale interaction state. The dynamic `data-*--*-target` attributes are removed // separately in sanitizePreview. +// `data-batch-selected` is deliberately absent: it lives on the sortable +// item element (the row, in Backlogs), while the preview target is the card +// — the row's child, never its clone. It can never end up on this clone, so +// stripping it here would be dead code. const PREVIEW_STRIPPED_ATTRIBUTES = [ 'data-controller', 'data-action', 'data-dragging', 'data-drop-position', 'data-drop-position-owner', - 'data-selected', 'aria-current', 'aria-describedby', 'aria-disabled', diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts index 09bc7bcb20d9..8729e01054bc 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts @@ -74,6 +74,7 @@ describe('Sortable lists scrollable controller', () => { moveAvailability: vi.fn(() => null), ownerListElementOf: vi.fn(() => null), ownerRowsContainer: vi.fn(() => null), + collapseSelectionForDrag: vi.fn(), }; } diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.spec.ts new file mode 100644 index 000000000000..446c606916be --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.spec.ts @@ -0,0 +1,732 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { LiveRegionElement } from '@primer/live-region-element'; +import { type MockInstance } from 'vitest'; +import type { SelectionHost } from './selection-orchestrator'; +import { selectionTranslations } from './testing/selection-translations'; + +// No Stimulus application, no outlets, no controller lifecycle: the host +// port lets selection be driven over a plain DOM. +describe('SelectionOrchestrator', () => { + // Imported dynamically, after the mock above registers: spec files share + // one module registry (the runner does not isolate them), so a static + // import here would bind whatever another spec already pulled in — with + // the real platform helper baked in. + let SelectionOrchestrator:typeof import('./selection-orchestrator').SelectionOrchestrator; + let batchSelectedAttribute:string; + + beforeAll(async () => { + ({ SelectionOrchestrator } = await import('./selection-orchestrator')); + ({ batchSelectedAttribute } = await import('./selection')); + }); + + let root:HTMLElement; + let announceSpy:MockInstance; + let busy = false; + let focused:HTMLElement|null = null; + + function hostFor(element:HTMLElement):SelectionHost { + return { + rootElement: element, + get busy() { return busy; }, + announcementScope: 'js.sortable_lists.selection', + descriptionId: 'selection-description', + focusItem: (item) => { focused = item; }, + ownerRowsContainer: (item) => { + const list = item.closest('[data-controller~="sortable-lists--list"]'); + return list ? (list.querySelector(':scope > ul') ?? list) : null; + }, + }; + } + + beforeEach(() => { + busy = false; + focused = null; + root = document.createElement('div'); + root.setAttribute('data-controller', 'sortable-lists'); + root.innerHTML = ` +
      +
        +
      • +
      • +
      • +
      +
      +
      +
        +
      • +
      +
      +
      +
        +
      • +
      • +
      +
      + `; + document.body.append(root, document.createElement('live-region')); + announceSpy = vi.spyOn(LiveRegionElement.prototype, 'announce'); + pretendPlatform('Windows'); + window.I18n.store({ en: { js: { sortable_lists: { selection: selectionTranslations } } } }); + }); + + const userAgentDataDescriptor = Object.getOwnPropertyDescriptor(navigator, 'userAgentData'); + + afterEach(() => { + root.remove(); + document.querySelector('live-region')?.remove(); + announceSpy.mockRestore(); + restorePlatform(); + }); + + function pretendPlatform(platform:string):void { + Object.defineProperty(navigator, 'userAgentData', { + value: { platform }, + configurable: true, + }); + } + + function restorePlatform():void { + if (userAgentDataDescriptor) { + Object.defineProperty(navigator, 'userAgentData', userAgentDataDescriptor); + } else { + delete (navigator as { userAgentData?:unknown }).userAgentData; + } + } + + // Type-qualified, because the fixture deliberately holds a section and a + // work package that share id 1 — the collision a nested topology makes + // routine. + const itemOfType = (type:string, id:string) => root.querySelector( + `[data-sortable-lists--item-type-value="${type}"][data-sortable-lists--item-id-value="${id}"]`, + )!; + const item = (id:string) => itemOfType('work_package', id); + const sectionItem = (id:string) => itemOfType('section', id); + const isSelected = (element:HTMLElement) => element.hasAttribute(batchSelectedAttribute); + const clickOn = (element:HTMLElement, init:MouseEventInit = {}) => { + const event = new MouseEvent('click', { bubbles: true, cancelable: true, ...init }); + Object.defineProperty(event, 'target', { value: element }); + return event; + }; + + const keydownOn = (element:HTMLElement, key:string, init:KeyboardEventInit = {}) => { + const event = new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...init }); + Object.defineProperty(event, 'target', { value: element }); + return event; + }; + + it('selects the clicked card without any Stimulus wiring', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + + orchestrator.handleClick(clickOn(item('1'))); + + expect(isSelected(item('1'))).toBe(true); + expect(orchestrator.selectedIds()).toEqual(['1']); + }); + + it('extends a range from the anchor', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + + orchestrator.handleClick(clickOn(item('1'))); + orchestrator.handleClick(clickOn(item('3'), { shiftKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['1', '2', '3']); + }); + + it.each(['mouse', 'keyboard'] as const)('preserves independent selection when resizing with %s', (input) => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + pretendPlatform('macOS'); + const gesture = (id:string, shiftKey = false) => { + if (input === 'mouse') { + orchestrator.handleClick(clickOn(item(id), { metaKey: !shiftKey, shiftKey })); + } else { + orchestrator.handleKeydown(keydownOn(item(id), ' ', { metaKey: !shiftKey, shiftKey })); + } + }; + + gesture('4'); + gesture('2'); + gesture('3', true); + expect(orchestrator.selectedIds()).toEqual(['2', '3', '4']); + expect(isSelected(item('4'))).toBe(true); + + orchestrator.reconcile(); + gesture('2', true); + expect(orchestrator.selectedIds()).toEqual(['2', '4']); + expect(isSelected(item('3'))).toBe(false); + + gesture('1', true); + expect(orchestrator.selectedIds()).toEqual(['1', '2', '4']); + }); + + it.each(['mouse', 'keyboard'] as const)('narrows select-all with %s while replacing picks outside the list', (input) => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('4'), { ctrlKey: true })); + orchestrator.handleKeydown(keydownOn(item('2'), 'a', { ctrlKey: true })); + expect(orchestrator.selectedIds()).toEqual(['1', '2', '3']); + + if (input === 'mouse') { + orchestrator.handleClick(clickOn(item('3'), { shiftKey: true })); + } else { + orchestrator.handleKeydown(keydownOn(item('3'), ' ', { shiftKey: true })); + } + expect(orchestrator.selectedIds()).toEqual(['2', '3']); + expect(isSelected(item('1'))).toBe(false); + expect(isSelected(item('4'))).toBe(false); + }); + + it.each(['mouse', 'keyboard'] as const)('preserves same-list picks and prunes a removed baseline card with %s', (input) => { + item('3').after(item('4'), item('5')); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const gesture = (id:string, shiftKey = false) => { + if (input === 'mouse') { + orchestrator.handleClick(clickOn(item(id), { ctrlKey: !shiftKey, shiftKey })); + } else { + orchestrator.handleKeydown(keydownOn(item(id), ' ', { ctrlKey: !shiftKey, shiftKey })); + } + }; + + gesture('1'); + gesture('3'); + gesture('5', true); + expect(orchestrator.selectedIds()).toEqual(['1', '3', '4', '5']); + gesture('4', true); + expect(orchestrator.selectedIds()).toEqual(['1', '3', '4']); + gesture('2', true); + expect(orchestrator.selectedIds()).toEqual(['1', '2', '3']); + + const removed = item('1'); + removed.remove(); + orchestrator.reconcile(); + gesture('4', true); + expect(orchestrator.selectedIds()).toEqual(['3', '4']); + item('2').before(removed); + orchestrator.reconcile(); + gesture('5', true); + expect(orchestrator.selectedIds()).toEqual(['3', '4', '5']); + expect(isSelected(removed)).toBe(false); + }); + + it('routes focus through the host rather than touching the element', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true, cancelable: true }); + Object.defineProperty(event, 'target', { value: item('1') }); + + orchestrator.handleKeydown(event); + + expect(focused).toBe(item('2')); + }); + + it('reads busy from the host and refuses to mutate while a move is in flight', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + busy = true; + + orchestrator.handleClick(clickOn(item('1'))); + + expect(orchestrator.selectedIds()).toEqual([]); + }); + + it('clears presentation without disturbing the model', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + + orchestrator.clearPresentation(); + + expect(isSelected(item('1'))).toBe(false); + expect(orchestrator.selectedIds()).toEqual(['1']); + }); + + // Falling through mid-move would open the details pane on a card the batch + // was not allowed to follow. + it('consumes a plain click while a move is in flight', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + busy = true; + + const event = clickOn(item('2')); + orchestrator.handleClick(event); + + expect(event.defaultPrevented).toBe(true); + expect(orchestrator.selectedIds()).toEqual(['1']); + }); + + // Both roots listen at the document; the first to clear consumes the key, + // which must not read as an overlay's Escape to the second. + it('clears every root\'s batch on one Escape, not only the first listener\'s', () => { + const secondRoot = root.cloneNode(true) as HTMLElement; + document.body.appendChild(secondRoot); + const first = new SelectionOrchestrator(hostFor(root)); + const second = new SelectionOrchestrator(hostFor(secondRoot)); + first.handleClick(clickOn(item('1'))); + second.handleClick(clickOn(secondRoot.querySelector('[data-sortable-lists--item-id-value="2"]')!)); + const event = new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }); + Object.defineProperty(event, 'target', { value: document.body }); + + try { + first.handleEscape(event); + second.handleEscape(event); + } finally { + secondRoot.remove(); + } + + expect(first.selectedIds()).toEqual([]); + expect(second.selectedIds()).toEqual([]); + }); + + it('still leaves an Escape an overlay consumed alone', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + const event = new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }); + Object.defineProperty(event, 'target', { value: document.body }); + event.preventDefault(); + + orchestrator.handleEscape(event); + + expect(orchestrator.selectedIds()).toEqual(['1']); + }); + + describe('announcements', () => { + const spoken = () => announceSpy.mock.calls.map((call) => call[0]); + + // A resized range of the same size still changed, and has no details + // pane to serve as its own feedback. + it('announces a range that swaps membership at the same size', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('2'))); + orchestrator.handleClick(clickOn(item('3'), { shiftKey: true })); + announceSpy.mockClear(); + + orchestrator.handleClick(clickOn(item('1'), { shiftKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['1', '2']); + expect(spoken()).toEqual(['[selected:2]']); + }); + + // The baseline is what the previous render painted: tracking the last + // announcement would leave a stale one behind a silent render. + it('stays silent when a selection gesture changes nothing after a silent click', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + orchestrator.handleClick(clickOn(item('2'))); + announceSpy.mockClear(); + + orchestrator.handleClick(clickOn(item('2'), { shiftKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['2']); + expect(spoken()).toEqual([]); + }); + + // The details pane opening is the feedback for that card; the count only + // matters once a wider batch is lost. + it('stays silent on the first plain click', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + + orchestrator.handleClick(clickOn(item('1'))); + + expect(orchestrator.selectedIds()).toEqual(['1']); + expect(spoken()).toEqual([]); + }); + + it('stays silent when a plain click on a fixed card drops a single selected card', () => { + item('2').setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + announceSpy.mockClear(); + + orchestrator.handleClick(clickOn(item('2'))); + + expect(orchestrator.selectedIds()).toEqual([]); + expect(spoken()).toEqual([]); + }); + + it('stays silent on a plain click that swaps a one-card selection', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + announceSpy.mockClear(); + + orchestrator.handleClick(clickOn(item('2'))); + + expect(orchestrator.selectedIds()).toEqual(['2']); + expect(spoken()).toEqual([]); + }); + + it('still announces a plain click that changes the count', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + orchestrator.handleClick(clickOn(item('3'), { shiftKey: true })); + announceSpy.mockClear(); + + orchestrator.handleClick(clickOn(item('2'))); + + expect(spoken()).toEqual(['[selected:1]']); + }); + }); + + describe('keyboard and pointer edge cases', () => { + it('still moves focus with the arrows while a move is in flight', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + busy = true; + + orchestrator.handleKeydown(keydownOn(item('1'), 'ArrowDown')); + + expect(focused).toBe(item('2')); + }); + + it('moves focus but does not extend the range with Shift+Arrow while busy', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + busy = true; + + orchestrator.handleKeydown(keydownOn(item('1'), 'ArrowDown', { shiftKey: true })); + + expect(focused).toBe(item('2')); + expect(orchestrator.selectedIds()).toEqual(['1']); + }); + + // Consuming the key with nothing to select would block the browser's own + // select-all and announce nothing in its place. + it('leaves Ctrl/Cmd+A alone when nothing is selectable', () => { + root.querySelectorAll('[data-sortable-lists--item-id-value]') + .forEach((el) => el.setAttribute('data-sortable-lists--item-mobility-value', 'fixed')); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'a', { ctrlKey: true }); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(false); + expect(orchestrator.selectedIds()).toEqual([]); + }); + + it('still consumes Ctrl/Cmd+A when there is something to select', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'a', { ctrlKey: true }); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(true); + expect(orchestrator.selectedIds()).toEqual(['1', '2', '3']); + }); + + it('anchors select-all inside the list when the focused card is fixed', () => { + item('1').setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + + orchestrator.handleKeydown(keydownOn(item('1'), 'a', { ctrlKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['2', '3']); + // The fallback anchor is the first orderable card of the same list, so + // a follow-up Shift ranges within it rather than from another list. + orchestrator.handleKeydown(keydownOn(item('3'), ' ', { shiftKey: true })); + expect(orchestrator.selectedIds()).toEqual(['2', '3']); + }); + + // Select all binds to the platform's one multi-select modifier: ⌘ on + // Apple platforms, Ctrl elsewhere — never the other way around. + it('consumes Cmd+A on Apple platforms', () => { + pretendPlatform('macOS'); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'a', { metaKey: true }); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(true); + expect(orchestrator.selectedIds()).toEqual(['1', '2', '3']); + }); + + it('leaves Ctrl+A alone on Apple platforms', () => { + pretendPlatform('macOS'); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'a', { ctrlKey: true }); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(false); + expect(orchestrator.selectedIds()).toEqual([]); + }); + + it('leaves Meta+A alone off Apple platforms', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'a', { metaKey: true }); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(false); + expect(orchestrator.selectedIds()).toEqual([]); + }); + + // Non-Latin layouts print another letter on the A key; AZERTY prints A + // on KeyQ. Both users expect Ctrl+A where their keyboard says A. + it('selects all from the physical A key on a non-Latin layout', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'ф', { ctrlKey: true, code: 'KeyA' }); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(true); + expect(orchestrator.selectedIds()).toEqual(['1', '2', '3']); + }); + + it('selects all from the A key of an AZERTY layout', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'a', { ctrlKey: true, code: 'KeyQ' }); + + orchestrator.handleKeydown(event); + + expect(orchestrator.selectedIds()).toEqual(['1', '2', '3']); + }); + + it('leaves Ctrl+Q alone on an AZERTY layout although it sits on KeyA', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'q', { ctrlKey: true, code: 'KeyA' }); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(false); + expect(orchestrator.selectedIds()).toEqual([]); + }); + + // Non-Latin keys throughout, so that only the guard under test, never + // the Latin exclusion, can be what leaves the key alone. + it.each([ + ['a dead key', { key: 'Dead', code: 'KeyA', ctrlKey: true }], + ['IME composition', { key: 'Process', code: 'KeyA', ctrlKey: true }], + ['a composing letter', { key: 'ф', code: 'KeyA', ctrlKey: true, isComposing: true }], + ['punctuation on the A key', { key: ';', code: 'KeyA', ctrlKey: true }], + ['an AltGr chord', { key: 'ф', code: 'KeyA', ctrlKey: true, altKey: true }], + ])('leaves %s alone although it sits on KeyA', (_label, init) => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const { key, ...rest } = init as KeyboardEventInit & { key:string }; + const event = keydownOn(item('1'), key, rest); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(false); + expect(orchestrator.selectedIds()).toEqual([]); + }); + + // Stubbed rather than passed as `modifierAltGraph`: not every engine maps + // the init member onto getModifierState. + it('leaves an AltGraph chord alone although it sits on KeyA', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'ф', { code: 'KeyA', ctrlKey: true }); + vi.spyOn(event, 'getModifierState').mockImplementation((key) => key === 'AltGraph'); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(false); + expect(orchestrator.selectedIds()).toEqual([]); + }); + + // With nothing selectable in this list, the browser's own select-all + // keeps the gesture even though another list has cards. + it('leaves Ctrl/Cmd+A alone when only other lists are selectable', () => { + root.querySelectorAll('[data-sortable-lists--list-id-value="7"] [data-sortable-lists--item-id-value]') + .forEach((el) => el.setAttribute('data-sortable-lists--item-mobility-value', 'fixed')); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = keydownOn(item('1'), 'a', { ctrlKey: true }); + + orchestrator.handleKeydown(event); + + expect(event.defaultPrevented).toBe(false); + expect(orchestrator.selectedIds()).toEqual([]); + }); + + // Holding Space would otherwise toggle the card over and over. + it('ignores a repeated Space keydown', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleKeydown(keydownOn(item('1'), ' ')); + + orchestrator.handleKeydown(keydownOn(item('1'), ' ', { repeat: true })); + + expect(orchestrator.selectedIds()).toEqual(['1']); + }); + + it('refuses to extend a range onto a fixed card', () => { + item('2').setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + + orchestrator.handleKeydown(keydownOn(item('1'), 'ArrowDown', { shiftKey: true })); + + expect(isSelected(item('2'))).toBe(false); + expect(orchestrator.selectedIds()).toEqual([]); + }); + + it('refuses to extend a range over a fixed card', () => { + item('2').setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + announceSpy.mockClear(); + + orchestrator.handleClick(clickOn(item('3'), { shiftKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['1']); + expect(isSelected(item('3'))).toBe(false); + expect(announceSpy.mock.calls.map((call) => call[0])).toEqual(['[range_blocked]']); + }); + + // Ctrl-click opens the contextual menu on Apple platforms whatever else + // is held: it must neither toggle, nor range, nor fall through to the + // ordinary-click path. + it.each([ + ['on its own', {}], + ['with Shift', { shiftKey: true }], + ['with Cmd', { metaKey: true }], + ])('ignores Ctrl-click %s on Apple platforms', (_label, extra) => { + pretendPlatform('macOS'); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + + const event = clickOn(item('2'), { ctrlKey: true, ...extra }); + orchestrator.handleClick(event); + + expect(orchestrator.selectedIds()).toEqual(['1']); + expect(isSelected(item('2'))).toBe(false); + expect(event.defaultPrevented).toBe(false); + }); + + it('still treats Ctrl-click as multi-select elsewhere', () => { + pretendPlatform('Windows'); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + + orchestrator.handleClick(clickOn(item('2'), { ctrlKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['1', '2']); + }); + + // Meta is not an alternate multi-select modifier off Apple platforms: a + // Meta-click classifies as an ordinary click and replaces the batch. + it('does not treat Meta-click as multi-select elsewhere', () => { + pretendPlatform('Windows'); + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + + orchestrator.handleClick(clickOn(item('2'), { metaKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['2']); + }); + }); + + // collapseForDrag stamps the anchor at drag start, so a cross-list drop + // leaves it naming the source list. Without a rebind the next Shift in the + // destination reads as cross-list and restarts instead of extending. + it('extends a range after the anchored card moved to another list', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const moved = item('1'); + orchestrator.handleClick(clickOn(moved)); + + root.querySelector('[data-sortable-lists--list-id-value="8"] ul')!.prepend(moved); + orchestrator.reconcile(); + orchestrator.handleClick(clickOn(item('4'), { shiftKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['1', '4']); + }); + + describe('one batch, one item type', () => { + // Section 1 and work package 1 are different items that happen to share + // an id, which is what a nested topology makes routine. + it('does not paint an item of another type that shares an id', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + + orchestrator.handleClick(clickOn(item('1'))); + + expect(isSelected(item('1'))).toBe(true); + expect(isSelected(sectionItem('1'))).toBe(false); + }); + + it('restarts the batch when Ctrl/Cmd-click lands on another type', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + orchestrator.handleClick(clickOn(item('2'), { ctrlKey: true })); + + orchestrator.handleClick(clickOn(sectionItem('1'), { ctrlKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['1']); + expect(isSelected(sectionItem('1'))).toBe(true); + expect(isSelected(item('1'))).toBe(false); + }); + + it('restarts the batch when Space toggles another type', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + + const event = new KeyboardEvent('keydown', { key: ' ', bubbles: true, cancelable: true }); + Object.defineProperty(event, 'target', { value: sectionItem('1') }); + orchestrator.handleKeydown(event); + + expect(isSelected(sectionItem('1'))).toBe(true); + expect(isSelected(item('1'))).toBe(false); + }); + + it('scopes select-all to the anchoring candidate type', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const event = new KeyboardEvent('keydown', { key: 'a', ctrlKey: true, bubbles: true, cancelable: true }); + Object.defineProperty(event, 'target', { value: sectionItem('1') }); + + orchestrator.handleKeydown(event); + + expect(orchestrator.selectedIds()).toEqual(['1']); + expect(isSelected(sectionItem('1'))).toBe(true); + expect(isSelected(item('2'))).toBe(false); + }); + + // A bare-id lookup would find the section first, since it appears + // earlier in document order, and rebind the anchor to the wrong list. + it('rebinds the anchor to its own type when an earlier item shares its id', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + const moved = item('1'); + orchestrator.handleClick(clickOn(moved)); + + root.querySelector('[data-sortable-lists--list-id-value="8"] ul')!.prepend(moved); + orchestrator.reconcile(); + orchestrator.handleClick(clickOn(item('4'), { shiftKey: true })); + + expect(orchestrator.selectedIds()).toEqual(['1', '4']); + }); + }); + + it('drops members that a morph removed from the document', () => { + const orchestrator = new SelectionOrchestrator(hostFor(root)); + orchestrator.handleClick(clickOn(item('1'))); + orchestrator.handleClick(clickOn(item('2'), { ctrlKey: true })); + item('1').remove(); + + orchestrator.reconcile(); + + expect(orchestrator.selectedIds()).toEqual(['2']); + expect(announceSpy).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts new file mode 100644 index 000000000000..bdf75f529dfe --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts @@ -0,0 +1,521 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { BatchSelection, type SelectionAnchor, type SelectionKey } from 'core-common/batch-selection'; +import { announce } from '@primer/live-region-element'; +import { resolveItemId, resolveItemType } from './list-dom'; +import { + applySelectionPresentation, + listBoundaryItem, + liveOrderableKeys, + liveOrderableListItems, + neighbourItem, + orderedItemElements, + orderedSelectedItems, + resolveCandidate, + resolveRangeItems, + type SelectionCandidate, +} from './selection'; +import { closestInteractiveElement } from 'core-common/interactive-element-helper'; +import { isApplePlatform } from 'core-stimulus/helpers/platform'; + +/** + * What the orchestrator needs from whatever hosts it. + */ +export interface SelectionHost { + readonly rootElement:HTMLElement; + readonly busy:boolean; + readonly announcementScope:string; + readonly descriptionId:string; + // The consumer decides which element inside a row holds the tab stop. + focusItem(itemElement:HTMLElement):void; + // Must be the container moves use, so ranges and moves agree on what a + // list's rows are. + ownerRowsContainer(itemElement:HTMLElement):HTMLElement|null; +} + +/** + * Batch selection: gestures in, model and presentation out. + */ +// Read the way browsers bind their own select-all: by the key's meaning on +// Latin layouts (AZERTY's Ctrl+A sits on physical KeyQ), by the physical key +// where the layout prints another letter (Cyrillic ф, Greek α on KeyA). A +// dead key, composition or an AltGr chord is input, not a shortcut. +function isSelectAllKey(event:KeyboardEvent):boolean { + if (event.isComposing || event.altKey || event.getModifierState('AltGraph')) { + return false; + } + + return event.key === 'a' || event.key === 'A' + || (event.code === 'KeyA' && /^\p{L}$/u.test(event.key) && !/^\p{Script=Latin}$/u.test(event.key)); +} + +// Every root listens for Escape at the document, so the first to clear +// would otherwise look to the next like an overlay that consumed the key. +const escapesClearedBySelection = new WeakSet(); + +export class SelectionOrchestrator { + private readonly selection = new BatchSelection(); + + // Last painted membership, not last announced: a silent navigation render + // in between would leave a stale baseline and make the next no-op speak. + private lastRenderedKeys:ReadonlySet = new Set(); + + constructor(private readonly host:SelectionHost) {} + + // Live ordered membership, for AGILE-278's batch move. + selectedIds():string[] { + return orderedSelectedItems(this.host.rootElement, this.selection.keys).map((item) => item.id); + } + + // A menu move relocates exactly one card, so it collapses like a drag. + collapseForMove(itemElement:HTMLElement):void { + this.collapseForDrag(itemElement); + } + + collapseForDrag(itemElement:HTMLElement):void { + // Nothing selected is nothing to collapse: a drag must not manufacture + // a one-card batch. + if (this.selection.size === 0) { + return; + } + + const candidate = resolveCandidate(this.host.rootElement, itemElement); + if (!candidate?.orderable) { + return; + } + + this.selection.replace({ type: candidate.type, id: candidate.id }, candidate.listKey); + this.renderSelection('selection'); + } + + // The platform's one multi-select modifier: ⌘ on Apple platforms, Ctrl + // elsewhere. Meta is not an alternate modifier on Windows or Linux, and + // Ctrl on Apple is the secondary click, never multi-select. + private multiSelectModifier(event:MouseEvent|KeyboardEvent):boolean { + return isApplePlatform() ? event.metaKey : event.ctrlKey; + } + + readonly handleClick = (event:MouseEvent):void => { + // Ctrl-click is the secondary click on Apple platforms whatever else is + // held, so it opens the contextual menu; Cmd is the multi-select key. + if (event.ctrlKey && isApplePlatform()) { + return; + } + + const multiSelect = this.multiSelectModifier(event); + const modified = event.shiftKey || multiSelect; + const candidate = this.candidateForGesture(event.target); + if (!candidate) { + return; + } + + if (!modified) { + // Consumed before the busy check: falling through mid-move would open + // the details pane on a card the batch was not allowed to follow. + if (this.host.busy) { + event.preventDefault(); + event.stopPropagation(); + return; + } + + // A fixed card cannot join the batch, but must still clear the one + // behind it. + if (candidate.orderable) { + this.selection.replace({ type: candidate.type, id: candidate.id }, candidate.listKey); + this.renderSelection('navigation'); + } else { + this.selection.clear(); + this.renderSelection('navigation'); + } + return; + } + + // Consumed regardless of `busy`: a modified gesture reaching the card's + // own click handler would open the details pane on a selection toggle. + event.preventDefault(); + event.stopPropagation(); + + if (this.host.busy) { + return; + } + + if (!candidate.orderable) { + this.announceSelection('not_selectable'); + return; + } + + if (event.shiftKey) { + this.extendSelectionTo(candidate); + } else { + this.toggleWithinCohort(candidate); + this.renderSelection('selection'); + } + }; + + // A link inside a card is a link first. The walk stops at the focus host + // when the gesture landed inside it — the host may be focusable itself, + // Backlogs cards carry tabindex — and at the item otherwise, which keeps + // it bounded to the row for a host nested deeper than the item. + private candidateForGesture(target:EventTarget|null):SelectionCandidate|null { + const candidate = resolveCandidate(this.host.rootElement, target); + if (!candidate) { + return null; + } + + if (!(target instanceof Element)) { + return candidate; + } + + const boundary = candidate.focusHost.contains(target) ? candidate.focusHost : candidate.itemElement; + const interactive = closestInteractiveElement(target, boundary); + + return interactive ? null : candidate; + } + + readonly handleKeydown = (event:KeyboardEvent):void => { + const candidate = this.candidateForGesture(event.target); + if (!candidate) { + return; + } + + switch (event.key) { + case ' ': + this.handleSpace(event, candidate); + break; + case 'ArrowDown': + case 'ArrowUp': + this.handleArrow(event, candidate, event.key === 'ArrowDown' ? 1 : -1); + break; + case 'Home': + case 'End': + this.handleBoundary(event, candidate, event.key === 'Home' ? 'first' : 'last'); + break; + default: + // Enter belongs to the card's own activation handler; Escape is + // handled at the document. + if (isSelectAllKey(event)) { + this.handleSelectAll(event, candidate); + } + break; + } + }; + + private handleSpace(event:KeyboardEvent, candidate:SelectionCandidate):void { + event.preventDefault(); + + // A held Space would otherwise toggle the card over and over. + if (event.repeat) { + return; + } + + if (this.host.busy) { + return; + } + + if (!candidate.orderable) { + this.announceSelection('not_selectable'); + return; + } + + if (event.shiftKey) { + this.extendSelectionTo(candidate); + } else { + this.toggleWithinCohort(candidate); + this.renderSelection('selection'); + } + } + + // Consumed even at a list boundary: an unconsumed arrow scrolls the page + // while focus stays put. + private handleArrow(event:KeyboardEvent, candidate:SelectionCandidate, offset:1|-1):void { + event.preventDefault(); + + const next = neighbourItem(this.host.rootElement, candidate.itemElement, offset); + if (!next) { + return; + } + + this.focusAndMaybeExtend(event, next); + } + + // Consumed like an arrow, including in both no-op cases below. + private handleBoundary(event:KeyboardEvent, candidate:SelectionCandidate, edge:'first'|'last'):void { + event.preventDefault(); + + const target = listBoundaryItem(this.host.rootElement, candidate.itemElement, edge); + if (!target) { + return; + } + + // With Shift held the range still resizes out to the boundary. + if (target === candidate.itemElement && !event.shiftKey) { + return; + } + + this.focusAndMaybeExtend(event, target); + } + + // Focus moves even mid-move: only the range mutation waits for the host. + private focusAndMaybeExtend(event:KeyboardEvent, target:HTMLElement):void { + this.focusItemElement(target); + + if (!event.shiftKey || this.host.busy) { + return; + } + + const candidate = resolveCandidate(this.host.rootElement, target); + if (!candidate) { + return; + } + + if (candidate.orderable) { + this.extendSelectionTo(candidate); + } else { + this.announceSelection('not_selectable'); + } + } + + private focusItemElement(target:HTMLElement):void { + this.host.focusItem(target); + } + + // Confined to the focused card's list, like a range. + private handleSelectAll(event:KeyboardEvent, candidate:SelectionCandidate):void { + if (!this.multiSelectModifier(event)) { + return; + } + + const items = liveOrderableListItems(this.host.rootElement, candidate.itemElement) + .filter((item) => item.type === candidate.type); + // Only consumed once there is something to select: otherwise the + // browser's own select-all still has to work. + if (items.length === 0) { + return; + } + + event.preventDefault(); + + if (this.host.busy) { + return; + } + + // A fixed focused card cannot anchor the batch, so the first orderable + // card of the same list stands in. + const anchor:SelectionAnchor = candidate.orderable + ? { type: candidate.type, id: candidate.id, listKey: candidate.listKey } + : { ...items[0], listKey: candidate.listKey }; + + this.selection.selectAll(items, anchor); + this.renderSelection('selection'); + } + + // At the document, in the bubble phase: focus routinely sits off the root + // after a mouse selection, and an overlay's own Escape must run first. + readonly handleEscape = (event:KeyboardEvent):void => { + // A consumed Escape already answered the keystroke — unless it was + // another root's selection that consumed it, in which case this root's + // batch still has to go. + if (event.key !== 'Escape' || (event.defaultPrevented && !escapesClearedBySelection.has(event))) { + return; + } + + if (!this.escapeConcernsSelection(event.target)) { + return; + } + + // BatchSelection#toggle re-bases the anchor even on a deselect, so an + // emptied selection can still leave one behind for Escape to drop. + const hadSelection = this.selection.size > 0; + if (!hadSelection && this.selection.anchor === null) { + return; + } + + event.preventDefault(); + escapesClearedBySelection.add(event); + this.selection.clear(); + this.renderSelection('selection'); + }; + + // Escape drops the selection from wherever focus sits, except where the + // key already means "dismiss this thing": an open overlay anywhere — focus + // can still sit on its invoker for a frame after it opens — or a field + // whose widget owns it (a picker, an inline editor, an autocompleter). + private escapeConcernsSelection(target:EventTarget|null):boolean { + if (document.querySelector('dialog[open], :popover-open')) { + return false; + } + + if (!(target instanceof Element)) { + return true; + } + + return target.closest( + '[role="dialog"], [role="menu"], [role="listbox"], input, textarea, select, [contenteditable]', + ) === null; + } + + // A batch holds one item type: "all of these together" has no meaning + // across two kinds of thing, and a collection move sends one list of ids + // to one endpoint. + private cohortMatches(candidate:SelectionCandidate):boolean { + const { anchor } = this.selection; + + return anchor === null || anchor.type === candidate.type; + } + + // Every adding gesture routes through here, which is what keeps a mixed + // batch unreachable. + private toggleWithinCohort(candidate:SelectionCandidate):void { + if (!this.cohortMatches(candidate)) { + this.renderRangeRestart(candidate); + return; + } + + this.selection.toggle({ type: candidate.type, id: candidate.id }, candidate.listKey); + this.renderSelection('selection'); + } + + private extendSelectionTo(candidate:SelectionCandidate):void { + const { anchor } = this.selection; + + if (anchor && !this.cohortMatches(candidate)) { + this.renderRangeRestart(candidate); + return; + } + + if (!anchor) { + this.renderRangeRestart(candidate); + return; + } + + const range = resolveRangeItems( + this.host.rootElement, + anchor, + candidate, + this.host.ownerRowsContainer(candidate.itemElement), + ); + + if (range.ok) { + this.selection.range(range.items); + this.renderSelection('selection'); + return; + } + + if (range.reason === 'crossList') { + this.renderRangeRestart(candidate); + return; + } + + // Expanding the list can surface a truncated block, but never makes a + // locked card orderable, so the two speak different messages. + this.announceSelection(range.reason === 'locked' ? 'range_blocked' : 'range_unavailable'); + } + + // Always speaks, unlike renderSelection's count rule: a Shift gesture that + // failed to form a range has no other feedback, even when the resulting + // count is unchanged. + private renderRangeRestart(candidate:SelectionCandidate):void { + this.selection.replace({ type: candidate.type, id: candidate.id }, candidate.listKey); + this.syncSelectionPresentation(); + this.lastRenderedKeys = this.selection.keys; + this.announceSelection('range_restarted'); + } + + // Announcement follows the gesture class, not the count: `navigation` (a + // plain click) speaks only when it collapsed a batch, since the details + // pane it opens is its own feedback for the card itself; `selection` + // speaks on any membership change. + private renderSelection(kind:'navigation'|'selection'):void { + const previous = this.lastRenderedKeys; + this.syncSelectionPresentation(); + + const current = this.selection.keys; + this.lastRenderedKeys = current; + + const changed = kind === 'navigation' + ? previous.size > 1 && current.size !== previous.size + : current.size !== previous.size || [...current].some((id) => !previous.has(id)); + + if (changed) { + this.announceSelection(current.size === 0 ? 'cleared' : 'selected'); + } + } + + private syncSelectionPresentation():void { + applySelectionPresentation(this.host.rootElement, this.selection.keys, this.host.descriptionId); + } + + private announceSelection(key:'selected'|'cleared'|'not_selectable'|'range_unavailable'|'range_blocked'|'range_restarted'):void { + void announce(this.selectionMessage(key), { politeness: 'polite' }); + } + + // The consumer's vocabulary: Backlogs says "work package", not "item". + private selectionMessage(key:string):string { + return I18n.t(`${this.host.announcementScope}.${key}`, { count: this.selection.size }); + } + + // Repaints whether or not prune dropped anything: a morph can strip or + // preserve the marker attribute independently of the model. + reconcile():void { + this.selection.prune(liveOrderableKeys(this.host.rootElement)); + this.rebindAnchorList(); + this.renderSelection('selection'); + } + + // The anchor's list key is stamped when the anchor is set — drag *start* + // for a drag — so a card dropped elsewhere still names its old list. Runs + // after prune, so it only ever sees an anchor that still exists. + private rebindAnchorList():void { + const { anchor } = this.selection; + if (!anchor) { + return; + } + + // Matched on type as well as id: ids collide across source tables. + const element = orderedItemElements(this.host.rootElement) + .find((item) => resolveItemId(item) === anchor.id && resolveItemType(item) === anchor.type); + const candidate = element ? resolveCandidate(this.host.rootElement, element) : null; + + if (candidate) { + this.selection.rebindAnchor(candidate.listKey); + } + } + + // Presentation only: a restored page brings its markup back under a fresh + // orchestrator, whose model and render baseline are already empty. + clearPresentation():void { + applySelectionPresentation(this.host.rootElement, new Set(), this.host.descriptionId); + } + + teardown():void { + this.selection.clear(); + this.lastRenderedKeys = new Set(); + this.clearPresentation(); + } +} diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection.spec.ts new file mode 100644 index 000000000000..bf9de05b3679 --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection.spec.ts @@ -0,0 +1,425 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { selectionKey } from 'core-common/batch-selection'; +import { + applySelectionPresentation, + batchSelectedAttribute, + listBoundaryItem, + liveOrderableItems, + liveOrderableListItems, + neighbourItem, + orderedItemElements, + orderedSelectedItems, + resolveCandidate, + resolveRangeItems, +} from './selection'; + +describe('sortable-lists selection adapter', () => { + let root:HTMLElement; + + // Two lists. Sprint 7 holds movable 1 and 2, a truncation marker, and + // movable 3. Sprint 8 holds movable 4 and non-movable 5. + beforeEach(() => { + root = document.createElement('div'); + root.setAttribute('data-controller', 'sortable-lists'); + root.innerHTML = ` +
      +
        +
      • one
        • +
      • +
      • +
      • +
      +
      +
      +
        +
      • +
      • +
      +
      + `; + document.body.appendChild(root); + }); + + afterEach(() => { + root.remove(); + }); + + // The rows container the host resolves in production; supplied directly + // here because this spec drives the adapter without one. + const rowsContainerFor = (item:HTMLElement) => { + const list = item.closest('[data-controller~="sortable-lists--list"]'); + return list ? (list.querySelector(':scope > ul') ?? list) : null; + }; + + const itemFor = (id:string) => root.querySelector(`[data-sortable-lists--item-id-value="${id}"]`)!; + const candidateFor = (id:string) => resolveCandidate(root, itemFor(id))!; + + // A trailing item in sprint 7 that hosts a list of its own, the nested + // topology a section-and-fields consumer renders. + const appendNestedList = () => { + const section = document.createElement('li'); + section.setAttribute('data-controller', 'sortable-lists--item'); + section.setAttribute('data-sortable-lists--item-id-value', '6'); + section.setAttribute('data-sortable-lists--item-type-value', 'section'); + section.innerHTML = ` +
      +
        +
      • +
      +
      + `; + root.querySelector('ul')!.appendChild(section); + }; + + it('resolves a candidate from a descendant of the item', () => { + const candidate = resolveCandidate(root, itemFor('1').querySelector('span')); + + expect(candidate).toEqual({ + type: 'work_package', + itemElement: itemFor('1'), + focusHost: itemFor('1'), + id: '1', + listKey: 'sprint:7', + orderable: true, + }); + }); + + it('keys the list by its type and id, not its DOM id', () => { + itemFor('1').closest('[data-controller~="sortable-lists--list"]')!.id = 'inbox_project_4'; + + expect(candidateFor('1').listKey).toBe('sprint:7'); + }); + + // The focus host is also the boundary the interactive-descendant check + // stops at, so a focusable host does not disqualify its own card. + it('reports the focus target as the focus host when there is one', () => { + const card = document.createElement('div'); + card.setAttribute('data-sortable-lists--item-target', 'focus'); + card.tabIndex = 0; + itemFor('2').appendChild(card); + + expect(resolveCandidate(root, card)!.focusHost).toBe(card); + }); + + it('does not take a nested item\'s focus target as the focus host', () => { + appendNestedList(); + itemFor('60').setAttribute('data-sortable-lists--item-target', 'focus'); + + expect(candidateFor('6').focusHost).toBe(itemFor('6')); + }); + + it('resolves a non-movable candidate', () => { + expect(candidateFor('5').orderable).toBe(false); + }); + + it('does not resolve a truncation marker as a candidate', () => { + const marker = root.querySelector('[data-sortable-lists-prev-item-id]')!; + + expect(resolveCandidate(root, marker)).toBeNull(); + }); + + it('does not resolve anything outside the root', () => { + expect(resolveCandidate(root, document.body)).toBeNull(); + }); + + it('refuses to resolve a candidate that declares no type', () => { + const untyped = document.createElement('li'); + untyped.setAttribute('data-controller', 'sortable-lists--item'); + untyped.setAttribute('data-sortable-lists--item-id-value', '99'); + root.querySelector('ul')!.appendChild(untyped); + + expect(resolveCandidate(root, untyped)).toBeNull(); + }); + + // An independently nested root is an ownership boundary. + it('does not own items belonging to a nested root', () => { + const nested = document.createElement('div'); + nested.setAttribute('data-controller', 'sortable-lists'); + nested.innerHTML = ` +
      +
        +
      • +
      +
      + `; + root.appendChild(nested); + const inner = nested.querySelector('[data-sortable-lists--item-id-value="90"]')!; + + expect(resolveCandidate(root, inner)).toBeNull(); + expect(orderedItemElements(root)).not.toContain(inner); + }); + + it('orders selected items by live document order across lists', () => { + const keys = new Set(['4', '1', '3'].map((id) => selectionKey({ type: 'work_package', id }))); + + expect(orderedSelectedItems(root, keys).map((item) => item.id)).toEqual(['1', '3', '4']); + }); + + it('lists only live orderable items', () => { + expect(liveOrderableItems(root).map((item) => item.id)).toEqual(['1', '2', '3', '4']); + }); + + it('resolves an ascending range within one list', () => { + const anchor = { type: 'work_package', id: '1', listKey: 'sprint:7' }; + + expect(resolveRangeItems(root, anchor, candidateFor('2'), rowsContainerFor(itemFor('2')))).toEqual({ ok: true, items: [{ type: 'work_package', id: '1' }, { type: 'work_package', id: '2' }] }); + }); + + it('resolves a descending range within one list', () => { + const anchor = { type: 'work_package', id: '2', listKey: 'sprint:7' }; + + expect(resolveRangeItems(root, anchor, candidateFor('1'), rowsContainerFor(itemFor('1')))).toEqual({ ok: true, items: [{ type: 'work_package', id: '1' }, { type: 'work_package', id: '2' }] }); + }); + + it('rejects a range that would cross a truncation marker', () => { + const anchor = { type: 'work_package', id: '2', listKey: 'sprint:7' }; + + expect(resolveRangeItems(root, anchor, candidateFor('3'), rowsContainerFor(itemFor('3')))).toEqual({ ok: false, reason: 'unavailable' }); + }); + + it('rejects a range that would cross a list boundary', () => { + const anchor = { type: 'work_package', id: '3', listKey: 'sprint:7' }; + + expect(resolveRangeItems(root, anchor, candidateFor('4'), rowsContainerFor(itemFor('4')))).toEqual({ ok: false, reason: 'crossList' }); + }); + + it('does not take a same-id item of another type for the anchor', () => { + const decoy = document.createElement('li'); + decoy.setAttribute('data-controller', 'sortable-lists--item'); + decoy.setAttribute('data-sortable-lists--item-id-value', '1'); + decoy.setAttribute('data-sortable-lists--item-type-value', 'decoy'); + root.querySelector('ul')!.prepend(decoy); + const anchor = { type: 'work_package', id: '1', listKey: 'sprint:7' }; + const candidate = candidateFor('2'); + + expect(resolveRangeItems(root, anchor, candidate, rowsContainerFor(candidate.itemElement))).toEqual({ ok: true, items: [{ type: 'work_package', id: '1' }, { type: 'work_package', id: '2' }] }); + }); + + // A structural row holding only a nested list has no item of its own; the + // nested list's first field must not be mistaken for one. + it('rejects a range across a row that only hosts a nested list', () => { + const hostRow = document.createElement('li'); + hostRow.innerHTML = ` +
      +
        +
      • +
      +
      + `; + itemFor('1').after(hostRow); + const anchor = { type: 'work_package', id: '1', listKey: 'sprint:7' }; + const candidate = candidateFor('2'); + + expect(resolveRangeItems(root, anchor, candidate, rowsContainerFor(candidate.itemElement))).toEqual({ ok: false, reason: 'unavailable' }); + }); + + it('rejects a range whose anchor id was never a row in this list', () => { + const anchor = { type: 'work_package', id: '99', listKey: 'sprint:7' }; + + expect(resolveRangeItems(root, anchor, candidateFor('2'), rowsContainerFor(itemFor('2')))).toEqual({ ok: false, reason: 'unavailable' }); + }); + + // Refused, not trimmed to the movable cards — and with its own reason, + // since expanding the list can never resolve a locked card. + it('refuses a range that would include a non-movable card', () => { + const anchor = { type: 'work_package', id: '4', listKey: 'sprint:8' }; + + expect(resolveRangeItems(root, anchor, candidateFor('5'), rowsContainerFor(itemFor('5')))).toEqual({ ok: false, reason: 'locked' }); + }); + + // list-dom's contract lets a row wrap its item instead of being it. + it('resolves a range across rows that wrap their item element', () => { + const wrappingRoot = document.createElement('div'); + wrappingRoot.setAttribute('data-controller', 'sortable-lists'); + wrappingRoot.innerHTML = ` +
      +
        +
        • +
        • +
        • +
      +
      + `; + document.body.appendChild(wrappingRoot); + + try { + const wrappedItemFor = (id:string) => wrappingRoot.querySelector( + `[data-sortable-lists--item-id-value="${id}"]`, + )!; + const anchor = { type: 'work_package', id: '20', listKey: 'sprint:20' }; + const candidate = resolveCandidate(wrappingRoot, wrappedItemFor('22'))!; + + expect(resolveRangeItems(wrappingRoot, anchor, candidate, rowsContainerFor(candidate.itemElement))).toEqual({ ok: true, items: [{ type: 'work_package', id: '20' }, { type: 'work_package', id: '21' }, { type: 'work_package', id: '22' }] }); + } finally { + wrappingRoot.remove(); + } + }); + + // A candidate whose item lives outside the list's rows container has no + // row to find there. + it('rejects a range when the candidate item sits outside the rows container', () => { + const list = root.querySelector('[data-sortable-lists--list-id-value="7"]')!; + const strayItem = document.createElement('div'); + strayItem.setAttribute('data-controller', 'sortable-lists--item'); + strayItem.setAttribute('data-sortable-lists--item-id-value', '30'); + strayItem.setAttribute('data-sortable-lists--item-type-value', 'work_package'); + list.appendChild(strayItem); + + const anchor = { type: 'work_package', id: '1', listKey: 'sprint:7' }; + const candidate = resolveCandidate(root, strayItem)!; + + expect(resolveRangeItems(root, anchor, candidate, rowsContainerFor(candidate.itemElement))).toEqual({ ok: false, reason: 'unavailable' }); + }); + + it('applies and clears the batch presentation', () => { + applySelectionPresentation(root, new Set([selectionKey({ type: 'work_package', id: '1' }), selectionKey({ type: 'work_package', id: '3' })]), 'selected-description'); + + expect(itemFor('1').hasAttribute(batchSelectedAttribute)).toBe(true); + expect(itemFor('2').hasAttribute(batchSelectedAttribute)).toBe(false); + + applySelectionPresentation(root, new Set([selectionKey({ type: 'work_package', id: '3' })]), 'selected-description'); + + expect(itemFor('1').hasAttribute(batchSelectedAttribute)).toBe(false); + expect(itemFor('3').hasAttribute(batchSelectedAttribute)).toBe(true); + }); + + // An accessible description is computed from the focused element's own + // `aria-describedby` and never inherited from an ancestor, so it belongs on + // the focus host: in Backlogs the row never receives focus, the card does. + it('describes a selected card and stops describing a deselected one', () => { + const focusHost = document.createElement('div'); + focusHost.setAttribute('data-sortable-lists--item-target', 'focus'); + itemFor('1').appendChild(focusHost); + + applySelectionPresentation(root, new Set([selectionKey({ type: 'work_package', id: '1' })]), 'selected-description'); + + expect(focusHost.getAttribute('aria-describedby')).toBe('selected-description'); + expect(itemFor('1').hasAttribute('aria-describedby')).toBe(false); + + applySelectionPresentation(root, new Set(), 'selected-description'); + + expect(focusHost.hasAttribute('aria-describedby')).toBe(false); + }); + + // Nothing prunes duplicates on read, so a repeated apply is the only thing + // that catches a regression of the write-time de-duplication. + it('describes the outer item, not a nested item\'s focus target', () => { + appendNestedList(); + const nestedTarget = itemFor('60'); + nestedTarget.setAttribute('data-sortable-lists--item-target', 'focus'); + + applySelectionPresentation(root, new Set([selectionKey({ type: 'section', id: '6' })]), 'selected-description'); + + expect(itemFor('6').getAttribute('aria-describedby')).toBe('selected-description'); + expect(nestedTarget.hasAttribute('aria-describedby')).toBe(false); + }); + + it('does not accumulate duplicate description tokens on repeated apply', () => { + applySelectionPresentation(root, new Set([selectionKey({ type: 'work_package', id: '1' })]), 'selected-description'); + applySelectionPresentation(root, new Set([selectionKey({ type: 'work_package', id: '1' })]), 'selected-description'); + + expect(itemFor('1').getAttribute('aria-describedby')).toBe('selected-description'); + }); + + it('leaves a description the card already had', () => { + itemFor('1').setAttribute('aria-describedby', 'card-hint'); + + applySelectionPresentation(root, new Set([selectionKey({ type: 'work_package', id: '1' })]), 'selected-description'); + expect(itemFor('1').getAttribute('aria-describedby')).toBe('card-hint selected-description'); + + applySelectionPresentation(root, new Set(), 'selected-description'); + expect(itemFor('1').getAttribute('aria-describedby')).toBe('card-hint'); + }); + + it('skips the description wiring when no description element is configured', () => { + applySelectionPresentation(root, new Set([selectionKey({ type: 'work_package', id: '1' })]), ''); + + expect(itemFor('1').hasAttribute('aria-describedby')).toBe(false); + expect(itemFor('1').hasAttribute(batchSelectedAttribute)).toBe(true); + }); + + it('finds the next and previous item within one list', () => { + expect(neighbourItem(root, itemFor('1'), 1)).toBe(itemFor('2')); + expect(neighbourItem(root, itemFor('2'), -1)).toBe(itemFor('1')); + }); + + it('does not step across a list boundary', () => { + expect(neighbourItem(root, itemFor('3'), 1)).toBeNull(); + }); + + // Not filtered by movability, unlike listBoundaryItem below. + it('steps onto a non-movable card with the arrow', () => { + expect(neighbourItem(root, itemFor('4'), 1)).toBe(itemFor('5')); + }); + + it('finds the first and last item of the containing list', () => { + expect(listBoundaryItem(root, itemFor('2'), 'first')).toBe(itemFor('1')); + expect(listBoundaryItem(root, itemFor('2'), 'last')).toBe(itemFor('3')); + }); + + // Sprint 8 holds movable 4 and non-movable 5 (see the fixture comment + // above): the trailing non-movable card must not become the End target. + it('skips a non-movable card at the list boundary', () => { + expect(listBoundaryItem(root, itemFor('4'), 'last')).toBe(itemFor('4')); + }); + + // A section item hosting its own list is an ownership boundary too. + it('does not step into a list nested inside the list', () => { + appendNestedList(); + + expect(neighbourItem(root, itemFor('6'), 1)).toBeNull(); + }); + + it('confines list-scoped select-all to the items the list itself owns', () => { + appendNestedList(); + + expect(liveOrderableListItems(root, itemFor('1')).map((item) => item.id)).toEqual(['1', '2', '3', '6']); + }); + + it('returns null when no movable card remains in the list', () => { + itemFor('4').setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + + expect(listBoundaryItem(root, itemFor('4'), 'first')).toBeNull(); + expect(listBoundaryItem(root, itemFor('4'), 'last')).toBeNull(); + }); +}); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection.ts new file mode 100644 index 000000000000..ee3ef3044692 --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection.ts @@ -0,0 +1,337 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { selectionKey, type SelectionAnchor, type SelectionItem, type SelectionKey } from 'core-common/batch-selection'; +import { attributeTokenList } from 'core-app/shared/helpers/dom-helpers'; +import { + isOrderableItem, + resolveItemType, + sortableListsRootSelector, + resolveItemElement, + resolveItemId, + rowOf, + sortableItemSelector, + sortableListSelector, +} from './list-dom'; + +// Distinct from `aria-current`: a card may be either, both, or neither. +export const batchSelectedAttribute = 'data-batch-selected'; + +export interface SelectionCandidate extends SelectionItem { + itemElement:HTMLElement; + // The element the consumer made focusable, and the boundary an + // interactive-descendant check stops at. + focusHost:HTMLElement; + listKey:string; + orderable:boolean; +} + +export const itemFocusTargetSelector = '[data-sortable-lists--item-target~="focus"]'; + +// Decides only whether a range stays inside the list it started in. Derived +// from the list's own values rather than its DOM id, so every consumer keys +// its lists the same way. +function listKeyOf(listElement:HTMLElement):string { + const type = listElement.getAttribute('data-sortable-lists--list-type-value') ?? ''; + const id = listElement.getAttribute('data-sortable-lists--list-id-value') ?? ''; + + return `${type}:${id}`; +} + +// Bounded to the item's own subtree: a nested list's items carry focus +// targets of their own, and a descendant's must never stand in for the +// outer item's. +function focusHostOf(itemElement:HTMLElement):HTMLElement { + return Array.from(itemElement.querySelectorAll(itemFocusTargetSelector)) + .find((target) => target.closest(sortableItemSelector) === itemElement) ?? itemElement; +} + +// A child belongs to the nearest root, not to any root containing it: an +// independently nested root is an ownership boundary. +function ownsElement(root:HTMLElement, element:Element):boolean { + return element.closest(sortableListsRootSelector) === root; +} + +function ownerList(root:HTMLElement, itemElement:HTMLElement):HTMLElement|null { + const list = itemElement.closest(sortableListSelector); + + return list && ownsElement(root, list) ? list : null; +} + +function rowItem(row:Element, rowsContainer:Element, root:HTMLElement, list:HTMLElement):HTMLElement|null { + const item = resolveItemElement(row, rowsContainer); + + return item && ownerList(root, item) === list ? item : null; +} + +export function orderedItemElements(root:HTMLElement):HTMLElement[] { + return Array.from(root.querySelectorAll(sortableItemSelector)) + .filter((item) => ownsElement(root, item)); +} + +/** + * The item a gesture landed on, or null when the gesture did not land on one. + * + * A structural row such as a truncation marker is not an item, and so not a + * candidate whose selection could be refused either. + */ +export function resolveCandidate(root:HTMLElement, target:EventTarget|null):SelectionCandidate|null { + if (!(target instanceof Element) || !root.contains(target)) { + return null; + } + + const itemElement = target.closest(sortableItemSelector); + const id = itemElement ? resolveItemId(itemElement) : null; + if (!itemElement || !id || !root.contains(itemElement)) { + return null; + } + + const list = ownerList(root, itemElement); + if (!list) { + return null; + } + + // Type is half of identity: an item declaring none cannot be identified, + // and so cannot be selected. + const type = resolveItemType(itemElement); + if (type === null) { + return null; + } + + return { + type, + itemElement, + focusHost: focusHostOf(itemElement), + id, + listKey: listKeyOf(list), + orderable: isOrderableItem(itemElement), + }; +} + +// Live document order: a morph can reorder rows underneath a selection formed +// minutes ago. +export function orderedSelectedItems(root:HTMLElement, keys:ReadonlySet):SelectionItem[] { + return orderedItemElements(root) + .map((item) => itemIdentity(item)) + .filter((item):item is SelectionItem => item !== null && keys.has(selectionKey(item))); +} + +function itemIdentity(itemElement:Element):SelectionItem|null { + const id = resolveItemId(itemElement); + const type = resolveItemType(itemElement); + + return id && type ? { type, id } : null; +} + +export function liveOrderableItems(root:HTMLElement):SelectionItem[] { + return orderedItemElements(root) + .filter(isOrderableItem) + .map((item) => itemIdentity(item)) + .filter((item):item is SelectionItem => item !== null); +} + +export function liveOrderableListItems(root:HTMLElement, from:HTMLElement):SelectionItem[] { + return listItems(root, from) + .filter(isOrderableItem) + .map((item) => itemIdentity(item)) + .filter((item):item is SelectionItem => item !== null); +} + +export function liveOrderableKeys(root:HTMLElement):Set { + return new Set(liveOrderableItems(root).map(selectionKey)); +} + +// `unavailable` is remediable by expanding the list, most often a truncation +// marker in the span. `locked` is not: a card the user may not move sits in +// it, and expanding changes nothing. +export type RangeUnavailableReason = 'crossList'|'unavailable'|'locked'; + +export type RangeResolution = + | { ok:true; items:SelectionItem[] } + | { ok:false; reason:RangeUnavailableReason }; + +/** + * The contiguous, orderable range between the anchor and the candidate, or a + * reason the range cannot be expressed. + * + * A range that would cross a list boundary, a truncation marker or a card the + * user may not move is refused whole rather than trimmed. + */ +export function resolveRangeItems( + root:HTMLElement, + anchor:SelectionAnchor, + candidate:SelectionCandidate, + // Must be the container moves use: a row is any direct child of it, not + // necessarily an item element, so it cannot be derived from the item's own + // parent. + rowsContainer:HTMLElement|null, +):RangeResolution { + if (anchor.listKey !== candidate.listKey) { + return { ok: false, reason: 'crossList' }; + } + + const list = ownerList(root, candidate.itemElement); + if (!list || !rowsContainer) { + return { ok: false, reason: 'unavailable' }; + } + + const rows = Array.from(rowsContainer.children); + const anchorKey = selectionKey(anchor); + const anchorRow = rows.find((row) => { + const item = rowItem(row, rowsContainer, root, list); + const identity = item && itemIdentity(item); + return identity !== null && selectionKey(identity) === anchorKey; + }); + // A candidate whose item sits outside the rows container has no row here. + const candidateRow = rowOf(rowsContainer, candidate.itemElement); + if (!anchorRow || !candidateRow) { + return { ok: false, reason: 'unavailable' }; + } + + const from = rows.indexOf(anchorRow); + const to = rows.indexOf(candidateRow); + const span = rows.slice(Math.min(from, to), Math.max(from, to) + 1); + + const items:SelectionItem[] = []; + for (const row of span) { + const item = rowItem(row, rowsContainer, root, list); + const id = item ? resolveItemId(item) : null; + // A truncation marker in the span and a card the user cannot move are + // both hard boundaries, but only the first can be resolved by expanding. + if (!item || !id) { + return { ok: false, reason: 'unavailable' }; + } + + if (!isOrderableItem(item)) { + return { ok: false, reason: 'locked' }; + } + + const type = resolveItemType(item); + if (!type) { + return { ok: false, reason: 'unavailable' }; + } + + items.push({ type, id }); + } + + return { ok: true, items }; +} + +/** + * Marks the selected items and describes them to assistive technology. + * + * `describedById` points at an element the consumer renders once, holding the + * word for "selected"; an empty string skips the description entirely. + * + * `data-batch-selected` goes on the item element, `aria-describedby` on the + * focus host: an accessible description is computed from the focused + * element's own attribute and never inherited from an ancestor. + */ +export function applySelectionPresentation( + root:HTMLElement, + keys:ReadonlySet, + describedById:string, +):void { + for (const item of orderedItemElements(root)) { + const identity = itemIdentity(item); + const focusHost = focusHostOf(item); + + if (identity && keys.has(selectionKey(identity))) { + item.setAttribute(batchSelectedAttribute, ''); + addDescription(focusHost, describedById); + } else { + item.removeAttribute(batchSelectedAttribute); + removeDescription(focusHost, describedById); + } + } +} + +// The card may already be described by something of its own, so the shared +// reference is added to the token list rather than replacing it. +function addDescription(item:HTMLElement, describedById:string):void { + if (describedById === '') { + return; + } + + attributeTokenList(item, 'aria-describedby').add(describedById); +} + +function removeDescription(item:HTMLElement, describedById:string):void { + if (describedById === '') { + return; + } + + const describedBy = attributeTokenList(item, 'aria-describedby'); + describedBy.remove(describedById); + + // `remove` leaves an empty attribute behind rather than dropping it, and a + // card that describes nothing should carry no `aria-describedby` at all. + if (describedBy.length === 0) { + item.removeAttribute('aria-describedby'); + } +} + +// Filtered back to the items this list owns: a nested topology puts another +// list's items inside this one's subtree. +function listItems(root:HTMLElement, from:HTMLElement):HTMLElement[] { + const list = ownerList(root, from); + + return list + ? Array.from(list.querySelectorAll(sortableItemSelector)) + .filter((item) => ownerList(root, item) === list) + : []; +} + +// Arrows step through the list as rendered, fixed cards included. Not +// symmetric with listBoundaryItem below, which does filter. +export function neighbourItem(root:HTMLElement, from:HTMLElement, offset:1|-1):HTMLElement|null { + const items = listItems(root, from); + const index = items.indexOf(from); + + if (index === -1) { + return null; + } + + return items[index + offset] ?? null; +} + +// Home/End land on the first/last *orderable* card, so a leading or trailing +// fixed card is skipped rather than becoming the jump target. +export function listBoundaryItem( + root:HTMLElement, + from:HTMLElement, + edge:'first'|'last', +):HTMLElement|null { + const items = listItems(root, from).filter(isOrderableItem); + + if (items.length === 0) { + return null; + } + + return edge === 'first' ? items[0] : items[items.length - 1]; +} diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/testing/selection-translations.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/testing/selection-translations.ts new file mode 100644 index 000000000000..ee586e2cc921 --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/testing/selection-translations.ts @@ -0,0 +1,36 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +export const selectionTranslations = { + cleared: '[cleared]', + not_selectable: '[not_selectable]', + range_blocked: '[range_blocked]', + range_restarted: { one: '[range_restarted:1]', other: '[range_restarted:%{count}]' }, + range_unavailable: '[range_unavailable]', + selected: { one: '[selected:1]', other: '[selected:%{count}]' }, +}; diff --git a/frontend/src/stimulus/helpers/platform.ts b/frontend/src/stimulus/helpers/platform.ts new file mode 100644 index 000000000000..a80e8bace45a --- /dev/null +++ b/frontend/src/stimulus/helpers/platform.ts @@ -0,0 +1,49 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +interface NavigatorUAData { + platform?:string; +} + +/** + * Whether the browser is running on an Apple platform. + * + * Matters for modifier keys: Ctrl-click is the secondary-click gesture on + * macOS, where Cmd is the multi-select modifier instead. Treating Ctrl as + * multi-select there fights the contextual menu. + * + * `navigator.platform` is deprecated but still the only universally + * available signal, so `userAgentData` is preferred where a browser exposes + * it. + */ +export function isApplePlatform():boolean { + const uaData = (navigator as Navigator & { userAgentData?:NavigatorUAData }).userAgentData; + const platform = uaData?.platform ?? navigator.platform; + + return /mac|iphone|ipad|ipod/i.test(platform); +} diff --git a/frontend/src/turbo/pragmatic-dnd-morph-attributes.spec.ts b/frontend/src/turbo/pragmatic-dnd-morph-attributes.spec.ts index 4a25a78ef827..77399faf58f8 100644 --- a/frontend/src/turbo/pragmatic-dnd-morph-attributes.spec.ts +++ b/frontend/src/turbo/pragmatic-dnd-morph-attributes.spec.ts @@ -92,8 +92,8 @@ describe('Pragmatic DnD morph attribute preservation', () => { }); }); - it('preserves the split view selection markers during Turbo morphs', () => { - ['data-selected', 'aria-current'].forEach((attributeName) => { + it('preserves the batch selection and current-work-package markers during Turbo morphs', () => { + ['data-batch-selected', 'aria-current'].forEach((attributeName) => { const element = appendSortableRow(); const event = beforeMorphAttributeEvent(attributeName); diff --git a/frontend/src/turbo/pragmatic-dnd-morph-attributes.ts b/frontend/src/turbo/pragmatic-dnd-morph-attributes.ts index 1efac1248b15..9717e24baeef 100644 --- a/frontend/src/turbo/pragmatic-dnd-morph-attributes.ts +++ b/frontend/src/turbo/pragmatic-dnd-morph-attributes.ts @@ -27,6 +27,7 @@ //++ import { sortableListsRootSelector } from '../stimulus/controllers/dynamic/sortable-lists/list-dom'; +import { batchSelectedAttribute } from '../stimulus/controllers/dynamic/sortable-lists/selection'; // `data-dragging` is shared with the legacy generic-drag-and-drop controller, // so preservation is scoped to elements inside a sortable-lists root. @@ -45,10 +46,13 @@ const preservedAttributes = new Set([ 'data-drop-position', 'data-drop-position-owner', 'data-drop-container', - // Split view selection markers set by the backlogs work-package - // controller; the controller only re-syncs them on `turbo:visit`, so a - // morph would otherwise strip the highlight of the open work package. - 'data-selected', + // Batch membership, owned by the sortable-lists root. The root reconciles it + // after every morph batch anyway, but preserving it avoids a visible flash + // between the morph and that reconciliation. + batchSelectedAttribute, + // Current-work-package marker set by the backlogs work-package controller, + // which only re-syncs it on `turbo:visit`; a morph would otherwise strip + // the highlight of the open work package. 'aria-current', ]); diff --git a/modules/backlogs/app/components/backlogs/concerns/work_package_movability.rb b/modules/backlogs/app/components/backlogs/concerns/work_package_movability.rb index d9b4cf9cf6f2..ca018d86e662 100644 --- a/modules/backlogs/app/components/backlogs/concerns/work_package_movability.rb +++ b/modules/backlogs/app/components/backlogs/concerns/work_package_movability.rb @@ -75,5 +75,18 @@ def sortable? def movable? sortable? && !work_package.readonly_status? end + + # What ordering this card takes part in. + # + # `fixed` is no part at all: no drag, no positional move, no selection. + # `confined` reorders within its own list but is refused by every other + # container. `free` may move to any list that accepts it. + # + # @return [String] one of "fixed", "confined", "free". + def mobility + return "fixed" unless sortable? + + movable? ? "free" : "confined" + end end end diff --git a/modules/backlogs/app/components/backlogs/selection_description_component.html.erb b/modules/backlogs/app/components/backlogs/selection_description_component.html.erb new file mode 100644 index 000000000000..ed8eef46f316 --- /dev/null +++ b/modules/backlogs/app/components/backlogs/selection_description_component.html.erb @@ -0,0 +1,34 @@ +<%# -- copyright +OpenProject is an open source project management software. +Copyright (C) the OpenProject GmbH + +This program is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License version 3. + +OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +Copyright (C) 2006-2013 Jean-Philippe Lang +Copyright (C) 2010-2013 the ChiliProject Team + +This program is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License +as published by the Free Software Foundation; either version 2 +of the License, or (at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program; if not, write to the Free Software +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +See COPYRIGHT and LICENSE files for more details. + +++%> + +<%# Permanently hidden: `aria-describedby` reaches it regardless, and the + planning columns keep their geometry. %> +<%= render(Primer::Box.new(tag: :span, id: DESCRIPTION_ID, hidden: true)) do %> + <%= I18n.t("js.backlogs.selection.card_state") %> +<% end %> diff --git a/modules/backlogs/app/components/backlogs/selection_description_component.rb b/modules/backlogs/app/components/backlogs/selection_description_component.rb new file mode 100644 index 000000000000..f49243af0aa2 --- /dev/null +++ b/modules/backlogs/app/components/backlogs/selection_description_component.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Backlogs + # The single description every selected card points at through + # `aria-describedby`, so batch membership reaches assistive technology + # without duplicating the string onto every card. + class SelectionDescriptionComponent < ApplicationComponent + DESCRIPTION_ID = "op-backlogs-batch-selected-description" + end +end diff --git a/modules/backlogs/app/components/backlogs/work_package_card_list_item_component.rb b/modules/backlogs/app/components/backlogs/work_package_card_list_item_component.rb index d68b9d3f2b9f..02e53d4c5e3f 100644 --- a/modules/backlogs/app/components/backlogs/work_package_card_list_item_component.rb +++ b/modules/backlogs/app/components/backlogs/work_package_card_list_item_component.rb @@ -44,7 +44,7 @@ def build_card end # Every sortable card drags: a read-only one stays a drag source too, only - # confined to its own list (see {#confined?}). + # confined to its own list (see {#mobility}). def draggable? sortable? end @@ -75,7 +75,7 @@ def card_arguments end def card_data - data = { + { story: true, # Non-movable cards opt in too: they have no move actions, but their # singular menu is still worth reaching contextually. @@ -83,12 +83,10 @@ def card_data backlogs__work_package_id_value: work_package.id, backlogs__work_package_display_id_value: work_package.display_id, backlogs__work_package_split_url_value: split_url, - backlogs__work_package_full_url_value: full_url + backlogs__work_package_full_url_value: full_url, + # The card, not the row, carries the tab stop. + sortable_lists__item_target: "preview handle focus" } - - return data unless draggable? - - data.merge(sortable_lists__item_target: "preview handle") end # @return [Hash] ARIA wiring announcing the card's Enter activation and its @@ -103,20 +101,15 @@ def card_aria } end - # An unmovable card registers as a sortable item like any other: it keeps - # its drag and its positional moves, stays a drop target and keeps counting - # as a row of its list. The confined value is what pins it to that list. + # Every card row is a sortable item, movable or not: a non-movable row is + # still an addressable position its neighbours anchor drops on. def row_data - sortable? ? sortable_item_data : {} - end - - def sortable_item_data { controller: "sortable-lists--item", sortable_lists__item_id_value: work_package.id, sortable_lists__item_label_value: work_package.to_fs(:caption), sortable_lists__item_type_value: "work_package", - sortable_lists__item_confined_value: confined?, + sortable_lists__item_mobility_value: mobility, # Native drag payload for external consumers; the same absolute URL # as the card menu's "Copy URL to clipboard" item. The label above # doubles as the link text of the text/html flavour. @@ -124,12 +117,6 @@ def sortable_item_data } end - # Whether the card's drag is pinned to its own list: it may reorder in - # place, but no other container accepts it. - def confined? - sortable? && !movable? - end - public def row_args diff --git a/modules/backlogs/app/helpers/backlogs/common_helper.rb b/modules/backlogs/app/helpers/backlogs/common_helper.rb index dd5766caf289..1ed6d9dd22aa 100644 --- a/modules/backlogs/app/helpers/backlogs/common_helper.rb +++ b/modules/backlogs/app/helpers/backlogs/common_helper.rb @@ -49,6 +49,16 @@ def sprint_management_allowed? user_allowed?(:share_sprint) end + # Batch selection is the same capability as ordering: without the right to + # reorder, enabling it would only cost the viewer their Space, arrow, + # Home/End and Ctrl/Cmd+A keys. + # + # Takes the project because the backlog view, unlike the components the + # predicates above serve, exposes no `project` method. + def batch_selection_allowed?(project) + user_allowed?(:manage_sprint_items, project:) + end + def backlog_filters RequestStore.fetch(:backlog_filters) do Backlogs::BacklogFilters.from_params(permitted_params.backlog_filters) diff --git a/modules/backlogs/app/views/backlogs/backlog/_backlog_list.html.erb b/modules/backlogs/app/views/backlogs/backlog/_backlog_list.html.erb index 43fde6457009..bad4c3f83314 100644 --- a/modules/backlogs/app/views/backlogs/backlog/_backlog_list.html.erb +++ b/modules/backlogs/app/views/backlogs/backlog/_backlog_list.html.erb @@ -29,6 +29,10 @@ See COPYRIGHT and LICENSE files for more details. <%# Attributes on this frame tag are ignored; only its children are reconciled — set them in show.html.erb. %> <%= turbo_frame_tag "backlogs_container" do %> + <%# Rendered once for the whole page: every selected card in either column + points its `aria-describedby` at this one element. %> + <%= render(Backlogs::SelectionDescriptionComponent.new) %> +
      <%= diff --git a/modules/backlogs/app/views/backlogs/backlog/show.html.erb b/modules/backlogs/app/views/backlogs/backlog/show.html.erb index 555d771c275d..961b551c20d6 100644 --- a/modules/backlogs/app/views/backlogs/backlog/show.html.erb +++ b/modules/backlogs/app/views/backlogs/backlog/show.html.erb @@ -53,6 +53,10 @@ See COPYRIGHT and LICENSE files for more details. sortable_lists_optimistic_value: true, action: "#{Backlogs::WorkPackagesController::WORK_PACKAGE_MOVED_EVENT}@document->backlogs--split-view-sync#onWorkPackageMoved", sortable_lists_move_url_template_value: backlogs_move_url_template(@project), + sortable_lists_selection_enabled_value: batch_selection_allowed?(@project), + sortable_lists_announcement_scope_value: "js.backlogs.selection", + sortable_lists_selection_description_id_value: + Backlogs::SelectionDescriptionComponent::DESCRIPTION_ID, sortable_lists_sortable_lists__list_outlet: "#backlogs_container [data-controller~='sortable-lists--list']", sortable_lists_sortable_lists__item_outlet: "#backlogs_container [data-controller~='sortable-lists--item']", sortable_lists_sortable_lists__scrollable_outlet: "#backlogs_container [data-controller~='sortable-lists--scrollable']" diff --git a/modules/backlogs/config/locales/js-en.yml b/modules/backlogs/config/locales/js-en.yml index 2b16dfa89bf4..a07d340d8d2d 100644 --- a/modules/backlogs/config/locales/js-en.yml +++ b/modules/backlogs/config/locales/js-en.yml @@ -29,6 +29,19 @@ --- en: js: + backlogs: + selection: + card_state: "Selected" + cleared: "Selection cleared." + not_selectable: "Selection unchanged. This work package takes no part in this list's ordering." + range_blocked: "Selection unchanged. That range contains a work package that takes no part in this list's ordering." + range_restarted: + one: "Could not extend the range. 1 work package selected." + other: "Could not extend the range. %{count} work packages selected." + range_unavailable: "Selection unchanged. Expand this list to select that range." + selected: + one: "1 work package selected." + other: "%{count} work packages selected." burndown: day: "Day" points: "Points" diff --git a/modules/backlogs/spec/components/backlogs/bucket_component_spec.rb b/modules/backlogs/spec/components/backlogs/bucket_component_spec.rb index e55610b570c1..19fbdf378900 100644 --- a/modules/backlogs/spec/components/backlogs/bucket_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/bucket_component_spec.rb @@ -164,7 +164,7 @@ def render_component expect(rendered_component).to have_css(".op-work-package-card") do |card| expect(card["data-controller"].split).to include("backlogs--work-package") - expect(card["data-sortable-lists--item-target"]).to eq("preview handle") + expect(card["data-sortable-lists--item-target"]).to eq("preview handle focus") expect(card["data-backlogs--work-package-split-url-value"]) .to end_with(project_backlogs_backlog_details_path(project, work_package)) expect(card["data-backlogs--work-package-full-url-value"]) @@ -241,13 +241,13 @@ def render_component position: 1) end - it "does not mark work package rows as draggable" do - expect(rendered_component).to have_css(".Box-row#work_package_#{work_package.id}") - expect(rendered_component) - .to have_no_css(".Box-row#work_package_#{work_package.id}[data-sortable-lists--item-id-value]") + it "still renders the row as a sortable item, but fixed and not draggable" do + expect(rendered_component).to have_css(".Box-row#work_package_#{work_package.id}") do |row| + expect(row["data-controller"]).to eq("sortable-lists--item") + expect(row["data-sortable-lists--item-mobility-value"]).to eq("fixed") + end expect(rendered_component).to have_no_css(".Box-row#work_package_#{work_package.id}[draggable='true']") expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-id-value]") - expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-target]") expect(rendered_component).to have_no_css(".op-work-package-card[draggable='true']") expect(rendered_component).to have_no_css(".op-work-package-card.Box-card--draggable") end diff --git a/modules/backlogs/spec/components/backlogs/selection_description_component_spec.rb b/modules/backlogs/spec/components/backlogs/selection_description_component_spec.rb new file mode 100644 index 000000000000..0db999264b17 --- /dev/null +++ b/modules/backlogs/spec/components/backlogs/selection_description_component_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "rails_helper" + +RSpec.describe Backlogs::SelectionDescriptionComponent, type: :component do + subject(:rendered_component) { render_inline(described_class.new) && page } + + # `visible: :all` because the element is permanently `hidden`, which is the + # one form of invisibility Capybara::Node::Simple does honour. + + it "renders the description every selected card points at" do + expect(rendered_component) + .to have_css("##{described_class::DESCRIPTION_ID}", text: I18n.t("js.backlogs.selection.card_state"), + visible: :all) + end + + # `hidden` keeps this out of the planning columns' layout: reserving space + # above two independently scrolling columns moved the cards under the user's + # cursor mid-drag. + it "keeps the description hidden, since only aria-describedby reaches it" do + expect(rendered_component).to have_css("##{described_class::DESCRIPTION_ID}[hidden]", visible: :all) + end +end diff --git a/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb b/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb index c36cc73b6a82..32477700145a 100644 --- a/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb @@ -131,7 +131,7 @@ def menu_items expect(rendered_component).to have_css(".Box-row#work_package_#{work_package1.id} .op-work-package-card") do |card| expect(card["data-controller"].split).to include("backlogs--work-package") - expect(card["data-sortable-lists--item-target"]).to eq("preview handle") + expect(card["data-sortable-lists--item-target"]).to eq("preview handle focus") expect(card["data-backlogs--work-package-display-id-value"]).to eq(work_package1.display_id.to_s) end end @@ -159,13 +159,13 @@ def menu_items priority: default_priority, story_points: 5, position: 1, sprint:) end - it "does not mark work package rows as draggable" do - expect(rendered_component).to have_css(".Box-row#work_package_#{work_package1.id}") - expect(rendered_component) - .to have_no_css(".Box-row#work_package_#{work_package1.id}[data-sortable-lists--item-id-value]") + it "still renders the row as a sortable item, but fixed and not draggable" do + expect(rendered_component).to have_css(".Box-row#work_package_#{work_package1.id}") do |row| + expect(row["data-controller"]).to eq("sortable-lists--item") + expect(row["data-sortable-lists--item-mobility-value"]).to eq("fixed") + end expect(rendered_component).to have_no_css(".Box-row#work_package_#{work_package1.id}[draggable='true']") expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-id-value]") - expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-target]") expect(rendered_component).to have_no_css(".op-work-package-card[draggable='true']") expect(rendered_component).to have_no_css(".op-work-package-card.Box-card--draggable") end diff --git a/modules/backlogs/spec/components/backlogs/work_package_card_list_component_spec.rb b/modules/backlogs/spec/components/backlogs/work_package_card_list_component_spec.rb index a57a7e8b1955..0aaf1ae8129a 100644 --- a/modules/backlogs/spec/components/backlogs/work_package_card_list_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/work_package_card_list_component_spec.rb @@ -121,7 +121,7 @@ def render_component(work_packages:, container:, drag_and_drop:) expect(rendered_component).to have_css(".Box-row#work_package_#{work_package.id} .op-work-package-card") do |card| expect(card["data-story"]).to be_present expect(card["data-backlogs--work-package-id-value"]).to eq(work_package.id.to_s) - expect(card["data-sortable-lists--item-target"]).to eq("preview handle") + expect(card["data-sortable-lists--item-target"]).to eq("preview handle focus") end end diff --git a/modules/backlogs/spec/components/backlogs/work_package_card_list_item_component_spec.rb b/modules/backlogs/spec/components/backlogs/work_package_card_list_item_component_spec.rb index 0de978d3f746..6c97cf845e0f 100644 --- a/modules/backlogs/spec/components/backlogs/work_package_card_list_item_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/work_package_card_list_item_component_spec.rb @@ -81,8 +81,8 @@ controller: "sortable-lists--item", sortable_lists__item_id_value: work_package.id, sortable_lists__item_type_value: "work_package", - sortable_lists__item_confined_value: false, - sortable_lists__item_label_value: work_package.to_fs(:caption) + sortable_lists__item_label_value: work_package.to_fs(:caption), + sortable_lists__item_mobility_value: "free" ) expect(item.row_args[:draggable]).to be(true) expect(item.row_args).not_to include(:tabindex) @@ -122,11 +122,22 @@ described_class.new(work_package:, project:, container:, params:, current_user: limited_user) end + it "still renders the row as a sortable item" do + expect(item.row_args[:data]).to include(controller: "sortable-lists--item") + end + + it "marks the item as taking no part in ordering" do + expect(item.row_args[:data]).to include(sortable_lists__item_mobility_value: "fixed") + end + it "does not mark the row as draggable" do expect(item.row_args[:data]).not_to include(:sortable_lists_prev_item_id) expect(item.row_args).not_to include(:draggable) expect(item.row_args).not_to include(:tabindex) - expect(item.row_args[:data]).not_to include(:sortable_lists__item_external_url_value) + end + + it "still exposes the external URL for drag consumers" do + expect(item.row_args[:data]).to include(:sortable_lists__item_external_url_value) end it "keeps the card focusable so keyboard users can interact with it" do @@ -160,7 +171,7 @@ expect(item.row_args[:data]).to include( controller: "sortable-lists--item", sortable_lists__item_id_value: work_package.id, - sortable_lists__item_confined_value: true + sortable_lists__item_mobility_value: "confined" ) end @@ -171,7 +182,7 @@ it "still offers the card as a drag handle", :aggregate_failures do render_inline(item.card) - expect(page).to have_css(".op-work-package-card[data-sortable-lists--item-target='preview handle']") + expect(page).to have_css(".op-work-package-card[data-sortable-lists--item-target='preview handle focus']") expect(page).to have_css(".Box-card--draggable") end end @@ -183,7 +194,7 @@ it "does not confine the row" do expect(item.row_args[:draggable]).to be(true) - expect(item.row_args[:data]).to include(sortable_lists__item_confined_value: false) + expect(item.row_args[:data]).to include(sortable_lists__item_mobility_value: "free") end end end @@ -236,7 +247,7 @@ "[data-backlogs--work-package-id-value='#{work_package.id}']" \ "[data-backlogs--work-package-display-id-value='#{work_package.display_id}']" \ "[data-backlogs--work-package-full-url-value='#{work_package_path(work_package)}']" \ - "[data-sortable-lists--item-target='preview handle']" \ + "[data-sortable-lists--item-target='preview handle focus']" \ "[tabindex='0']" ) end diff --git a/modules/backlogs/spec/features/work_packages/batch_selection_spec.rb b/modules/backlogs/spec/features/work_packages/batch_selection_spec.rb new file mode 100644 index 000000000000..b03959102b89 --- /dev/null +++ b/modules/backlogs/spec/features/work_packages/batch_selection_spec.rb @@ -0,0 +1,299 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" +require_relative "../../support/pages/backlog" + +# Selenium, not Cuprite: the modifier combinations are driven through native +# `send_keys`, and only a real page can prove the root's capture-phase +# listener beats the card's own click and Enter handlers. +RSpec.describe "Backlogs batch selection", :js, :selenium, :settings_reset do + let!(:project) do + create(:project, + types: [type], + enabled_module_names: %w(work_package_tracking backlogs)) + end + let(:manage_sprint_items_role) do + create(:project_role, + permissions: %i(view_sprints + manage_sprint_items + view_work_packages + edit_work_packages)) + end + + let(:type) { create(:type) } + + let!(:sprint) { create(:sprint, project:) } + let!(:story1) { create(:work_package, sprint:, type:, project:) } + let!(:story2) { create(:work_package, sprint:, type:, project:) } + let!(:story3) { create(:work_package, sprint:, type:, project:) } + let!(:story4) { create(:work_package, sprint:, type:, project:) } + let!(:bucket) { create(:backlog_bucket, project:, name: "Backlog bucket") } + let!(:bucket_wp1) { create(:work_package, backlog_bucket: bucket, position: 1, type:, project:) } + let!(:bucket_wp2) { create(:work_package, backlog_bucket: bucket, position: 2, type:, project:) } + + let(:backlogs_page) { Pages::Backlog.new(project) } + + current_user do + create(:user, member_with_roles: { project => manage_sprint_items_role }) + end + + before do + backlogs_page.visit! + end + + describe "mouse gestures" do + it "collapses a wider selection onto the clicked card and still opens its details" do + backlogs_page.select_card(story1) + backlogs_page.toggle_card(story2) + backlogs_page.select_card(story3) + + expect(page).to have_css("[data-batch-selected]", count: 1) + expect(backlogs_page.selected_card_ids).to eq([story3.id.to_s]) + backlogs_page.expect_details_view(story3) + end + + it "toggles a sparse selection across two lists without navigating" do + # Seeded with a toggle, not a plain click: a plain click schedules its + # details-pane visit on a timer, so the path assertion below would pass + # with a still-pending navigation either way. + backlogs_page.toggle_card(story1) + backlogs_page.toggle_card(bucket_wp1) + + expect(page).to have_css("[data-batch-selected]", count: 2) + expect(backlogs_page.selected_card_ids).to contain_exactly(story1.id.to_s, bucket_wp1.id.to_s) + # Had the modified click reached the card's own handler, the details + # pane would have opened and the path would have changed. + expect(page).to have_current_path(backlogs_page.path, ignore_query: true) + end + + # BatchSelection#toggle re-bases the anchor even on a deselect, so a + # later range starts from story3 rather than story1. + it "re-bases the anchor to a toggled card even when the toggle deselects it" do + backlogs_page.select_card(story1) + backlogs_page.toggle_card(story3) + backlogs_page.toggle_card(story3) + + expect(page).to have_css("[data-batch-selected]", count: 1) + + backlogs_page.extend_selection_to(story4) + + expect(page).to have_css("[data-batch-selected]", count: 3) + expect(backlogs_page.selected_card_ids).to eq([story1, story3, story4].map { it.id.to_s }) + end + + it "resizes one fixed-anchor range rather than walking it" do + backlogs_page.select_card(story1) + backlogs_page.extend_selection_to(story3) + + expect(page).to have_css("[data-batch-selected]", count: 3) + expect(backlogs_page.selected_card_ids).to eq([story1, story2, story3].map { it.id.to_s }) + + backlogs_page.extend_selection_to(story2) + + expect(page).to have_css("[data-batch-selected]", count: 2) + expect(backlogs_page.selected_card_ids).to eq([story1, story2].map { it.id.to_s }) + end + + # The follow-up range proves the cross-list gesture re-anchored rather + # than being refused outright. + it "refuses to extend a range across lists and starts a fresh selection there" do + backlogs_page.select_card(story1) + backlogs_page.extend_selection_to(bucket_wp2) + + expect(page).to have_css("[data-batch-selected]", count: 1) + expect(backlogs_page.selected_card_ids).to eq([bucket_wp2.id.to_s]) + + backlogs_page.extend_selection_to(bucket_wp1) + + expect(page).to have_css("[data-batch-selected]", count: 2) + expect(backlogs_page.selected_card_ids).to eq([bucket_wp1, bucket_wp2].map { it.id.to_s }) + end + end + + describe "keyboard interaction" do + it "paints a row with a different background once it is selected" do + # Focus is established first and held constant across both reads: a + # click would move focus, open the details pane and toggle membership at + # once, leaving the colour difference unattributable. + backlogs_page.focus_work_package_card(story1) + unselected_color = backlogs_page.row_background_color(story1) + + backlogs_page.work_package_card(story1).send_keys(:space) + + expect(page).to have_css("[data-batch-selected]", count: 1) + expect(backlogs_page.row_background_color(story1)).not_to eq(unselected_color) + end + + it "selects and clears with the keyboard" do + backlogs_page.work_package_card(story1).send_keys(:space) + + expect(page).to have_css("[data-batch-selected]", count: 1) + expect(backlogs_page.selected_card_ids).to eq([story1.id.to_s]) + + backlogs_page.work_package_card(story1).send_keys(:escape) + + expect(page).to have_no_css("[data-batch-selected]") + expect(backlogs_page.selected_card_ids).to be_empty + end + + # A click on empty column space leaves focus nowhere, and Escape there + # still means "drop the selection". + it "clears the selection with Escape when no card holds focus" do + backlogs_page.toggle_card(story1) + expect(page).to have_css("[data-batch-selected]", count: 1) + + page.execute_script("document.activeElement.blur()") + backlogs_page.send_keys_to_focused_element(:escape) + + expect(page).to have_no_css("[data-batch-selected]") + expect(backlogs_page.selected_card_ids).to be_empty + end + + # The fixtures span a sprint and a bucket, so a regression to root-wide + # select-all would take this past the sprint's four cards. + it "selects every orderable card of the focused card's list with Ctrl/Cmd+A" do + backlogs_page.send_work_package_card_keys(story1, [backlogs_page.multi_select_modifier, "a"]) + + expect(page).to have_css("[data-batch-selected]", count: 4) + expect(backlogs_page.selected_card_ids).to contain_exactly( + story1.id.to_s, story2.id.to_s, story3.id.to_s, story4.id.to_s + ) + end + + it "extends the selection from a fixed anchor with Shift+ArrowDown" do + backlogs_page.work_package_card(story1).send_keys(:space) + expect(page).to have_css("[data-batch-selected]", count: 1) + + backlogs_page.send_work_package_card_keys(story1, %i[shift arrow_down]) + + expect(page).to have_css("[data-batch-selected]", count: 2) + expect(backlogs_page.selected_card_ids).to eq([story1, story2].map { it.id.to_s }) + end + + # Enter belongs to the card's own activation handler, which only a real + # page can prove. + it "opens details with Enter without the selection handler swallowing it" do + backlogs_page.send_work_package_card_keys(story2, [:enter]) + + backlogs_page.expect_details_view(story2) + end + end + + describe "selection persistence and accessibility" do + it "keeps the batch selected when the current work package changes" do + backlogs_page.select_card(story1) + # A plain click waits out a double-click window before it visits, so + # without this wait story1's pending visit lands while story3's lazily + # loaded actions menu is opening below, racing its fetch. + backlogs_page.expect_details_view(story1) + backlogs_page.toggle_card(story2) + + expect(page).to have_css("[data-batch-selected]", count: 2) + + backlogs_page.open_work_package_details(story3) + + expect(page).to have_css("[data-batch-selected]", count: 2) + expect(backlogs_page.selected_card_ids).to contain_exactly(story1.id.to_s, story2.id.to_s) + end + + it "describes a selected card to assistive technology via the shared description" do + backlogs_page.select_card(story1) + backlogs_page.toggle_card(story2) + + expect(page).to have_css("[data-batch-selected]", count: 2) + # Rendered exactly once, so the assertions below speak about one + # element rather than two that happen to agree. + backlogs_page.expect_selection_description_present + + # The card, not the row: a description is computed from the focused + # element's own `aria-describedby`, never inherited. The computed value + # also proves the browser traverses into the `hidden` element. + description = I18n.t("js.backlogs.selection.card_state") + backlogs_page.expect_work_package_card_described_as(story1, description) + backlogs_page.expect_work_package_card_described_as(story2, description) + # Without this, the two above would pass with every card described. + backlogs_page.expect_work_package_card_not_described(story3) + end + + # The expander is a frame navigation, not a morph: it swaps the inbox rows + # wholesale. The selection made before it must be back on the fresh rows, + # not stranded in the model while the cards paint as unselected. + context "with a truncated inbox" do + let!(:inbox_wps) { create_list(:work_package, 5, project:, type:) } + + # The constant has to be stubbed before the page renders, so the visit + # from the outer hook is repeated here. + before do + stub_const("Backlogs::InboxComponent::TRUNCATE_MIDDLE", 2) + backlogs_page.visit! + end + + it "keeps the selection across the show-more expander" do + backlogs_page.expect_inbox_show_more + backlogs_page.toggle_card(inbox_wps.first) + expect(backlogs_page.selected_card_ids).to eq([inbox_wps.first.id.to_s]) + + backlogs_page.click_inbox_show_more + backlogs_page.expect_no_inbox_show_more + + expect(backlogs_page.selected_card_ids).to eq([inbox_wps.first.id.to_s]) + end + end + end + + # Selection consumes Space, the arrows, Home/End and Ctrl/Cmd+A. With every + # card fixed those keys would stop scrolling the page for a capability it + # never offers, so the root does not opt in at all. + describe "without the permission to manage sprint items" do + let(:view_role) do + create(:project_role, permissions: %i(view_sprints view_work_packages)) + end + + current_user do + create(:user, member_with_roles: { project => view_role }) + end + + # `current_user` declared here registers its login after the outer + # `before` has already visited, so the page is visited again. + before do + backlogs_page.visit! + end + + it "does not enable selection, and leaves its gestures to the browser" do + backlogs_page.expect_batch_selection_disabled + + backlogs_page.toggle_card(story1) + backlogs_page.send_work_package_card_keys(story2, [:space]) + + expect(page).to have_no_css("[data-batch-selected]") + end + end +end diff --git a/modules/backlogs/spec/helpers/backlogs/common_helper_spec.rb b/modules/backlogs/spec/helpers/backlogs/common_helper_spec.rb index 7140946c62f4..8edbdcdb9ed0 100644 --- a/modules/backlogs/spec/helpers/backlogs/common_helper_spec.rb +++ b/modules/backlogs/spec/helpers/backlogs/common_helper_spec.rb @@ -62,6 +62,39 @@ end end + describe "#batch_selection_allowed?" do + let(:project) { build_stubbed(:project) } + + before do + # The helper resolves the actor through `current_user`, which a bare + # helper object does not define; the describe above stubs it the same + # way. + without_partial_double_verification do + allow(helper).to receive_messages(current_user:) + end + + mock_permissions_for(current_user) do |mock| + mock.allow_in_project(*permissions, project:) + end + end + + context "when the user may manage sprint items" do + let(:permissions) { %i[manage_sprint_items] } + + it "offers batch selection" do + expect(helper.batch_selection_allowed?(project)).to be true + end + end + + context "when the user may only view" do + let(:permissions) { %i[view_sprints view_work_packages] } + + it "does not offer batch selection" do + expect(helper.batch_selection_allowed?(project)).to be false + end + end + end + describe "#backlog_filters" do context "with no params" do let(:params) { {} } diff --git a/modules/backlogs/spec/requests/backlogs/backlog_spec.rb b/modules/backlogs/spec/requests/backlogs/backlog_spec.rb index 479958b5c08e..3618bd98ca13 100644 --- a/modules/backlogs/spec/requests/backlogs/backlog_spec.rb +++ b/modules/backlogs/spec/requests/backlogs/backlog_spec.rb @@ -156,6 +156,43 @@ expect(response.body).to include(%(data-sortable-lists--list-id-value="#{backlog_bucket.id}")) end end + + context "when a bucket filter hides the inbox" do + shared_let(:backlog_bucket) { create(:backlog_bucket, project:) } + + it "still renders the shared selection description" do + get "/projects/#{project.identifier}/backlogs/backlog", + params: { bucket_ids: [backlog_bucket.id] }, + headers: { "Turbo-Frame" => "backlogs_container" } + + expect(response).to have_http_status(:ok) + # The inbox itself is filtered out... + expect(response.body).not_to include(%(id="inbox_project_#{project.id}")) + # ...but the shared description survives for the lists that remain. + expect(response.body).to include(%(id="#{Backlogs::SelectionDescriptionComponent::DESCRIPTION_ID}")) + end + end + + # With every card fixed, opting in would only cost the viewer their + # Space, arrow, Home/End and Ctrl/Cmd+A keys. + context "when the user cannot manage sprint items" do + shared_let(:viewer_role) do + create(:project_role, permissions: %i[view_sprints view_work_packages]) + end + shared_let(:viewer) { create(:user, member_with_roles: { project => viewer_role }) } + + current_user { viewer } + + # A full page load, not a frame request: the sortable root and its + # selection values live on the wrapper in show.html.erb, which a + # Turbo-Frame request never renders. + it "does not enable batch selection" do + get "/projects/#{project.identifier}/backlogs/backlog" + + expect(response).to have_http_status(:ok) + expect(response.body).to include('data-sortable-lists-selection-enabled-value="false"') + end + end end end diff --git a/modules/backlogs/spec/support/pages/backlog.rb b/modules/backlogs/spec/support/pages/backlog.rb index efe9b88d2a61..23d7327018d8 100644 --- a/modules/backlogs/spec/support/pages/backlog.rb +++ b/modules/backlogs/spec/support/pages/backlog.rb @@ -29,6 +29,7 @@ #++ require "support/pages/page" +require "support/browsers/browser_platform" module Pages class Backlog < Page @@ -342,6 +343,31 @@ def work_package_card(work_package) find(work_package_card_selector(work_package)) end + # The row wrapper, not the card: it carries the item id and, once + # selected, `data-batch-selected`. The description lives on the card, + # which is the focus host, so it is checked through {#work_package_card}. + def work_package_row(work_package) + find(work_package_selector(work_package)) + end + + # The row's resolved background colour. Read it twice around one isolated + # change, never across two rows and never around a gesture that also + # focuses or navigates: the background depends on focus and `aria-current` + # too, so anything else makes the difference unattributable. + def row_background_color(work_package) + work_package_row(work_package).style("background-color").fetch("background-color") + end + + # Moves focus onto the card without a pointer and without any event a + # Backlogs controller listens for. Establishes focus before a + # background-colour control read, so a later keyboard gesture on the same + # card does not change it a second time. + def focus_work_package_card(work_package) + work_package_card(work_package).execute_script( + "this.focus({ focusVisible: true, preventScroll: true })" + ) + end + # Right-clicks near the card's top-left corner: the offset keeps the # pointer off the subject link and the actions menu button, both of which # keep their native context menu on purpose. @@ -385,6 +411,20 @@ def expect_work_package_card_focused(work_package) .to have_css(work_package_card_selector(work_package), focused: true) end + # The *computed* accessible description, not the raw `aria-describedby` + # token: an id that resolves to nothing, or an element the browser + # declines to traverse into, leaves this empty while the attribute reads + # exactly as intended. + def expect_work_package_card_described_as(work_package, description) + expect(page) + .to have_css(work_package_card_selector(work_package), accessible_description: description) + end + + def expect_work_package_card_not_described(work_package) + expect(page) + .to have_css(work_package_card_selector(work_package), accessible_description: "") + end + # The presenter takes the overlay's `anchor` idref away for the duration of # a contextual invocation, so its absence is what distinguishes a menu # opened at the pointer (or on the card) from one opened at the More @@ -536,12 +576,26 @@ def expect_no_sprint_menu_item(sprint, item_name) end end + # Opening details morphs the row into its "current work package" state, so + # a reference captured beforehand can go stale mid-morph. `retry_block` + # spaces the attempts out and finds the button, menu and view fresh on + # each one. def open_work_package_details(work_package) - within_work_package(work_package) do - button = find(:button, accessible_name: "Work package actions") - open_controlled_menu(button).find(:menuitem, text: I18n.t(:"js.button_open_details")).click + retry_block( + args: { + tries: 3, + on: [ + Capybara::Cuprite::ObsoleteNode, + Selenium::WebDriver::Error::StaleElementReferenceError + ] + } + ) do + within_work_package(work_package) do + button = find(:button, accessible_name: "Work package actions") + open_controlled_menu(button).find(:menuitem, text: I18n.t(:"js.button_open_details")).click + end + expect_details_view(work_package) end - expect_details_view(work_package) end def expect_details_view(work_package) @@ -606,9 +660,24 @@ def expect_and_confirm_backlog_bucket_delete_modal end end + # Every row carries the item id whether or not the user may move it, so + # the id no longer distinguishes an orderable row. `draggable` and + # `mobility` are set independently and read by separate consumers, so + # both are checked. + def expect_work_package_draggable(work_package) + selector = work_package_selector(work_package) + expect(page).to have_css("#{selector}[draggable='true']") + expect(page).to have_no_css("#{selector}[data-sortable-lists--item-mobility-value='fixed']") + end + + # Both assertions are negative, so they also pass against a page that + # never rendered the card — including the rack-session page a racing + # Selenium login can strand the browser on. Asserting the row exists first + # is blocked on fix/selenium-rack-session-login-flake. def expect_work_package_not_draggable(work_package) - expect(page) - .to have_no_css(draggable_work_package_selector(work_package)) + selector = work_package_selector(work_package) + expect(page).to have_no_css("#{selector}[draggable='true']") + expect(page).to have_no_css("#{selector}[data-sortable-lists--item-mobility-value='free']") end # A read-only card keeps its drag but is confined to its own list: it can @@ -616,8 +685,8 @@ def expect_work_package_not_draggable(work_package) # confined value is what the foreign drop targets read. def expect_work_package_confined(work_package) expect(page) - .to have_css("#{draggable_work_package_selector(work_package)}" \ - "[data-sortable-lists--item-confined-value='true']") + .to have_css("#{work_package_selector(work_package)}" \ + "[data-sortable-lists--item-mobility-value='confined']") expect(page) .to have_css("#{work_package_selector(work_package)}[draggable]") end @@ -637,6 +706,51 @@ def expect_work_package_not_locked(work_package) end end + # An unmodified click: narrows the batch to this card and opens its + # details pane. Offset near the top-left corner because the card's centre + # sits on the subject link or the actions menu button, both of which the + # selection root ignores as interactive descendants. + def select_card(work_package) + work_package_card(work_package).click(x: 6, y: 6, offset: :position) + end + + # Toggles membership without navigating, and re-bases the anchor to this + # card even when the toggle deselects it. + def toggle_card(work_package) + modified_click(work_package, multi_select_modifier) + end + + def multi_select_modifier + BrowserPlatform.multi_select_modifier(page) + end + + # Selects the contiguous range from the anchor to this card. Repeated + # calls resize one range rather than walking it. + def extend_selection_to(work_package) + modified_click(work_package, :shift) + end + + # Asserted directly because an empty batch proves nothing: a root that + # opted in and refused every gesture looks identical from outside, yet + # differs in whether the browser still gets the keystroke. + def expect_batch_selection_disabled + expect(page).to have_css("[data-controller~='sortable-lists'][data-sortable-lists-selection-enabled-value='false']", + visible: :all) + end + + # Live batch membership, in document order. + def selected_card_ids + all("[data-batch-selected]").pluck("data-sortable-lists--item-id-value") + end + + # The shared description every selected card's `aria-describedby` points + # at. Rendered once, permanently `hidden` — screen readers still reach it + # through the reference despite that — so `visible: :all` is required. + def expect_selection_description_present + expect(page).to have_css("##{Backlogs::SelectionDescriptionComponent::DESCRIPTION_ID}", + visible: :all, count: 1) + end + def pick_up_and_release_work_package(work_package) # A mid-drag list refresh can detach the grabbed row, so retry a bounded # number of times on a stale node. retry_block no-ops under @@ -652,7 +766,7 @@ def pick_up_and_release_work_package(work_package) ] } ) do - moved_element = find(draggable_work_package_selector(work_package)) + moved_element = find(work_package_selector(work_package)) install_backlogs_move_request_probe begin pick_up_and_release_backlogs_item(moved_element) @@ -756,7 +870,7 @@ def drag_work_package(moved, before: nil, after: nil, into: nil) raise ArgumentError, "You must specify exactly one of before, after or into" end - moved_element = find(draggable_work_package_selector(moved)) + moved_element = find(work_package_selector(moved)) target_element, edge = if before [find(work_package_selector(before)), :top] @@ -788,7 +902,7 @@ def drag_work_package_without_move(moved, into:) ] } ) do - moved_element = find(draggable_work_package_selector(moved)) + moved_element = find(work_package_selector(moved)) target_element = find(list_body_selector(sprint_selector(into))) install_backlogs_move_request_probe begin @@ -838,7 +952,7 @@ def expect_backlogs_drag_refused end def drag_work_package_to_backlog_inbox(work_package) - moved_element = find(draggable_work_package_selector(work_package)) + moved_element = find(work_package_selector(work_package)) inbox = find(backlog_inbox_selector) target_item = inbox.all("[data-sortable-lists--item-id-value]", minimum: 0).last target_element = target_item || inbox.find("[data-empty-list-item]") @@ -853,7 +967,7 @@ def drag_work_package_to_backlog_inbox(work_package) end def drag_work_package_to_backlog_bucket(work_package, bucket) - moved_element = find(draggable_work_package_selector(work_package)) + moved_element = find(work_package_selector(work_package)) target_element = find(list_body_selector(bucket_selector(bucket))) wait_for_backlogs_turbo_stream(frame_reload: true) do @@ -865,7 +979,7 @@ def drag_work_package_to_backlog_bucket(work_package, bucket) end def drag_work_package_to_sprint(work_package, sprint) - moved_element = find(draggable_work_package_selector(work_package)) + moved_element = find(work_package_selector(work_package)) target_element = find(list_body_selector(sprint_selector(sprint))) wait_for_backlogs_turbo_stream(frame_reload: true) do drag_backlogs_item(source: moved_element, target: target_element) @@ -946,6 +1060,15 @@ def within_work_package(work_package, &) private + # Node::Element#click takes the held key and positional options, so no + # action chain is needed. The offset avoids the card's centre, where the + # subject link or actions menu button sits: the selection root ignores a + # gesture starting on either, and which one lands dead centre varies with + # the card's content. + def modified_click(work_package, key) + work_package_card(work_package).click(key, x: 6, y: 6, offset: :position) + end + def within_sprint(sprint, &) within(sprint_selector(sprint), &) end @@ -1020,10 +1143,6 @@ def menu_owner_overlay_selector(menu_owner) "##{ActionView::RecordIdentifier.dom_target(menu_owner, :menu)}-overlay" end - def draggable_work_package_selector(work_package) - "#{work_package_selector(work_package)}[data-sortable-lists--item-id-value]" - end - # Located by the lock's accessible name so the expectation fails if the # icon ever loses the text that explains it. def readonly_lock_selector diff --git a/spec/support/browsers/browser_platform.rb b/spec/support/browsers/browser_platform.rb new file mode 100644 index 000000000000..cba8e2b0dd0e --- /dev/null +++ b/spec/support/browsers/browser_platform.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# The modifier a page under test honours for multi-select: Cmd on an Apple +# platform, Ctrl elsewhere. The browser under test decides, since a grid can +# run it on another OS than this host. +module BrowserPlatform + module_function + + def multi_select_modifier(session = Capybara.current_session) + apple?(session) ? :meta : :control + end + + def apple?(session = Capybara.current_session) + platform_name(session).match?(/mac|darwin|ios/i) + end + + # Selenium and Cuprite can both drive a browser on another machine; only + # the latter has no platform capability, but its user agent names the OS. + def platform_name(session) + driver = session.driver + reported = + case driver + when Capybara::Selenium::Driver then driver.browser.capabilities.platform_name + when Capybara::Cuprite::Driver then driver.browser.version.user_agent + end + + reported.presence || RbConfig::CONFIG["host_os"] + end +end diff --git a/spec/support/pages/page.rb b/spec/support/pages/page.rb index 06fe051d8ff1..2fa6fe024acd 100644 --- a/spec/support/pages/page.rb +++ b/spec/support/pages/page.rb @@ -53,7 +53,7 @@ def visit! visit(path) - wait_for_reload + wait_for_reload if using_cuprite? end def reload! diff --git a/spec/support_spec/browsers/browser_platform_spec.rb b/spec/support_spec/browsers/browser_platform_spec.rb new file mode 100644 index 000000000000..9a3ff86c5a21 --- /dev/null +++ b/spec/support_spec/browsers/browser_platform_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require "spec_helper" +require "support/browsers/browser_platform" + +RSpec.describe BrowserPlatform do + let(:app) { ->(_env) { [200, {}, []] } } + + # Real drivers, so `case` dispatches on the class; none of them starts a + # browser until `browser` is called, which is the one method stubbed. + def selenium_session(platform_name) + capabilities = instance_double(Selenium::WebDriver::Remote::Capabilities, platform_name:) + driver = Capybara::Selenium::Driver.new(app) + allow(driver).to receive(:browser).and_return(instance_double(Selenium::WebDriver::Driver, capabilities:)) + instance_double(Capybara::Session, driver:) + end + + def cuprite_session(user_agent) + version = instance_double(Ferrum::Browser::VersionInfo, user_agent:) + driver = Capybara::Cuprite::Driver.new(app) + allow(driver).to receive(:browser).and_return(instance_double(Capybara::Cuprite::Browser, version:)) + instance_double(Capybara::Session, driver:) + end + + def host_os(value) + stub_const("RbConfig::CONFIG", RbConfig::CONFIG.merge("host_os" => value)) + end + + describe ".multi_select_modifier" do + it "is Cmd when a Selenium browser runs on macOS, whatever the host" do + host_os("linux-gnu") + + expect(described_class.multi_select_modifier(selenium_session("mac"))).to eq(:meta) + end + + it "is Ctrl when a Selenium browser runs elsewhere, whatever the host" do + host_os("darwin24") + + expect(described_class.multi_select_modifier(selenium_session("linux"))).to eq(:control) + end + + it "reads the platform from a Cuprite browser's user agent" do + host_os("darwin24") + linux_ua = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/140.0 Safari/537.36" + + expect(described_class.multi_select_modifier(cuprite_session(linux_ua))).to eq(:control) + end + + it "falls back to the host when the browser reports no platform" do + host_os("darwin24") + + expect(described_class.multi_select_modifier(selenium_session(nil))).to eq(:meta) + end + + it "falls back to the host for a RackTest driver" do + host_os("linux-gnu") + session = instance_double(Capybara::Session, driver: Capybara::RackTest::Driver.new(app)) + + expect(described_class.multi_select_modifier(session)).to eq(:control) + end + end +end