[AGILE-361] Add batch selection to Backlogs cards - #24525
Draft
myabc wants to merge 39 commits into
Draft
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. |
myabc
commented
Jul 30, 2026
Contributor
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. |
myabc
force-pushed
the
implementation/AGILE-361-batch-selection
branch
from
August 5, 2026 12:11
09c5889 to
8a57b46
Compare
Deploying openproject with ⚡ PullPreview
|
myabc
changed the title
myabc
force-pushed
the
implementation/AGILE-361-batch-selection
branch
from
August 5, 2026 14:30
8a57b46 to
5273f41
Compare
Contributor
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.
myabc
commented
Aug 5, 2026
myabc
commented
Aug 5, 2026
myabc
force-pushed
the
implementation/AGILE-361-batch-selection
branch
2 times, most recently
from
August 5, 2026 16:08
2a7bbac to
f3da126
Compare
This was referenced Aug 5, 2026
Replaces the drag path's closest('li') source-row lookup and the item's
parentElement assumption with one invariant: an item's owning list is
the innermost list outlet containing it. Rows now work as any element,
which admin BorderBox rows and table rows require.
https://community.openproject.org/wp/DREAM-786
Adds a type-to-template map on the root so one root can host item types with different move endpoints, falling back to the single template. Menu moves read the type off the item element, so map-only roots serve the directional path too. https://community.openproject.org/wp/DREAM-786
The optimistic=true query flag suppresses the server's frame reload and belongs to Backlogs' event-only success contract. Admin surfaces reconcile through morph streams instead, so the root now sends the flag only when a consumer declares it; Backlogs opts in. https://community.openproject.org/wp/DREAM-786
Sortable-list moves arrive as a prev_id anchor rather than an absolute position. Adds the shared move-after-anchor primitive with strict validation: blank means top, and an unknown, out-of-scope, or self anchor is rejected without mutation. https://community.openproject.org/wp/DREAM-786
Custom fields inside a section are ordered by the attribute_order array rather than acts_as_list. Adds the anchor-based counterpart to add_to_order with the same strict semantics: blank anchors the head, an unresolvable anchor is rejected without mutation. https://community.openproject.org/wp/DREAM-786
Accepts the sortable-lists list_id/prev_id wire alongside the legacy target_id/position wire, which the user custom fields surface still sends until its own migration. Anchor moves validate before mutating and lock both section rows in id order against concurrent reorders. Reuses the single locked section object for the same-row case so every read and write goes through it, closing a lost-update window where a fresh, unlocked lookup of the target section could be read from and written to before the lock took effect. Runs the anchor move in a savepoint transaction so its rollback guard undoes a partially applied cross-section reparent even when nested inside a caller's own transaction. https://community.openproject.org/wp/DREAM-786
Converts both project-attribute drop endpoints from absolute positions to the sortable-lists list_type/list_id/prev_id wire with a strict contract: exact list type, list id only where a section is addressed, and a required scalar prev_id filtered through permit, so a collection id cannot reach the anchor lookup as an IN match. Violations and rejected anchors answer 422 without mutation. https://community.openproject.org/wp/DREAM-786
Listening in the capture phase at the root is what lets a modified click be consumed before the card's navigation handler runs, without depending on the order two independent controllers happen to connect in. The walk deciding whether a click landed on an interactive descendant stops at the focus host, which a Backlogs card nests inside its row rather than being. An ordinary click is deliberately let through: it opens the details pane as it always has, and collapses the batch either way — a card that cannot join one still clears what was selected, or the count would linger over a pane the user just opened. https://community.openproject.org/wp/AGILE-361
The bindings follow the conventional multi-selection model rather than the table view's Ctrl+D, so what a user learns here transfers. Space is claimed for selection, which forecloses it for any later keyboard grab-and-drop design; movement stays reachable through the card menu. Select-all reaches every loaded movable card rather than one list, so keyboard users are never stranded away from cards in another column, while a range stays confined to a single list. Every recognised key is consumed even where it cannot act, and resolved before the in-flight move bail, since Escape must still clear a mistaken selection. https://community.openproject.org/wp/AGILE-361
Batch movement is not implemented yet, so a drag moves exactly one card: a wider selection collapses onto the dragged one rather than implying the rest came along. An empty selection stays empty, since collapsing a batch is not the same as forming one. https://community.openproject.org/wp/AGILE-361
The Backlogs frame refreshes by morphing, so a selection formed before a refresh has to survive one and shed whatever the refresh took away. The model is the authority: a marker the morph preserved but the model no longer holds is cleared rather than trusted. https://community.openproject.org/wp/AGILE-361
Plain-click collapse, drag collapse and morph pruning all changed how many cards were selected without telling assistive technology, leaving a screen-reader user believing a larger batch was still selected. Centralising the decision in renderSelection means a later call site cannot forget, and an unchanged count stays silent. A Shift gesture has no other feedback to fall back on, unlike a click that opens a pane, so the two cases that leave the count unchanged speak for themselves: a range collapsed to one card, and a range refused by a card the user may not move, which the expand-the-list wording could never help with. https://community.openproject.org/wp/AGILE-361
Omitting the controller from cards the user cannot move took them out of the list's addressable order altogether, so they could not anchor a neighbour's drop and were invisible to position counting. Movability becomes a value on the item rather than a reason to withhold it, which also retires the page object's equation of item id with draggability. https://community.openproject.org/wp/AGILE-361
Two attributes both named for selection meant two different things: an optimistic marker for the card being opened, and nothing at all for a batch. The optimistic marker goes, since a plain click now forms a one-card batch that supplies the same immediate feedback, and the current work package is what navigation says it is. The batch marker is written on the row while aria-current stays on the card inside it, so the stylesheet matches the row: a selector aimed at the nested card paints nothing while attribute assertions stay green. https://community.openproject.org/wp/AGILE-361
Selection stays opt-in so no other sortable consumer changes behaviour by being upgraded. The translation scope is what lets the shared controller say "work package" here while remaining free of any Backlogs vocabulary itself. The count renders once above both planning columns rather than inside either: they scroll independently, so an anchored count showed up on the wrong side and scrolled away. It holds its space unconditionally, since appearing at the second card would otherwise jump the columns under the cursor mid-selection. https://community.openproject.org/wp/AGILE-361
The unit specs prove each gesture in isolation. What they cannot prove is whether the capture-phase listener beats the card's own navigation handler in a real page, the interaction most likely to regress. https://community.openproject.org/wp/AGILE-361
Sortable-lists opt-in batch selection now has a home in AGENTS.md, including the item/card attribute split and the fact that batch movement is not yet implemented, so the next agent building on it does not have to rediscover either from the controller source. https://community.openproject.org/wp/AGILE-361
Reserving a line above both planning columns shifted every card beneath it, which moved drop geometry enough to break positional drags: the unchanged drag_in_bucket spec failed on reordering to a bucket's first position and on dragging out to the inbox. Design had not settled on needing an on-screen count, so it goes rather than earning a workaround. The shared description stays, renamed with its component: permanently hidden, so it costs no layout, and it is what tells a screen-reader user which cards are in the batch. Counts still announce through the live region. https://community.openproject.org/wp/AGILE-361
Selection resolved rows through the unbounded helper, which descends into nested lists: a section row answered with the first field of the list inside it. The rows container now bounds every lookup. https://community.openproject.org/wp/AGILE-361
The movable and confined booleans could express an item that takes no part in ordering yet is pinned to its list, which means nothing. One union makes that unrepresentable, and drops the collision between the JS movable value and Ruby's unrelated movable? predicate. Missing and malformed now differ: absent stays free for consumers that render none, while an unrecognised value falls closed to fixed rather than offering controls the server will refuse. https://community.openproject.org/wp/AGILE-361
Every card is fixed without manage_sprint_items, but the root opted in regardless, so a viewer lost Space, the arrows, Home/End and Ctrl/Cmd+A to a capability the page never offered them. The predicate takes its project explicitly: the sibling predicates run in components, which expose one as a method, while this runs in the backlog view, which does not. https://community.openproject.org/wp/AGILE-361
The root had grown to own dragging, moves, gesture interpretation, keyboard navigation, announcements, focus and presentation at once. Selection moves behind a narrow host port, so it is testable without a Stimulus application and a root that never opted in constructs none of it — the listeners go up with the orchestrator rather than attaching and returning early. SortableListsRoot drops selectionEnabled with it: no child ever read the flag, and selection is no longer root state to branch on. https://community.openproject.org/wp/AGILE-361
A snapshot taken mid-selection restored rows that looked selected to a fresh orchestrator holding nothing, so Escape did nothing and a Shift gesture ranged from no anchor. Cleaning up in disconnect cannot fix this: Turbo clones the snapshot before the visit replaces the body, and that replacement is what disconnects the controller. https://community.openproject.org/wp/AGILE-361
A click during an in-flight move fell through to the card's own delay and opened the details pane behind a stale batch. A menu move left a wider batch highlighted while relocating one card, so the two move surfaces disagreed about what a single-card move means. The collapse runs last, after every resolution step: an unavailable direction must not cost the user their selection for nothing. https://community.openproject.org/wp/AGILE-361
Gating on the count alone silenced a Shift gesture that swapped a range for a different set of the same size, which is the one gesture with no other feedback at all. Call sites now state which kind of gesture they are, and the rule reads that rather than a number. The baseline is the last rendered membership, not the last announced one: a silent navigation render in between would otherwise leave a stale baseline and make the next genuine no-op speak. https://community.openproject.org/wp/AGILE-361
Ctrl/Cmd+A was a black hole with nothing selectable: the key was consumed before anything was resolved, so the browser's own select-all was blocked and nothing was announced in its place. Held Space toggled a card repeatedly. Shift+Arrow was the one entry point with no movability check, so it could paint a card every other gesture refuses. Ctrl-click now falls through untouched on Apple platforms, where it is the secondary click that opens the card's menu. Classifying it as unmodified would not do: that path replaces the batch outright. https://community.openproject.org/wp/AGILE-361
The anchor's list key is stamped when the anchor is set, which for a drag is drag start, so a card dropped into another list left the key naming the list it came from. The next Shift gesture there read as cross-list and restarted the range instead of extending it. Reconciliation re-derives the key from the live document, after prune so it only ever sees an anchor that still exists. https://community.openproject.org/wp/AGILE-361
myabc
force-pushed
the
implementation/AGILE-361-batch-selection
branch
from
August 6, 2026 08:19
63d626a to
6e3c26e
Compare
turbo:before-cache fires for every visit, including the details-pane navigation that morphs the Backlogs page in place. Clearing there stripped the highlight from a live page whose controller never went away, which is what AC-7 forbids and what two feature specs caught. A restored page brings back markup but not the model, so connect time is where stale presentation can be recognised as stale. The page object also still asserted the old confined value. https://community.openproject.org/wp/AGILE-361
Ids are unique per source table, not per root, so a nested list can hold an unrelated item with a colliding id: selecting one painted the other, and the anchor rebind could follow the wrong one entirely. An item declaring no type is now refused rather than sharing an empty namespace with every other consumer. A batch holds one item type, enforced wherever a candidate can join rather than on ranges alone, so Ctrl/Cmd-click and Space cannot build the mixed batch the rule forbids. That cohort policy lives in the orchestrator: the model owns identity, not what an action means. An item also belongs to its nearest ancestor root, so an independently nested root is an ownership boundary. https://community.openproject.org/wp/AGILE-361
The refusal messages still said a card could not be moved, which names a different property than the one being refused: a confined card moves within its list perfectly well and is selectable. Ranges now take the rows container from the host rather than deriving it, so what counts as a list's rows is defined once and shared with moves instead of drifting in a second copy. https://community.openproject.org/wp/AGILE-361
Selection consumes Space, the arrows, Home/End and Ctrl/Cmd+A, so a page that offers no selection must not attach anything that swallows them. Nothing covered that, which made the permission gate the least tested of the fixes and the one that reaches the most users. The context re-visits after logging in: `current_user` declared inside a describe registers its login after the outer visit hook, so the first navigation lands before the user exists and the browser sits on the rack-session page. https://community.openproject.org/wp/AGILE-361
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 across the whole page. Escape clears the selection. 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. Select-all deliberately is not, so keyboard users are never stranded away from cards in the other column.
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. That 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.
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
movableas 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 14 commits are ordered to be read in sequence and each one builds and passes on its own: the framework-free model first, then item movability, the DOM adapter, each gesture, announcements, morph durability, then the Backlogs adoption, and finally the end-to-end spec. No commit introduces something a later one retracts.
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, drag-with-nothing-selected, and Escape during an in-flight move.