Skip to content

refactor(spotlight): consolidate picker state and UI components - #1843

Merged
beruro merged 2 commits into
developfrom
dev/spotlight-components
Sep 15, 2026
Merged

beruro merged 2 commits into
developfrom
dev/spotlight-components

Conversation

@Chloe-JY

@Chloe-JY Chloe-JY commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Spotlight duplicates route ownership, row/panel presentation and form plumbing. Parallel layer flags and unreachable flows obscure keyboard ownership, while worktree refreshes can reuse pre-mutation requests. Compact model rows also nest variant actions inside selection buttons, and clone forms do not expose their pending state.

Solution

Consolidate Spotlight ownership and reusable UI boundaries in one cleanup:

  • Use one active route with an exhaustive render switch, cancel deferred focus when inactive, and remove unreachable confirmation/branch flows and unused exports. Editor command mode uses supported registered commands.
  • Guard stale worktree/GitHub completions, share opener/footer navigation plumbing, and separate full-row activation from pin/checkbox actions.
  • Complete the six approved UI sweeps: five dropdown surfaces, six compact row presenters, three directory fields, field layout across six forms, four form action footers, and the GUI/TUI segmented filter.
  • Add an opt-in compact Form.Item layout with label/help associations. Keep variant actions beside the row button, preserve hover-card refs and tutorial anchors, and propagate clone loading with a synchronous duplicate-submit guard.
  • Delete the unreachable combined clone form and subtab route, including their four unused translation keys across all 13 locales. Explicitly handle async navigation/directory rejection and use queueMicrotask for synchronous selector callbacks. Include regression tests and architecture/UI/lifecycle reports.

The original cleanup and component consolidation share the objective of reducing competing Spotlight state and UI owners. Second-pass audit reports explicitly identify findings still left for follow-up; this PR does not claim to fix every issue recorded there.

Potential risks

  • Native pointer hit-testing, keyboard focus timing, light/dark themes and narrow-window layout remain unverified. No screenshots or recordings were collected because the user requires explicit opt-in for desktop control. jsdom tests verify markup/events, not native layout.
  • Form.Item is shared beyond Spotlight. Compact presentation is opt-in; existing default layout and colon behavior remain. Review label/help and forwarded DOM attributes for compatibility.
  • GitHub branch tests use deferred mock responses; the current OSS connection provider is a stub. Live authentication/network behavior was not exercised.
  • Clone requests already dispatched continue after form close; cancellation and cross-instance deduplication are outside this change. CPU/RSS and real-window lifecycle measurements were not run, so the performance report records a blocked measurement verdict and makes no speed claim.
  • No dependency, lockfile, Rust, persistence/schema or IPC/wire changes. Rollback is a revert of this commit, with no data migration or historical cleanup needed.

Verification

GitHub CI passed on 88234504823f79ced784b56c85c00a9d13adc994: frontend run completed 719 test files, 4,952 tests passed, 1 skipped; typed lint and CodeQL analysis also passed. All active PR checks passed; Rust jobs were correctly skipped.

Local verification:

  • pnpm test src/features/SessionCreator/components/WorkItemPickerModal/WorkItemPickerModal.test.ts src/features/SessionCreator/variants/ChatPanel/WorkItemAttachmentControl.test.ts src/scaffold/GlobalSpotlight src/hooks/keyboard — 67 files, 325 tests. Covers root/child keyboard ownership and retargeting, deferred focus, stale responses and worktree invalidation, registered commands, independent row actions, directory hover, organization guide anchors, field labels/help, folder selection/cancellation, native submit and clone busy/concurrency behavior.
  • pnpm typecheck:fast — full project passed; normal commit-hook typecheck also passed.
  • pnpm check:i18n-keys — zero new findings; four unused keys removed consistently across locales.
  • NODE_OPTIONS=--max-old-space-size=6144 pnpm check:typed-lint — 1,086 existing findings, zero new or increased findings. No lint baselines or suppressions changed.
  • git diff --name-only -z origin/develop > /tmp/spotlight-ci-paths followed by node scripts/ci/run-unit-tests.mjs < /tmp/spotlight-ci-paths selected 719 files. 717 passed; two consumer suites exposed seven stale wrapper-click assertions. Both corrected suites pass in the 325-test focused rerun above, now activating the shared Button and preserving selection/refresh/focus/disabled assertions.
  • git diff --name-only -z --diff-filter=ACMR origin/develop...HEAD -- '*.ts' '*.tsx' | xargs -0 pnpm exec eslint --max-warnings 0 --report-unused-disable-directives — 105 changed TypeScript files passed.
  • git diff --name-only -z --diff-filter=ACMR origin/develop...HEAD -- '*.ts' '*.tsx' | xargs -0 pnpm exec oxlint -c .oxlintrc.json --max-warnings 0 — passed; normal lint-staged hooks also passed.
  • pnpm check:boundaries — 8,263 modules, zero new forbidden edges.
  • pnpm check:test-placement — consistent across 585 directories.
  • git diff --name-only -z --diff-filter=ACMR origin/develop...HEAD -- '*.ts' '*.tsx' | node scripts/ci/check-changed-file-length.cjs — all 86 changed production TypeScript files within 700 lines.
  • git diff origin/develop...HEAD --check — passed.
  • TypeScript AST inspection found no raw button/input/textarea/select controls in changed production files. The three existing shell div click handlers route focus/contain events. Final diff inspected for scope, personal paths, secrets, debug logs and generated artifacts; none introduced.

Native UI, real GitHub requests, CPU/RSS measurements and Rust checks were not run for the reasons above; Rust was not changed. The branch starts at the latest fetched develop commit. Both commit author and committer are Chloe-JY.

Audit

  • Component consolidation: 1 fix + 5 abstraction sweeps completed; final verdict 0 fix, 10 keep with reason, 0 abstract, 1 watch in SpotlightComponents.
  • Architecture: all ten layers reviewed at the applicable scope; no wire change. Cleanup report.
  • Automated lifecycle/concurrency checks pass. Native performance measurement remains blocked by the desktop-control opt-in requirement. Lifecycle report.

@beruro
beruro merged commit 4e5eda7 into develop Sep 15, 2026
15 of 16 checks passed
@Harry19081 Harry19081 added refactor Code restructuring without an intended behavior change frontend-ui Frontend UI, design system, accessibility, layout, or theming labels Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend-ui Frontend UI, design system, accessibility, layout, or theming refactor Code restructuring without an intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants