[AGILE-361] Add batch selection to Backlogs cards - #24525
Conversation
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
There was a problem hiding this comment.
Pull request overview
This PR introduces opt-in batch selection to Backlogs’ sortable lists, including keyboard/mouse interactions, persistent selection count UI, and consistent selection/current-work-package styling across Turbo morphs.
Changes:
- Add a framework-agnostic
BatchSelectionmodel plus a DOM adapter to drive selection behavior in the sharedsortable-listsStimulus controller. - Render a persistent Backlogs selection count component (and shared “selected” description element) and wire Backlogs’ root to enable selection + consumer-specific announcements.
- Update Backlogs item/card DOM contract and styling: movable vs non-movable items, focus target, and selection/current markers preserved across morphs.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/backlogs/spec/support/pages/backlog.rb | Extends Backlogs page object with batch selection helpers and updated drag expectations. |
| modules/backlogs/spec/requests/backlogs/backlog_spec.rb | Adds request spec to ensure selection UI/description is rendered even when inbox is filtered out. |
| modules/backlogs/spec/features/work_packages/batch_selection_spec.rb | New Selenium feature spec covering mouse + keyboard batch selection behavior and accessibility wiring. |
| modules/backlogs/spec/components/backlogs/work_package_card_list_item_component_spec.rb | Updates component specs for movability flag, focus target, and row/card attributes. |
| modules/backlogs/spec/components/backlogs/work_package_card_list_component_spec.rb | Adjusts expectations for updated item target tokens. |
| modules/backlogs/spec/components/backlogs/sprint_component_spec.rb | Aligns sprint rendering expectations with “sortable item always, movable maybe” contract. |
| modules/backlogs/spec/components/backlogs/selection_count_component_spec.rb | New component spec for persistent selection count + shared description rendering. |
| modules/backlogs/spec/components/backlogs/bucket_component_spec.rb | Aligns bucket rendering expectations with updated item contract. |
| modules/backlogs/config/locales/js-en.yml | Adds Backlogs-specific selection announcement/count strings. |
| modules/backlogs/app/views/backlogs/backlog/show.html.erb | Enables selection on Backlogs root and configures announcement scope + description id. |
| modules/backlogs/app/views/backlogs/backlog/_backlog_list.html.erb | Renders selection count component above both planning columns. |
| modules/backlogs/app/components/backlogs/work_package_card_list_item_component.rb | Makes every row a sortable item, adds movable value, and adds a focus item target on the card. |
| modules/backlogs/app/components/backlogs/selection_count_component.sass | Styles persistent selection count and hides it via visibility when empty. |
| modules/backlogs/app/components/backlogs/selection_count_component.rb | Introduces SelectionCount component + shared description id constant. |
| modules/backlogs/app/components/backlogs/selection_count_component.html.erb | Renders persistent count region and shared description element. |
| modules/backlogs/app/components/_index.sass | Registers selection count styles in Backlogs components bundle. |
| frontend/src/turbo/pragmatic-dnd-morph-attributes.ts | Preserves batch selection marker across Turbo morphs to avoid visual flashing. |
| frontend/src/turbo/pragmatic-dnd-morph-attributes.spec.ts | Updates morph preservation test for new selection marker. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/selection.ts | Adds selection DOM adapter: candidate resolution, range resolution, focus navigation helpers, and presentation wiring. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/selection.spec.ts | Unit tests for selection adapter behavior. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts | Updates fake root interface in tests for new selection capabilities. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.ts | Removes legacy split-view data-selected stripping; documents batch selection attribute placement. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/preview.spec.ts | Updates preview sanitization tests accordingly. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts | Updates fake root interface in tests for new selection capabilities. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts | Adds movable attribute contract and isMovableItem helper. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts | Adds movable value, focus target support, and collapses selection on drag start. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts | Adds coverage for movability gating + focus behavior + drag-start selection collapse. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts | Extends root interface with selectionEnabled and collapseSelectionForDrag. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts | Implements opt-in batch selection interactions, announcements, morph reconciliation, and selection count rendering. |
| frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts | Extensive new test coverage for selection interactions, announcements, and morph reconciliation. |
| frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.ts | Removes legacy data-selected handling; keeps only aria-current syncing to URL. |
| frontend/src/stimulus/controllers/dynamic/backlogs/work-package.controller.spec.ts | Updates tests for current-work-package behavior and ensures batch membership is unaffected by URL sync. |
| frontend/src/global_styles/content/modules/_backlogs.sass | Adjusts Backlogs layout to accommodate persistent selection count above scroll columns. |
| frontend/src/common/batch-selection.ts | Adds framework-agnostic batch selection model (ids + anchor + prune). |
| frontend/src/common/batch-selection.spec.ts | Unit tests for batch selection model semantics. |
| frontend/AGENTS.md | Documents sortable-lists batch selection opt-in, vocabulary, and marker placement. |
| config/locales/js-en.yml | Adds default sortable-lists selection announcement strings (generic “item” vocabulary). |
| app/components/open_project/common/border_box_list_component.sass | Updates styling selectors from legacy data-selected to new data-batch-selected + aria-current separation. |
09c5889 to
8a57b46
Compare
Deploying openproject with ⚡ PullPreview
|
8a57b46 to
5273f41
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments.
Suppressed comments (2)
modules/backlogs/app/components/backlogs/selection_count_component.html.erb:55
hidden: truerenders a literal HTMLhiddenattribute (i.e.,display: none), which commonly removes the element from the accessibility tree. Because this span is referenced viaaria-describedby, it should be visually hidden (e.g.,sr-only) rather thanhidden, otherwise some screen readers may ignore the referenced description.
<%= render(Primer::Box.new(tag: :span, id: DESCRIPTION_ID, hidden: true)) do %>
<%= I18n.t("js.backlogs.selection.card_state") %>
<% end %>
modules/backlogs/spec/support/pages/backlog.rb:764
- This comment says the description is permanently
hiddenand still reachable viaaria-describedby. With the HTMLhiddenattribute that isn’t reliably true; the description should be treated as visually hidden (e.g.,sr-only) so assistive tech can still reference it. Updating this wording will prevent future changes from reintroducinghiddenand breaking the a11y contract.
2a7bbac to
f3da126
Compare
f3da126 to
63d626a
Compare
63d626a to
6e3c26e
Compare
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
cf99663 to
5cbd3ae
Compare
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
9065e03 to
3e98867
Compare
dbcbe0d to
41a3461
Compare
41a3461 to
9fcbd5f
Compare
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
Adds a framework-agnostic anchor/range/toggle model over opaque (type, id) identities, so card and table views can share one selection policy without inheriting any view's concepts. https://community.openproject.org/wp/AGILE-361
Renders every card as a sortable item and replaces presence-based movability with a mobility value (free, confined or fixed), so a card the user cannot move still anchors drops and position counting. An unrecognised value falls closed to fixed. https://community.openproject.org/wp/AGILE-361
Adds the DOM-facing selection adapter and an orchestrator turning pointer and keyboard gestures into model changes and announcements. Ranges and Ctrl/Cmd+A are confined to the focused card's list; Escape clears the selection from anywhere on the page. https://community.openproject.org/wp/AGILE-361
Attaches selection opt-in to the sortable root: capture-phase listeners beat the card's own click handler, a drag collapses the batch onto the dragged card, and membership survives Turbo morphs and cache restores. https://community.openproject.org/wp/AGILE-361
Opts the sprint planning page in, speaks "work package" through the announcement scope, and describes membership through one shared hidden element every selected card references via aria-describedby. https://community.openproject.org/wp/AGILE-361
Adds a Selenium feature spec for what units cannot prove: the capture-phase listeners really beat the card's own click and Enter handlers in a real page. https://community.openproject.org/wp/AGILE-361
9fcbd5f to
7089eaa
Compare
Ticket
https://community.openproject.org/wp/AGILE-361
What are you trying to accomplish?
Backlogs cards can only be acted on one at a time. This adds batch selection to the sprint planning page, so a user can build a selection of cards with the mouse or the keyboard, as a prerequisite for acting on several at once.
Mouse: a plain click selects just that card (and still opens its details pane, as before). Ctrl/Cmd+click toggles one card in or out without navigating. Shift+click selects a contiguous range from a fixed anchor, so repeated Shift+clicks resize one range rather than walking it.
Keyboard: Space toggles the focused card. Arrow keys move focus, and with Shift extend the range. Home/End jump to the first/last movable card in the list. Ctrl/Cmd+A selects every loaded movable card in the focused card's list. Escape clears the selection wherever focus sits — on a card, on a link inside one, or nowhere at all. Enter is left alone — it belongs to the card's own activation handler.
A range is confined to a single list, because a range that spanned the backlog and sprint columns has no meaningful order — and select-all is confined the same way, so "everything" means everything in the list the user is looking at. A selection spanning lists is still reachable: Ctrl/Cmd+click is not list-confined, and a keyboard user can Tab to a card in the other column and toggle it with Space. A one-gesture cross-list select-all is deliberately left to a separate, future mechanism.
Batch movement is not part of this PR. A drag still moves exactly one card, collapsing any wider selection onto the card being dragged rather than implying the rest came along. Acting on a batch is AGILE-278, which stacks on this branch.
Accessibility
Selection changes are announced through the shared Primer live region, in the consumer's own vocabulary ("work package", not "item"). Every selected card points at one shared, permanently hidden description element via
aria-describedby, so batch membership reaches a screen reader per card without duplicating a string onto every card. Gestures that change nothing visible — a range refused because it crosses lists, or because it contains a card the user may not move — get their own message rather than silently doing nothing.There is no visible on-screen count. An earlier revision had one; see below.
What approach did you choose and why?
The selection policy is framework-agnostic and lives outside Stimulus.
frontend/src/common/batch-selection.tsknows nothing about the DOM, Stimulus, or work packages — it is anchor/range/toggle logic over opaque ids. wp-fast-table and the Angular card view are the intended next consumers, and keeping the model free of this page's concepts is what makes that possible without a rewrite. The DOM-facing half is a separate adapter,sortable-lists/selection.ts, so neither the model nor the root controller grows a second job.Selection is opt-in per root. A root enables it with
selectionEnabled; every othersortable-listsconsumer is unchanged by being upgraded. The announcement scope and the shared description id are values too, so the shared controller speaks the consumer's vocabulary while staying free of Backlogs terminology itself.The pointer listener runs in the capture phase at the root — but Escape listens at the document, in the bubble phase. Capture is what lets a modified click be consumed before the card's own navigation handler sees it, without depending on the order two independent controllers happen to connect in. An ordinary click is deliberately let through. Escape is the deliberate exception: clearing the selection must not depend on focus sitting on a card row, and bubbling lets a dialog's or menu's own Escape run first and be respected.
Movability became a property of the item rather than a reason to withhold the controller. Previously a card the user could not move rendered without the sortable item controller at all, which took it out of the list's addressable order: it could not anchor a neighbour's drop and was invisible to position counting. Every card row is now a sortable item, with
mobilityas a value on it.Two things worth a reviewer's attention
No visible selection count. An earlier revision had one. It reserved a line of layout space above both planning columns, which shifted every card beneath it and broke positional drags against the unchanged
drag_in_bucket_spec. Design had not settled on needing it, so it went rather than earning a layout workaround.hiddenon the shared description is deliberate. An element referenced directly byaria-describedbyis still traversed for the accessible description (accname §4.1 step 2A). The feature spec asserts the computedaccessible_description, so this is verified rather than assumed — please don't "fix" it tosr-only, which would put it back in the layout and reintroduce that drag regression.Reviewing this
The 7 commits are ordered to be read in sequence: the framework-free model first, then item mobility, the selection adapter and gestures, the root wiring, the Backlogs adoption, the end-to-end spec, and the docs. Review feedback is folded into the commits it belongs to rather than appended as fixups.
Merge checklist
Test coverage: unit specs for the selection model, the DOM adapter, and each gesture against a synthetic root; component and request specs for the Backlogs wiring; and a Selenium feature spec for the one thing units cannot prove — that the capture-phase listener really does beat the card's own click and Enter handlers in a real page.
Lookbook is unticked because this adds no new visual component: the only component left is a permanently hidden description element, which has nothing to preview.
Browsers are unticked pending manual QA. Alongside that, the design checks still outstanding are the card states, narrow-width layout, a screen-reader pass, and drag-with-nothing-selected.