Skip to content

Commit 5e759f4

Browse files
Harry19081Harry19081
authored
Merge pull request #1159 from org2AI/dev/work-item-picker-spotlight
refactor(spotlight): reuse picker formats for work items
2 parents a963f5f + a9cd92c commit 5e759f4

20 files changed

Lines changed: 1061 additions & 445 deletions

File tree

‎docs/architecture-audit-2026-09-01/WorkItemSpotlight.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Work-item Spotlight architecture audit
2+
3+
Scope: replace the work-item picker's bespoke modal/list with existing Spotlight components and extract reusable pill/attached tabs and an optional checkbox row prop. Preserve the domain selection callback, data loaders, limits, repository reset and consumer behavior.
4+
5+
Acceptance: one shell/list format; one tab keyboard implementation; Tab/Shift+Tab source switching; no selection loss while filtering; only Add applies the draft; late loads cannot restore closed/replaced picker state; no backend/wire/persistence changes.
6+
7+
| Layer | Verdict | Evidence |
8+
| --------------------- | ------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
9+
| 1. Compilation | Verified separately | Full TypeScript typecheck and changed-file ESLint; see verification record |
10+
| 2. Deduplication | Pass | Removed modal chrome, tab buttons and bespoke checkbox-row markup. `SpotlightShell`, `PaletteBody`, `SpotlightItemList` and pinned actions are used in production |
11+
| 3. Naming | Pass | `SpotlightTabs` owns presentation/keyboard behavior; `BranchPickerTabs` supplies branch labels; `WorkItemPickerModal` remains the existing public dialog API |
12+
| 4. Semantic overload | Pass | Source filter, keyboard-highlighted row and checked domain keys remain distinct. Changing a source never applies or discards the selection |
13+
| 5. Defaults | Pass | Tab is the new component default; branch explicitly opts into Ctrl+Tab. Disabled tabs are skipped. Unknown PR statuses remain visible and neutral |
14+
| 6. Domain boundaries | Pass | Shared tabs accept generic string options, not work-item or branch types. The shared row receives optional `selectionState`; list, pinned-action and model-column renderers forward it. Its checkbox emits one toggle per native change |
15+
| 7. Readability | Pass | Row projection and pinned actions stay with the feature; data fetching and final selection stay in the controller |
16+
| 8. Wire/serialization | Not applicable | No API, IPC, schema or serialized payload changes; domain options pass through unchanged |
17+
| 9. Entry-point parity | Pass | Launchpad card/pill, composer solve flow and link-existing menu still use the same picker. Consumer integration tests verify final inserted pills and work-item context. Branch palette and dropdown use the same shared tabs |
18+
| 10. Resolver symmetry | Not applicable | No resolver or fallback-chain changes; existing repository/auth/data acquisition remains untouched |
19+
20+
The native shell is intentionally the existing Spotlight format, including Escape/backdrop dismissal rather than separate modal Cancel/close buttons. No source-data remediation or migration is needed. The explicit checkbox prop revealed duplicate wrapper-click and native-change dispatch; the native Checkbox change handler now owns toggling. Tests exercise both the input and visible checkbox icon, and ordinary row actions remain independent.
21+
22+
The virtual-list test layout helper now restores only its own geometry spies; reusing it must not erase unrelated consumer API mocks. Tests use production Spotlight controls and navigation, supplying jsdom geometry rather than replacing the virtualizer.
23+
24+
No Rust checks were run because no Rust/backend files belong to this change. Unrelated pre-existing workspace modifications are excluded from this audit.

