Skip to content

fix(ui): align project rail focus order - #3167

Merged
Astro-Han merged 2 commits into
apache:mainfrom
ARE404:agent/fix-rail-project-focus-order
Aug 18, 2026
Merged

fix(ui): align project rail focus order#3167
Astro-Han merged 2 commits into
apache:mainfrom
ARE404:agent/fix-rail-project-focus-order

Conversation

@ARE404

Copy link
Copy Markdown
Contributor

Summary

  • make project rows follow their visual keyboard order: project navigation → project actions → nested tasks
  • add a narrow Astryx SideNavItem.trailingAction seam so the menu stays a sibling control instead of moving before the whole project item or nesting inside its button
  • replace the sibling-control test's backtracking markup regex with a DOM structural assertion and pin the focus order in unit and Electron E2E coverage

The rail looked left-to-right like a project row followed by its action, but the DOM put the action first. SideNavItem owns both the project control and its nested task group: an external sibling can only sit before both or after both, while endContent renders inside the project button and cannot hold another button. The new slot is rendered after the primary control and before children, which gives the rail the intended order without custom Tab handling or invalid nested controls.

There is no visual geometry change. The menu keeps the same reserved trailing position and hover/pressed feedback.

Refs #2984

Verification

  • npm --workspace @maka/desktop run build:workspace-deps — pass
  • npm --workspace @maka/desktop run typecheck — pass
  • npm run format:check — pass
  • npm run lint — pass
  • node --test packages/ui/dist/__tests__/session-history-row-actions.test.js — 2 passed
  • npx playwright test --config e2e/playwright.config.ts e2e/sidebar-project-row.spec.ts — focus-order and grouping cases passed; the unchanged pointer case hit one Electron first-window startup timeout
  • npx playwright test --config e2e/playwright.config.ts e2e/sidebar-project-row.spec.ts -g 'task row action menu accepts pointer selection' — retry passed
  • Storybook Product/Sidebar Session List / Project Groups — verified unchanged geometry, zero nested buttons, and the actual Tab sequence project → actions → first task

Review focus

trailingAction extends the existing local Astryx patch rather than recreating SideNav collapse behavior in product code or intercepting Tab. It is deliberately a sibling-only render seam with no styling or focus policy of its own; remove the hunk when Astryx publishes an equivalent slot.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex audited the rail in Storybook, implemented the component seam and product integration, updated and ran the focused tests, and drafted this PR description. The human contributor owns review and submission.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@coderabbitai

coderabbitaiBot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3c3c386d-e959-4bfc-997f-cce571d83cf0

📥 Commits

Reviewing files that changed from the base of the PR and between 8dc8029 and a1c6f15.

📒 Files selected for processing (1)
  • packages/ui/src/__tests__/session-history-row-actions.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/ui/src/tests/session-history-row-actions.test.tsx

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

What this PR solves

The project rail now follows the visual keyboard order:

  1. Project navigation
  2. Project actions
  3. Nested tasks

SideNavItem.trailingAction places the project action menu after the project navigation control. This avoids invalid nested buttons, custom Tab handling, and CSS-only reordering.

Source of truth and solution size

The PR extends the existing SideNavItem source of truth. It does not create a parallel navigation path or focus-order authority.

The solution is the smallest coherent change. It adds one sibling slot and updates the project row to use it. The added complexity preserves DOM order, visual geometry, and existing interaction feedback.

No code or tests are clear deletion candidates. The raw-markup and parsed-DOM checks provide complementary protection against nested-button regressions.

Validation and risks

  • Unit tests verify button structure and project navigation order.
  • Electron E2E tests verify focus movement through successive Tab presses.
  • CSS updates preserve disclosure positioning and hover/pressed feedback.
  • Storybook evidence reports unchanged geometry and only a changed focus-ring position.
  • The author reported successful build, typecheck, formatting, linting, and affected-test checks. Required-check status is unverified here.

Complexity delta

  • Authorities: Reuses SideNavItem for trailing-action placement. No parallel focus-order authority was added.
  • State: Adds no runtime state.
  • Branches: Retains the existing conditional branch for available project actions.
  • Configuration: Adds no configuration.
  • Public surface: Adds optional SideNavItemProps.trailingAction.
  • Test maintenance: Adds DOM-structure and raw-markup validation, and updates focus-order expectations.

Total maintenance complexity stays justified. The public slot and focused tests directly support the required keyboard behavior.

Review-relevant risks

The diff changes user-visible keyboard focus order in the project rail. Independent human review is required for this behavior under repository policy.

The diff adds the public SideNavItemProps.trailingAction property. Independent human review is required for this public contract change under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge must review the final diff. A maintainer makes the final determination.

Walkthrough

The project action menu now uses SideNavItem.trailingAction. Sidebar styling targets the updated structure. Unit and end-to-end tests verify sibling buttons and visual keyboard order.

Changes

Project action layout

Layer / File(s)Summary
Add the trailing action seam
patches/@astryxdesign+core+0.4.0.patch, patches/README.md
SideNavItem exposes trailingAction and renders it as a sibling before nested navigation content. The patch documentation describes the new seam.
Integrate project actions
packages/ui/src/session-history-list.tsx, apps/desktop/src/renderer/styles/sidebar.css
Project actions move into SideNavItem.trailingAction. CSS selectors and documentation match the updated DOM structure.
Validate markup and keyboard order
packages/ui/src/__tests__/session-history-row-actions.test.tsx, apps/desktop/e2e/sidebar-project-row.spec.ts
Tests reject nested buttons and verify navigation focus precedes the project action and session controls.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to a1c6f

The project rail now follows the intended keyboard order while preserving the existing layout and interaction behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Ai Use Disclosure⚠️ WarningThe PR discloses Codex, but commit a1c6f15 has a valid Generated-by: Claude Code trailer, which names an undisclosed tool.Disclose Claude Code and its scope, or correct the trailer if inaccurate. Follow CONTRIBUTING.md’s “Human ownership and AI attribution” section; ensure the trailer survives squash or amend.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly summarizes the main change: correcting keyboard focus order in the project rail.
Description check✅ PassedThe description includes the required summary, verification, AI use, checklist, issue reference, and review focus details.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@jackwener

Copy link
Copy Markdown
Member

@ARE404 I confirmed the bug on current main: the project action precedes the project navigation control in DOM/Tab order even though it is visually trailing. CSS-only reordering would not fix keyboard order, and nesting the action in endContent would create invalid nested controls, so the narrow sibling slot is a reasonable minimal seam. I found no architectural blocker in the draft. Keep it draft until the required before/after visual evidence is attached; once ready, this user-visible focus-order change also needs independent human review under CONTRIBUTING.md.

@ARE404
ARE404force-pushed the agent/fix-rail-project-focus-order branch from 9db517f to 8dc8029CompareAugust 18, 2026 14:06
@ARE404

ARE404 commented Aug 18, 2026

Copy link
Copy Markdown
ContributorAuthor

Rebased onto 4a54ee25c; the only conflict was patches/README.md, where
#1874 had already taken the section to 0.4.0 / "Five seams" — resolved to
"Six" keeping both bullets. All local checks re-run green after the rebase
(format:check, lint, @maka/desktop typecheck incl. renderer + storybook,
and session-history-row-actions 6/6).

Visual evidence below, as requested. Both columns come from one Playwright
script run twice against the Storybook Product/Sidebar Session List → Project Groups story — once on this branch, once on main — focusing the
"By project" toggle above the rail and pressing Tab three times, recording
document.activeElement at each step alongside the screenshot:

image

Geometry is byte-identical across the three pairs; only the focus ring moves.

@ARE404
ARE404 marked this pull request as ready for review August 18, 2026 16:30
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Align project rail keyboard focus with visual order

🐞 Bug fix🧪 Tests🕐 20-40 Minutes

Grey Divider

AI Description

• Order project navigation, action menus, and nested tasks to match visual keyboard flow.
• Add an Astryx sibling action slot without nesting interactive controls.
• Cover DOM structure and Tab order in unit and Electron tests.
Diagram

graph TD
A["ProjectNavRow"] -->|configures| B["Patched SideNavItem"]
B -->|renders first| C["Project navigation"] -->|then| D["Trailing action"] -->|then| E["Nested tasks"]
F["Browser Tab"] -->|starts at| C
Loading
High-Level Assessment

Keep the narrow SideNavItem patch. It preserves native DOM-based Tab behavior, valid sibling controls, existing collapse semantics, and unchanged visual geometry. Placing the action in endContent would create nested buttons, while external placement or custom keyboard handling would produce incorrect ordering or unnecessary focus-management complexity; replace the patch when Astryx publishes an equivalent slot.

Files changed (6) +126 / -24

Bug fix (2) +20 / -16
sidebar.cssRetarget project action styles to the new wrapper depth+7/-5

