Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions app/components/open_project/common/border_box_list_component.sass
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@
&[data-batch-selected]
border-top-color: var(--box-list-item-selected-border-color)

&:has(> .Box-card[aria-current="true"])
&:has(> .Box-card[aria-current="true"]),
&:has(> .Box-card[data-activating])
border-top-color: var(--box-list-item-pressed-border-color)

&.op-border-box-list_transparent
Expand Down Expand Up @@ -208,7 +209,13 @@
// 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 batch selection.
.Box-row:has(> .Box-card[aria-current="true"])
//
// data-activating is that controller's synchronous pressed state — the card
// was just clicked or Enter-activated and its visit has not landed yet. It
// is visual only (no ARIA) and reuses the pressed border so it hands over
// seamlessly to aria-current once the URL reflects the details pane.
.Box-row:has(> .Box-card[aria-current="true"]),
.Box-row:has(> .Box-card[data-activating])
@include op-box-list-item-edge-borders(var(--box-list-item-pressed-border-color))

// Drag-and-drop row states, scoped to the border-box list. The global
Expand Down Expand Up @@ -306,6 +313,26 @@
.-browser-firefox &
box-shadow: none

// The stack and the lift share the one box-shadow property, so both are
// declared here or one replaces the other.
//
// The depth the preview writes is the batch's, uncapped: past four cards the
// added depth stops reading, so this default holds every deeper batch at the
// maximum and the shallow depths below override it.
&[data-preview][data-stack-depth]
box-shadow: op-drag-stack-shadows(), var(--shadow-floating-medium)

// Blur-free and inside the reserved overhang, so only the lift has to go.
.-browser-firefox &
box-shadow: op-drag-stack-shadows()

@for $layers from 1 through $op-drag-stack-max-layers - 1
&[data-preview][data-stack-depth="#{$layers}"]
box-shadow: op-drag-stack-shadows($layers: $layers), var(--shadow-floating-medium)

.-browser-firefox &
box-shadow: op-drag-stack-shadows($layers: $layers)

.Box--condensed .Box-card
padding: var(--stack-padding-condensed) var(--stack-padding-normal)

Expand Down
12 changes: 12 additions & 0 deletions config/locales/js-en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -861,11 +861,23 @@ en:

sortable_lists:
announcements:
batch_too_large: "Cannot move %{count} items at once. Select no more than %{max}."
fallback_item_label: "Item"
fallback_list_name: "another list"
move_failed_check_position: "Move failed. Check the item's current position."
# The batch keys are plural hashes so translators can add the plural
# categories their locale needs; the one: branch is unreachable (a
# one-item move announces through the singular keys).
move_failed_check_positions_batch:
other: "Move failed. Check the items' current positions."
move_failed_rolled_back: "Move failed. %{label} returned to its previous position."
move_failed_rolled_back_batch:
other: "Move failed. %{count} items returned to their previous positions."
moved: "%{label} moved to position %{position} of %{total}"
moved_batch:
other: "%{count} items moved to positions %{first} through %{last} of %{total}"
moved_batch_to_list:
other: "%{count} items moved to %{list}, positions %{first} through %{last} of %{total}"
moved_to_list: "%{label} moved to %{list}, position %{position} of %{total}"
selection:
cleared: "Selection cleared."
Expand Down
2 changes: 1 addition & 1 deletion frontend/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +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` batch selection is opt-in: a root enables it with a `selectionEnabled` value, and no other consumer's behavior changes. A root also sets `announcementScope`, so the shared controller's announcements speak the consumer's vocabulary instead of "item", and `selectionDescriptionId`, pointing at one shared element every selected card references via `aria-describedby`. Items declare `mobility` — `fixed`, `confined` or `free` — which gates dragging, selection eligibility, and positional moves alike. A missing value means `free`, so a consumer that renders none keeps working; an unrecognised one falls closed to `fixed` rather than handing the user controls the server will refuse. The pure selection model lives in `./src/common/batch-selection.ts` (framework-agnostic, so Angular consumers can adopt it); the DOM-facing adapter is `sortable-lists/selection.ts`, and gesture interpretation sits behind `sortable-lists/selection-orchestrator.ts`, which takes a narrow host port and imports no Stimulus. Selection identity is `(type, id)`, never the id alone: ids are unique per source table, so a nested list of another type can hold a colliding one. A root must render exactly one instance of each `(type, id)`, and an item declaring no type is refused as a candidate. A batch holds one item type — that cohort rule is orchestrator policy, not a constraint of the model, since identity namespacing and batch compatibility are different concerns. Ranges and select-all (Ctrl/Cmd+A) are both confined to the focused card's list; selecting across lists is a deliberate gap, reserved for a separate mechanism. An item belongs to its nearest ancestor root, so an independently nested root is an ownership boundary. Batch movement is not implemented: a drag still moves one card and collapses any wider selection onto it — that's a later work package.
- `sortable-lists` batch selection is opt-in: a root enables it with a `selectionEnabled` value, and no other consumer's behavior changes. A root also sets `announcementScope`, so the shared controller's announcements speak the consumer's vocabulary instead of "item", and `selectionDescriptionId`, pointing at one shared element every selected card references via `aria-describedby`. Items declare `mobility` — `fixed`, `confined` or `free` — which gates dragging, selection eligibility, and positional moves alike. A missing value means `free`, so a consumer that renders none keeps working; an unrecognised one falls closed to `fixed` rather than handing the user controls the server will refuse. The pure selection model lives in `./src/common/batch-selection.ts` (framework-agnostic, so Angular consumers can adopt it); the DOM-facing adapter is `sortable-lists/selection.ts`, and gesture interpretation sits behind `sortable-lists/selection-orchestrator.ts`, which takes a narrow host port and imports no Stimulus. Selection identity is `(type, id)`, never the id alone: ids are unique per source table, so a nested list of another type can hold a colliding one. A root must render exactly one instance of each `(type, id)`, and an item declaring no type is refused as a candidate. A batch holds one item type — that cohort rule is orchestrator policy, not a constraint of the model, since identity namespacing and batch compatibility are different concerns. Ranges and select-all (Ctrl/Cmd+A) are both confined to the focused card's list; selecting across lists is a deliberate gap, reserved for a separate mechanism. An item belongs to its nearest ancestor root, so an independently nested root is an ownership boundary. Dragging a selected card moves the whole batch: the root freezes the drag's batch in the preview callback (`freezeDragBatch`) and marks its rows at drag start (`markDragBatch`), and a selection-enabled root with a `collectionMoveUrl` value submits ordered `ids[]` to the collection move action — for one dragged card or many. Dragging an unselected card selects it, collapsing any wider selection. A root's `moveAnnouncementScope` value keys the move announcements the same way `announcementScope` keys the selection ones.
- `data-batch-selected` is written on the sortable item element — the row, in Backlogs — while `aria-current` is written on the card inside it. A stylesheet assuming both live on the same element will silently paint nothing while attribute assertions stay green.

## Configuration Files
Expand Down
64 changes: 64 additions & 0 deletions frontend/src/global_styles/content/drag_and_drop.sass
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
//
// See COPYRIGHT and LICENSE files for more details.
//++
@use "sass:list"

// The intend for this file is to become the place where all drag&drop styles are placed.
// As there will hopefully also be a shared style for resizing, and as those two will hopefully also share some styles
Expand All @@ -45,3 +46,66 @@

&:active
cursor: grabbing

// Ghost layers behind a multi-card drag preview, one per further card in the
// batch up to the maximum. Composed into the card's own shadow by the
// component that renders it (border_box_list_component.sass).
$op-drag-stack-max-layers: 3 !default
$op-drag-stack-step: 6px !default
$op-drag-stack-ring: 1px !default
$op-drag-stack-shade-blur: 4px !default
$op-drag-stack-shade-color: rgba(37, 41, 46, 0.18) !default

$op-drag-badge-overhang: 8px !default
$op-drag-stack-overhang: $op-drag-stack-step * $op-drag-stack-max-layers + $op-drag-stack-shade-blur

// renderDragPreview reserves the overhangs as container padding, and can only
// do so inline (Pragmatic inline-resets the container first), so they are
// published as custom properties rather than duplicated as numbers there.
:root
--op-drag-badge-overhang: #{$op-drag-badge-overhang}
--op-drag-stack-overhang: #{$op-drag-stack-overhang}

// Each depth emits shade, fill then ring: CSS paints a shadow list front to
// back, so a depth's shade lands above its own fill, in the gap under the card
// in front of it. The deepest shade reaches `step * layers + shade-blur` past
// the right and bottom edges; the blur is under every offset, so nothing
// reaches past the top or left.
@function op-drag-stack-shadows($layers: $op-drag-stack-max-layers, $step: $op-drag-stack-step, $ring: $op-drag-stack-ring, $blur: $op-drag-stack-shade-blur, $shade: $op-drag-stack-shade-color)
$shadows: ()

@for $depth from 1 through $layers
$offset: $step * $depth
$shadows: list.append($shadows, $offset $offset $blur 0 $shade, comma)
$shadows: list.append($shadows, $offset $offset 0 0 var(--bgColor-default), comma)
$shadows: list.append($shadows, $offset $offset 0 $ring var(--borderColor-default), comma)

@return $shadows

// Multi-card batch count badge on a drag preview, on Primer's Counter
// contract plus the positioning Counter does not own and the accent skin the
// drop indicator uses.
//
// Paint past the container's border box lands in Firefox's drag snapshot and
// shifts its origin off the grab offset, so the badge sits in the padding
// renderDragPreview writes. The right offset keeps it on the card's corner.
.op-sortable-lists-drag-preview-batch-badge
position: absolute
top: 0
right: $op-drag-stack-overhang - $op-drag-badge-overhang
min-width: 20px
height: 20px
padding: 0 6px
border-radius: 999px
background-color: var(--bgColor-accent-emphasis)
color: var(--fgColor-onEmphasis)
font-size: 12px
font-weight: 600
line-height: 20px
text-align: center
box-shadow: var(--shadow-floating-medium)

// The blur past the container's border box would shift Firefox's snapshot
// origin like the overhang above.
.-browser-firefox &
box-shadow: none
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,39 @@ describe('Backlogs split-view-sync controller', () => {

expect(refresh).not.toHaveBeenCalled();
});

it('refreshes every cached member of a batch event, in order', async () => {
await renderHost();

dispatchMoved({ work_package_ids: [11, 12, 13] });

await waitFor(() => {
expect(id).toHaveBeenCalledWith('11');
expect(id).toHaveBeenCalledWith('12');
expect(id).toHaveBeenCalledWith('13');
expect(refresh).toHaveBeenCalledTimes(3);
});
});

it('skips uncached members of a batch event', async () => {
hasValue.mockImplementation(() => state.mock.calls.length === 2);
await renderHost();

dispatchMoved({ work_package_ids: [11, 12] });

await waitFor(() => {
expect(state).toHaveBeenCalledWith('11');
expect(state).toHaveBeenCalledWith('12');
expect(refresh).toHaveBeenCalledTimes(1);
});
});

it('ignores an event with neither id field', async () => {
await renderHost();

dispatchMoved({});
await ctx.nextFrame();

expect(refresh).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,21 @@ export default class SplitViewSyncController extends Controller {
// was ever opened, it will still be in the cache leading to a refresh request. The upside of this potentially
// wasteful refresh is that when the work package is later on reopened in the split view, its information
// is correct as well.
onWorkPackageMoved(event:CustomEvent<{ work_package_id?:number }>):void {
const workPackageId = event.detail?.work_package_id;
onWorkPackageMoved(event:CustomEvent<{ work_package_id?:number; work_package_ids?:number[] }>):void {
const detail = event.detail ?? {};
const ids = detail.work_package_ids
?? (detail.work_package_id !== undefined ? [detail.work_package_id] : []);
// apiV3Service is wired asynchronously via useAngularServices, so it may be absent
// if the event somehow fires before the services resolve.
if (workPackageId === undefined || !this.apiV3Service) { return; }
if (ids.length === 0 || !this.apiV3Service) { return; }

const id = workPackageId.toString();
const { work_packages: workPackages } = this.apiV3Service;

if (workPackages.cache.state(id).hasValue()) {
void workPackages.id(id).refresh();
}
ids.forEach((rawId) => {
const id = rawId.toString();
if (workPackages.cache.state(id).hasValue()) {
void workPackages.id(id).refresh();
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,80 @@ describe('Backlogs work package controller', () => {
}
});

// The pressed state is visual only — data-activating, never ARIA — so
// every user gets synchronous feedback regardless of batch selection
// being enabled for them.
describe('activation feedback', () => {
it('shows pressed feedback synchronously on click, before any navigation', async () => {
const workPackage = renderWorkPackage();

await nextFrame();
workPackage.dispatchEvent(new MouseEvent('click', { bubbles: true }));

expect(workPackage.hasAttribute('data-activating')).toBe(true);
expect(workPackage.hasAttribute('aria-current')).toBe(false);
expect(navigation.openSplitPane).not.toHaveBeenCalled();
});

it('shows pressed feedback synchronously on Enter', async () => {
const workPackage = renderWorkPackage();

await nextFrame();
keydown(workPackage, 'Enter');

expect(workPackage.hasAttribute('data-activating')).toBe(true);
expect(workPackage.hasAttribute('aria-current')).toBe(false);
});

it('clears pressed feedback when the visit lands on the card', async () => {
const workPackage = renderWorkPackage();

await nextFrame();
workPackage.dispatchEvent(new MouseEvent('click', { bubbles: true }));
document.dispatchEvent(new CustomEvent('turbo:visit', {
detail: { url: '/projects/demo/backlogs/details/SP-42' },
}));

expect(workPackage.hasAttribute('data-activating')).toBe(false);
expect(workPackage.getAttribute('aria-current')).toBe('true');
});

it('clears pressed feedback when the visit lands elsewhere', async () => {
const workPackage = renderWorkPackage();

await nextFrame();
workPackage.dispatchEvent(new MouseEvent('click', { bubbles: true }));
document.dispatchEvent(new CustomEvent('turbo:visit', {
detail: { url: '/projects/demo/backlogs' },
}));

expect(workPackage.hasAttribute('data-activating')).toBe(false);
expect(workPackage.hasAttribute('aria-current')).toBe(false);
});

it('clears pressed feedback when a double-click cancels the pending click', async () => {
const workPackage = renderWorkPackage();

await nextFrame();
workPackage.dispatchEvent(new MouseEvent('click', { bubbles: true }));
workPackage.dispatchEvent(new MouseEvent('dblclick', { bubbles: true }));

expect(workPackage.hasAttribute('data-activating')).toBe(false);
expect(navigation.openFullPane).toHaveBeenCalledTimes(1);
});

it('clears pressed feedback when the card disconnects', async () => {
const workPackage = renderWorkPackage();

await nextFrame();
workPackage.dispatchEvent(new MouseEvent('click', { bubbles: true }));
fixture.remove();
await nextFrame();

expect(workPackage.hasAttribute('data-activating')).toBe(false);
});
});

it('marks the card as current when the URL points at it', async () => {
const workPackage = renderWorkPackage();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,16 @@ export default class WorkPackageController extends Controller<HTMLElement> imple
clearTimeout(this.clickTimeout);
this.clickTimeout = null;
}

// A card that reconnects must not come back pressed.
this.unmarkAsActivating();
}

private syncCurrentFromUrl(locationUrl:string):void {
// However the visit resolved, the pressed state hands over to
// aria-current or to nothing.
this.unmarkAsActivating();

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
Expand All @@ -88,7 +95,10 @@ export default class WorkPackageController extends Controller<HTMLElement> imple

// 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.
// package here would announce a navigation that may never happen. Feedback
// is visual only: data-activating goes on synchronously and carries no ARIA
// semantics, since the card is an article and role=button was rejected in
// AGILE-251.
markAsCurrent():void {
this.element.setAttribute('aria-current', 'true');
}
Expand All @@ -97,6 +107,14 @@ export default class WorkPackageController extends Controller<HTMLElement> imple
this.element.removeAttribute('aria-current');
}

markAsActivating():void {
this.element.setAttribute('data-activating', '');
}

unmarkAsActivating():void {
this.element.removeAttribute('data-activating');
}

handleEvent(event:Event):void {
switch (event.type) {
case 'click':
Expand All @@ -119,6 +137,7 @@ export default class WorkPackageController extends Controller<HTMLElement> imple

if (this.clickTimeout !== null) return;

this.markAsActivating();
this.clickTimeout = window.setTimeout(() => {
this.clickTimeout = null;
this.openSplitPane();
Expand All @@ -134,6 +153,7 @@ export default class WorkPackageController extends Controller<HTMLElement> imple
if (this.clickTimeout !== null) {
clearTimeout(this.clickTimeout);
this.clickTimeout = null;
this.unmarkAsActivating();
}

this.openFullPane();
Expand All @@ -149,6 +169,7 @@ export default class WorkPackageController extends Controller<HTMLElement> imple

event.preventDefault();

this.markAsActivating();
if (event.shiftKey) {
this.openFullPane();
} else {
Expand Down
Loading
Loading