‎docs/frontend-ui-audit-2026-09-01/SpotlightTabs.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# SpotlightTabs UI audit
2+
3+
| Line | Element | Verdict | Reason | Suggested change |
4+
| ---------------------------------------------------------------------------------------------------------- | ----------------------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------- |
5+
| `src/scaffold/GlobalSpotlight/components/SpotlightTabs.tsx:100` | Pill and attached tab formats | keep with reason | The pill format composes `SegmentedTextPill`; the requested attached format centralizes raised native tab buttons and the continuous divider. Domain consumers provide labels/icons only | None |
6+
| `src/scaffold/GlobalSpotlight/components/SpotlightTabs.tsx:29` | Keyboard handling | keep with reason | A scoped capture listener handles Tab/Shift+Tab, disabled options, wrapping and focused-tab activation before row navigation; composing and system-modified events are ignored | None |
7+
| `src/scaffold/GlobalSpotlight/palettes/BranchPalette/BranchPickerTabs.tsx:18` and `BranchDropdown.tsx:303` | Shared branch consumer | keep with reason | Branch labels/types remain domain-owned; the shared component replaces branch-specific keyboard code while retaining Ctrl+Tab and ordinary Tab section behavior. The dropdown marks its keyboard scope | None |
8+
9+
Verdict totals: **0 fix**, **3 keep with reason**, **0 abstract**.
10+
11+
Covered D1–D5. Native tab buttons belong to the shared tab primitive itself; no consumer implements its own tab markup. Colors and spacing use semantic tokens. This extraction is explicitly requested by the user, rather than an unrelated multi-file sweep. The pill format retains `aria-pressed`; attached tabs use `tablist`/`tab`, `aria-selected`, roving focus and a visible keyboard focus ring.
12+
13+
The attached format owns an `mb-2` bottom offset (8px), keeping the first result clear of the divider without per-picker spacing overrides.
14+
15+
Verification: seven shared-tab tests cover disabled-option skipping, reverse/wrap switching, focused activation, Ctrl+Tab compatibility, independent mounted pickers, composition/system shortcuts, selected-tab scrolling, and five mount/unmount cycles. Existing branch palette/dropdown tests pass. Native visual verification was not performed.

‎docs/frontend-ui-audit-2026-09-01/WorkItemPickerModal.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# WorkItemPickerModal UI audit
2+
3+
| Line | Element | Verdict | Reason | Suggested change |
4+
| --------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------ | ---------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------- |
5+
| `src/features/SessionCreator/components/WorkItemPickerModal/index.tsx:150` | Picker chrome | keep with reason | Uses the same `SpotlightShell` as the branch palette; retains an accessible dialog name and focus restoration without the former modal header/footer | None |
6+
| `src/features/SessionCreator/components/WorkItemPickerModal/WorkItemPickerPanel.tsx:237` | Search and result list | keep with reason | `PaletteBody` owns the standard search row and `SpotlightItemList`; no separate work-item row layout remains | None |
7+
| `src/features/SessionCreator/components/WorkItemPickerModal/WorkItemPickerPanel.tsx:170` and `src/scaffold/GlobalSpotlight/components/SpotlightItemRow.tsx:144` | Multi-selection | keep with reason | Exposes an optional `selectionState` row prop (checked state, accessible label, toggle callback); all list, pinned-action and two-column callers forward it. Native checkbox change is the only checkbox toggle dispatcher | None |
8+
| `src/features/SessionCreator/components/WorkItemPickerModal/WorkItemPickerPanel.tsx:277` | Add and Refresh | keep with reason | Uses `SpotlightPinnedActionSection`, including disabled state and keyboard navigation; Escape/backdrop dismiss without applying the draft | None |
9+
| `src/features/SessionCreator/components/WorkItemPickerModal/WorkItemPickerPanel.tsx:49` and `:257` | Status glyphs and empty-state sizing | keep with reason | Glyph colors come from existing semantic PR tokens; unknown states stay neutral. The 350px result viewport matches the branch palette, and the empty state uses `Placeholder` | None |
10+
11+
Verdict totals: **0 fix**, **5 keep with reason**, **0 abstract**.
12+
13+
Covered D1–D5. Colors and spacing use existing Tailwind mappings; the result-height literal matches the existing Spotlight format. Shared tabs are audited separately in `SpotlightTabs.md`. No additional config-level sweep is proposed.
14+
15+
Verification: picker/consumer/branch/tab/row suites are covered by the verification record; changed-file ESLint passed. No desktop UI control or visual screenshots were taken, per the user's Computer Use preference. Native theme, viewport, and screen-reader behavior remain unverified.

