Skip to content

Drop the status dropdown from kanban cards and let a query node order its columns - #3532

Merged
malibio merged 2 commits into
mainfrom
issue-3514-kanban-column-order
Oct 4, 2026
Merged

malibio merged 2 commits into
mainfrom
issue-3514-kanban-column-order

Conversation

@malibio

@malibio malibio commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

Closes #3514

What changed

Kanban cards carry no status control. The <select class="kanban-card-move"> on every card is gone; the card shows only its title, and its column is its status.

A card can still be moved without a mouse. A "Move to" menu opens on demand, from the card's context menu or with M on the focused card. It lists every column but the card's own, takes arrow keys / Home / End / Escape, and writes through the same moveCard path as a drag. After a move, focus follows the card into its new column.

A query node defines its kanban column order. view_config.kanban.columnOrder maps a group-by field to that field's values in column order, so each field keeps its own order when group-by changes.

  • Listed values come first; values the list omits follow in enum order; values the enum no longer has are ignored; "Unassigned" stays last.
  • With no stored order, columns keep enum order as before.
  • The viewer carries the order through every view-config write: view changes, group-by changes, the default view's per-type preferences, and the node a default view materializes.
  • parseViewConfigObject is now the one reader for both the query node's viewConfig and the default view's stored preferences.

Seeds. Linear's "Issues by Status" declares workflow order (Triage, Backlog, Open, In Progress, In Review, Done, Cancelled); its enum lists the base task.status values before the ones the playbook adds. The Jira and spec-driven boards are unchanged: their enum order is already the intended column order.

view_config stays an open object owned by the viewer, so this adds a key, not a schema field.

Generated files

  • packages/desktop-app/src/lib/types/generated/query-{fields,node}.ts — the view_config doc comment changed.
  • packages/skill/references/linear-playbook.md — the Linear view's seed config changed.

Docs

nodespace-docs: components/node-types.md (the query.view_config row lists kanban.columnOrder) and components/query-node.md (column order rules, card behaviour, seeding).

Tests

  • kanban-grouping.test.ts — column ordering: stored, partial, stale values, repeats, none.
  • query-node-model.test.ts, default-view-prefs.test.ts — parsing columnOrder; merge keeps it across view and group-by changes.
  • kanban-view-card-move.test.ts (new) — no dropdown on cards; keyboard move, Escape, context menu; column order applied by the board.
  • query-node-viewer-materialize-race.test.ts — a saved query keeps its order across view and group-by changes; a default view carries it into preferences and the materialized node.
  • Browser tier (kanban-dnd, kanban-consecutive-status-moves) — moves that used the select now go through the menu.
  • Rust — the view-config test checks columnOrder's shape; a Linear test checks the board orders every status and nothing else.

bun run test:changed passes (frontend, scripts, skill, browser, types:check, node-types:check, Rust).

🤖 Generated with Claude Code

@malibio

malibio commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Pragmatic Code Review: PR #3532

Review Type: Initial Review (no prior reviews or comments on the PR; one commit, 3c9fcaa3)

Summary: The change does what the issue asks, in one cohesive commit. No blockers. One item is worth checking before merge: the menu's mouse path has not been exercised with real pointer events, and the app ships on WebKit, not the Chromium the browser tier runs.

What I checked: the full source diff, kanban-view.svelte in full, the view-config write paths in query-node-viewer.svelte, the model and prefs readers, the Rust seeds and tests, the new and changed tests, and the uncommitted docs diff. I did not run any tests; test:changed was reported green by the author.


Requirements Check

# Acceptance criterion Status Notes
1 Cards show no status dropdown or other status control; title and click-to-open unchanged ✅ The <select> and its styles are gone. Tested in kanban-view-card-move.test.ts.
2 A keyboard user can move the focused card; same write as a drag ✅ M or the context menu opens the menu; chooseColumn calls moveCard. Focus follows the card. See findings 1 to 3 on robustness and discoverability.
3 view_config.kanban.columnOrder for the group-by field orders the columns ✅ enumColumns(field, order), keyed by activeGroupBy.
4 Omitted values follow in enum order; unknown values ignored; "Unassigned" last ✅ Repeats are handled too. "Unassigned" is appended after ordering.
5 No stored order means enum order ✅ Early return for a missing or empty list.
6 Changing view or group-by keeps the stored order ✅ Saved branch: persistViewConfig merges onto the node's parsed config. Default branch: currentViewConfig carries it into prefs and the materialized node. Both tested through the viewer.
7 A Playbook board can set its order; seeds pass the renderable-view-config test ✅ Linear sets it. Jira and spec-driven boards group by fields whose enum order is already the intended one (I checked the Jira status and sprint_status enums: no added values).
8 node-types.md query.view_config row lists kanban.columnOrder ✅ Present in the docs working tree, uncommitted. The merge gate reads the docs remote's main, so push it just before bun run merge.
9 Tests: ordering (stored, partial, stale, none); order kept across view and group-by changes; keyboard move; no dropdown ✅ All four present. Gaps in finding 1 and 4.
10 bun run quality:fix ✅ Not rerun by me; reported by the author.
11 bun run test:changed ✅ Not rerun by me; reported green on every tier.