Retarget project action styles to the new wrapper depth

• Adjusts positioning and hover/pressed selectors for actions now rendered inside SideNavItem's root wrapper. The reserved trailing geometry and project-header feedback remain unchanged.

apps/desktop/src/renderer/styles/sidebar.css

session-history-list.tsxRender project actions through SideNavItem trailingAction+13/-11

Render project actions through SideNavItem trailingAction

• Moves ProjectItemActions from before SideNavItem into its trailingAction slot. This places the menu after project navigation and before nested sessions without nesting interactive controls.

packages/ui/src/session-history-list.tsx

Tests (2) +20 / -7
sidebar-project-row.spec.tsVerify visual project-row Tab order+3/-3

Verify visual project-row Tab order

• Updates the Electron test to focus project navigation first and assert that Tab advances to the action menu, then the first nested task. Existing grouping, collapse, and non-nested-button checks remain intact.

apps/desktop/e2e/sidebar-project-row.spec.ts

session-history-row-actions.test.tsxAssert sibling controls and project button order structurally+17/-4

Assert sibling controls and project button order structurally

• Replaces the backtracking markup regex with a parsed-DOM nested-button assertion. Pins project navigation before its action and nested task controls in document order.

packages/ui/src/tests/session-history-row-actions.test.tsx

Documentation (1) +6 / -1
README.mdDocument the SideNavItem dependency patch+6/-1

Document the SideNavItem dependency patch

• Records the sixth Astryx component seam and explains why endContent or an external sibling cannot provide valid controls in the required Tab order.

patches/README.md

Other (1) +80 / -0
@astryxdesign+core+0.4.0.patchAdd an interactive SideNavItem trailing action seam+80/-0

Add an interactive SideNavItem trailing action seam

• Extends Astryx SideNavItem source, built JavaScript, and declarations with an optional trailingAction prop. The slot renders as a sibling after the primary navigation element and before nested children.

patches/@astryxdesign+core+0.4.0.patch

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 978ec180-a799-4281-b95d-aeea5c011e23

📥 Commits

Reviewing files that changed from the base of the PR and between 4a54ee2 and 8dc8029.

📒 Files selected for processing (6)
  • apps/desktop/e2e/sidebar-project-row.spec.ts
  • apps/desktop/src/renderer/styles/sidebar.css
  • packages/ui/src/__tests__/session-history-row-actions.test.tsx
  • packages/ui/src/session-history-list.tsx
  • patches/@astryxdesign+core+0.4.0.patch
  • patches/README.md

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at 8dc802964cbe493a6fff6c317657e18960186ffe.

This is a focused fix for keyboard focus order in the expanded project rail. The new trailingAction seam keeps the visual and DOM order aligned without duplicating navigation state, avoids nested interactive controls, and is exercised at the current product call site. The collapsed-rail concern raised during adversarial review is not reachable from that call site, so I do not see a concrete issue to block this change.

All current checks, including workspace tests, E2E, Storybook, package validation, Windows coverage, and CodeRabbit, are green.

AI-assisted review disclosure: Codex coordinated the review and verified the final diff, current call sites, DOM/focus ordering, test coverage, and live CI at this head. Two independent reviewer-agent passes and an OpenCode Go DeepSeek V4 Flash (high) pass were used as adversarial inputs; their claims were independently adjudicated against the code.

中文复核

已在 8dc802964cbe493a6fff6c317657e18960186ffe 复核。该改动通过最接近的现有组件扩展点调整展开态项目栏的 DOM 与键盘焦点顺序,没有复制导航状态,也没有引入嵌套交互控件。对抗审查提出的折叠态问题在当前唯一产品调用路径中不可达,因此不构成阻塞。当前所有检查均已通过。

本次为 AI 辅助审查:Codex 负责统筹并复核最终 diff、调用路径、焦点顺序、测试与实时 CI;另使用两次独立 reviewer 审查及一次 OpenCode Go DeepSeek V4 Flash(high)对抗审查,相关结论均已回到代码中独立核验。

parseHTML auto-closes a <button> that opens directly inside another, so
the structural assertion alone cannot see that shape. Count start and end
tags on the raw markup as well; a single-token match keeps it linear.
Generated-by: Claude Code
@Astro-Han
Astro-Han merged commit 781fa8d into apache:mainAug 18, 2026
18 checks passed
Sign up for freeto 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.

3 participants

@ARE404@jackwener@Astro-Han