‎docs/org2-performance-guard-2026-09-01/WorkItemSpotlight.md‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Work-item Spotlight lifecycle verification
2+
3+
| Area | Verdict | Evidence | Change or reason kept | Verification |
4+
| ------------------ | ------- | ----------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- |
5+
| Background work | keep | `SpotlightTabs` owns one capture listener; no polling, retries, timers or workers are introduced | Listener exists only while mounted and is restricted to the enclosing picker | Seven tab tests, including two mounted scopes and matching listener removal over five open/close cycles |
6+
| Memory | keep | Existing local projection remains capped at 500 work items and visible search results at 20; selection is scoped to the open dialog | Reuses Spotlight's virtualized fixed viewport; no new app-lifetime cache | Model limit tests; picker close/reopen and immediate-close tests |
7+
| Scope/isolation | keep | Controller is keyed by repository id/path; request generation invalidates stale completion | Data/cache/auth contracts remain unchanged | Repository switch, close during request, refresh removing a checked item, and error/retry tests |
8+
| Rendering/hot path | keep | Search and selection map bounded options; shared rows handle checked state and existing PR metadata | No streaming subscription, interval, or scan added; tab switching does not fetch | Keyboard/consumer tests and explicit no-extra-workspace-read assertion on source switching |
9+
10+
| Resource | Open/active | Idle/hidden | Close/reopen | Offline/scope changes |
11+
| --------------------- | ------------------------------------------------------------------ | ---------------------------------------------- | ------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- |
12+
| Shared tab listener | Responds only to keyboard events in its picker | No scheduled work; no key events means no work | Removed on unmount; five cycles verified | No network or identity state |
13+
| Workspace/GitHub data | Existing load-on-open and explicit refresh | No new polling or refresh trigger | Existing request generation guards retained | Error/retry and repository-reset tests; real GitHub offline/account transitions not rerun |
14+
| Spotlight search/list | Existing finite focus/placeholder tasks and viewport subscriptions | No new feature-level loops | Unmount removes rendered list; delayed focus cannot target detached input | Native WebView behavior unmeasured |
15+
16+
Verification commands:
17+
18+
```sh
19+
pnpm test src/scaffold/GlobalSpotlight/components/SpotlightTabs.test.ts src/scaffold/GlobalSpotlight/components/SpotlightItemRow.test.ts src/features/SessionCreator/components/WorkItemPickerModal src/features/SessionCreator/variants/ChatPanel/WorkItemAttachmentControl.test.ts src/scaffold/GlobalSpotlight/palettes/BranchPalette/__tests__ src/scaffold/GlobalSpotlight/palettes/adapters/__tests__/repoAdapter.test.ts src/scaffold/GlobalSpotlight/palettes/WorkspacePalette/workspacePaletteItems.test.ts src/scaffold/GlobalSpotlight/palettes/UnifiedModelPalette/__tests__
20+
pnpm run typecheck
21+
pnpm exec eslint src/scaffold/GlobalSpotlight/components/SpotlightTabs.tsx src/scaffold/GlobalSpotlight/components/SpotlightTabs.test.ts src/scaffold/GlobalSpotlight/components/SpotlightItemRow.test.ts src/scaffold/GlobalSpotlight/components/SpotlightItemRow.tsx src/scaffold/GlobalSpotlight/components/SpotlightItemList.tsx src/scaffold/GlobalSpotlight/components/SpotlightPinnedActionSection.tsx src/scaffold/GlobalSpotlight/palettes/UnifiedModelPalette/TwoColumnModelBody.tsx src/scaffold/GlobalSpotlight/shared/types.ts src/scaffold/GlobalSpotlight/palettes/BranchPalette/BranchPickerTabs.tsx src/scaffold/GlobalSpotlight/palettes/BranchPalette/BranchDropdown.tsx src/scaffold/GlobalSpotlight/palettes/BranchPalette/__tests__/BranchPalette.test.ts src/scaffold/GlobalSpotlight/palettes/BranchPalette/__tests__/virtualListTestLayout.ts src/features/SessionCreator/components/WorkItemPickerModal/index.tsx src/features/SessionCreator/components/WorkItemPickerModal/WorkItemPickerPanel.tsx src/features/SessionCreator/components/WorkItemPickerModal/WorkItemPickerModal.test.ts src/features/SessionCreator/variants/ChatPanel/WorkItemAttachmentControl.test.ts --max-warnings 0
22+
pnpm run check:test-placement
23+
pnpm run check:circular
24+
git diff --check
25+
```
26+
27+
Tests: 100 passed across 16 files. Full TypeScript typecheck passed. Changed-file ESLint passed with zero warnings. Test placement passed across 442 directories. Circular-dependency check passed across 6,375 modules. `git diff --check` and scoped Prettier verification passed.
28+
29+
Performance verdict: **blocked for native runtime measurement**. Automated interaction/lifecycle checks pass, but primary/secondary Tauri visible-idle, hidden-idle and post-close CPU/RSS measurements and visual theme/viewport verification were not performed. Computer Use was not authorized; this refactor makes no measured CPU/RAM improvement claim. No provider-ingestion or sync behavior changed, so those transition matrices are not applicable.

0 commit comments

Comments
 (0)