Project rules: no compatibility or migration code, no lint suppression, no raw console, no issue numbers in comments, no literal node_type comparisons. view_config stays an open object, so no schema field is added and the node type sequence does not apply. The generated TypeScript diff is the regenerated doc comment only.


Code Review Findings

1. 🟡 Important: the menu's mouse path is untested with real pointer events, and the app runs on WebKit

📁 packages/desktop-app/src/lib/components/query/kanban-view.svelte:420-425, 535-553

onMenuFocusOut closes the menu whenever focus leaves it with a relatedTarget outside the menu, or none. On macOS WebKit, clicking a <button> does not focus it. The first item holds focus (from focusFirstItem), so a mouse click on another item blurs it on mousedown. If relatedTarget is null at that point, moveMenuFor is cleared, the menu unmounts, and the click never fires: the move silently does nothing.

The tabindex="-1" on the menu container probably rescues this, because WebKit then focuses the container and menu.contains(relatedTarget) holds. I have not verified that in the app. Neither test tier would catch a failure: the browser tier is Chromium, and every menu test uses fireEvent.click, which sends no mousedown and moves no focus.

Recommendation:

  • Open a board in the real app, right-click a card, and click an item other than the first. That is the check that settles it.
  • Either way, make it independent of the browser's focus rules: add onmousedown={(e) => e.preventDefault()} on the menu so a click inside it never moves focus.

Principle: a close-on-blur popup must not depend on click-focus behaviour, which differs between engines.

2. 🟡 Important: nothing tells a sighted keyboard user that M exists

📁 packages/desktop-app/src/lib/components/query/kanban-view.svelte:526-534

The persistent control is gone, which is the point, but the only hint of the replacement is aria-keyshortcuts="M", which only assistive technology reads. The button's title still says only Open <title>. macOS has no context-menu key, so a keyboard-only user has no way to find the move path.

Recommendation: put it in the tooltip, for example title={Open ${title} (M to move)}. Also add aria-haspopup="menu" to the title button so the menu is announced.

Principle: an accessible alternative that can't be discovered is not an alternative. The issue's decision was that removing the dropdown must not remove the keyboard path.

3. 🟢 Suggestion: stale moveMenuFor can reopen the menu and take focus

📁 packages/desktop-app/src/lib/components/query/kanban-view.svelte:97, 368-370, 535-544

moveMenuFor is cleared only by a choice, Escape or focusout. If the card's element is destroyed while its menu is open (another pane changes its status, or the query re-runs), Chromium does not reliably fire focusout for a removed element. The id stays set, the menu mounts again with the card in its new column, and use:focusFirstItem pulls focus there.

Recommendation: clear moveMenuFor in the effect that already reseeds on nodeIds / displayColumns change, or when the open card's column changes for a reason other than chooseColumn.

4. 🟢 Suggestion: test gaps on the menu's close paths

📁 packages/desktop-app/src/tests/components/kanban-view-card-move.test.ts, packages/desktop-app/src/tests/browser/kanban-dnd.test.ts

Not covered:

  • focus leaving the menu (Tab, a click elsewhere) closes it without a write;
  • a real pointer click on a non-first item (finding 1). The browser tier can do this with userEvent.click from @vitest/browser/context;
  • Enter or Space on a focused item, which is the actual keyboard commit. The "keyboard" test commits with fireEvent.click.

5. 🟢 Suggestion: ArrowUp from the menu container lands one item short

📁 packages/desktop-app/src/lib/components/query/kanban-view.svelte:404-409

When focus is on the menu container and not an item, at is -1. ArrowDown gives items[0], which is right. ArrowUp gives items[(−2 + n) % n], the second-to-last item. The container can hold focus (it has tabindex="-1", and finding 1's WebKit path puts focus there).

Recommendation: treat at === -1 separately: ArrowUp goes to the last item.

6. 🟢 Suggestion: the menu can open below the visible board

📁 packages/desktop-app/src/lib/components/query/kanban-view.svelte:613-620, 704-718

The menu is position: absolute; top: 100% inside the card, and .kanban-board sets overflow-x: auto, which makes overflow-y compute to auto as well. A menu on a card near the bottom of the board is not clipped, but it extends the board's scroll area, so its lower items (up to seven on the Linear board) need scrolling to reach. Worth a look in the app on a tall column; flipping the menu above the card when there is no room below would fix it.

7. 🟢 Suggestion: the default-view branch carries an order nothing can set

📁 packages/desktop-app/src/lib/components/viewers/query-node-viewer.svelte:326, packages/desktop-app/src/lib/components/query/default-view-prefs.ts

The default view's preferences live in localStorage and are written only by the viewer, which has no control for column order. So prefs.kanban.columnOrder can exist only if someone edits localStorage by hand; the viewer test does exactly that. The cost is one line, and folding the prefs reader into parseViewConfigObject removes a duplicated parser, which is a clear gain. No change needed; noting it so the default-branch test isn't read as covering a reachable state.

Nits

  • Nit: kanban-view.svelte:731-735: hover and keyboard focus share one highlight, so two items can look selected at once. After a right-click, :focus-visible may not match the programmatically focused first item, so it may show no highlight.
  • Nit: kanban-view.svelte:521-524: the handler suppresses the native context menu even when openMoveMenu opens nothing (a board with a single column).
  • Nit: linear.rs:633: the test spells out the base statuses again; the neighbouring test has the same list as BASE (line 607). One module-level test constant would serve both.
  • Nit: mod.rs:325-341: the shared test checks columnOrder's shape, but not that its key for a kanban view includes the view's groupBy. A seed that orders a field it does not group by would pass.
  • Nit: kanban-dnd.test.ts and kanban-consecutive-status-moves.test.ts each define the same openMoveMenu / moveViaMenu helpers. This matches how those files already duplicate cardFor and columnFor.
  • Nit: kanban-view-cap.test.ts:4-5: the reflowed header comment has one over-long line.

What is good

  • enumColumns stays a pure function with the ordering rule in one place, and its tests cover stored, partial, stale, repeated and empty orders.
  • The saved branch needed no change to keep the order: persistViewConfig already merged onto the node's own config. The PR adds a test that pins it.
  • One reader (parseViewConfigObject) for both stores of the shape.
  • The Linear test compares the declared order against the real enum, so a status added later fails the test instead of misplacing a column.
  • The file header, the query.rs doc comment, the generated TypeScript, the skill reference and both docs pages all changed together.

Recommendation: APPROVE

No critical issues; every acceptance criterion is met. Before merging, check finding 1 in the real app (and preferably add the mousedown guard), and consider the one-line tooltip from finding 2. Push the nodespace-docs change just before bun run merge.

@malibio malibio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

@malibio

malibio commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review addressed (058646bc)

Addressed

  • 1 (menu mouse path): the menu cancels mousedown, so a press on an item never moves focus and the close-on-blur can't unmount the menu before the click lands, whatever the engine does with button focus. Asserted in Happy-DOM. Not exercised in the real WebKit app.
  • 2 (discoverability): the card tooltip now reads Open <title> (M to move).
  • 3 (stale open menu): an open menu closes when the board's columns or result set change.
  • 4 (test gaps): added tests for focus leaving the menu (closes, no write), focus moving between items (stays open), and the mousedown guard.
  • 5 (ArrowUp from the container): goes to the last item; tested.
  • Nits: the native context menu is left alone when a card has nowhere to move; the seeded view-config test requires columnOrder to order the groupBy field; the over-long comment line is reflowed.

Skipped

  • 2, aria-haspopup on the title button: its click opens the node, not the menu, so the attribute would misdescribe the button.
  • 4, a real-pointer browser test and Enter/Space on an item: the browser tier has no userEvent setup, and the items are native buttons, so Enter/Space is the browser's own click.
  • 6 (menu below the visible board): the board scrolls to it; flipping the menu is worth doing only if it proves awkward in use.
  • 7: no change requested.
  • Nits: shared hover/focus highlight, the repeated base-status list in the Linear test, and the helper duplication across the two browser test files (it matches how those files already duplicate cardFor / columnFor).

Tests: the four kanban Happy-DOM files (22 tests), the two kanban browser files (13 tests) and the methodology Rust tests (99) pass; quality:fix is clean.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: the review approved with no critical findings. The follow-up is a handful of small, local changes to the menu's event handling, each covered by a test, plus a tooltip string, a test assertion and a comment reflow. No design or data-shape change.

malibio and others added 2 commits October 4, 2026 16:58
… its columns (closes #3514)

A kanban card showed a select with its own status. The card now shows only its title; a Move to menu opens from the card's context menu or with M on the focused card, and writes through the same path as a drag.

A query node's view_config.kanban.columnOrder holds, per group-by field, that field's values in column order. Listed values come first, values the list omits follow in enum order, unknown values are ignored and Unassigned stays last. The viewer carries the order through every view-config write. Linear's Issues by Status board declares its workflow order.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ke M discoverable

- Cancel mousedown inside the menu so a click on an item never blurs the menu shut before the click lands, whatever the engine does with button focus
- Name the M shortcut in the card tooltip
- Close an open menu when the board's columns or result set change
- ArrowUp from the menu container goes to the last item
- Leave the native context menu alone when a card has nowhere to move
- The seeded view-config test requires columnOrder to order the groupBy field

Skipped: flipping the menu above the card near the bottom of the board (the board scrolls to it); aria-haspopup on the title button (its click opens the node, not the menu); a real-pointer browser test (the browser tier has no userEvent setup, and the mousedown guard is asserted in Happy-DOM).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@malibio
malibio force-pushed the issue-3514-kanban-column-order branch from 058646b to b92cf22 Compare October 4, 2026 14:58
@malibio
malibio merged commit e9beb1a into main Oct 4, 2026
@malibio
malibio deleted the issue-3514-kanban-column-order branch October 4, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drop the status dropdown from kanban cards, and let a query node define its kanban column order

1 participant