From a99bd60974624c19b17615dd3377d291a8407b74 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 8 Jun 2026 22:12:39 +0200 Subject: [PATCH] amend D-88: shared base + content components per surface (not one ClideMenu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record the decomposition: ClideAnchoredOverlay is the shared base every anchored surface adopts; content matches the surface — ClideMenu for menus, a new ClideTypeahead for the slash/@ typeaheads, bespoke for quick-open and the theme picker. Re-scopes T-288 accordingly. The base blockers (focus race, follower untappable in the test harness) are what to fix first. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 24 +++++++++++++++++++++++ .pql/changelog/tickets/2026-06.sql | 16 +++++++++++++++ governance/decisions/design.md | 6 ++++++ 3 files changed, 46 insertions(+) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index 98428042..26af5bf5 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -882,3 +882,27 @@ INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-286', 'status', 'backlog', 'ready', NULL, '2026-06-08 17:34:29', '2026-06-08 17:34:29', '2026-06-08 17:34:29', NULL, '01a873c4191650aa6faa3e7f63926439', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-286', 'status', 'ready', 'in_progress', NULL, '2026-06-08 17:34:31', '2026-06-08 17:34:31', '2026-06-08 17:34:31', NULL, 'b2ff3d5dbf5d72be72ba59b2d7e7c4d8', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-29', 'status', 'backlog', 'ready', NULL, '2026-06-08 17:37:13', '2026-06-08 17:37:13', '2026-06-08 17:37:13', NULL, '296037989ed5819a589d12dd46f555da', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-288', 'description', 'Follow-up to the D-88 sweep. The primitive (ClideAnchoredOverlay + ClideMenu) shipped and the menu bar (T-286) + T-275 picker are migrated. The theme-picker migration was attempted and REVERTED after hitting two real blockers that affect all the remaining anchored surfaces; resolve these first, then migrate theme picker, @-mention typeahead, slash typeahead, and quick-open. + +Blockers found (theme picker attempt): +1. Focus-capture race. ClideAnchoredOverlay.captureFocus does a post-frame _scope.requestFocus(); ClideMenu also autofocuses its own node. Depending on mount timing (e.g. an extra ListenableBuilder wrapper) the scope wins and STEALS focus from ClideMenu''s node, so arrow/enter never reach the menu (Esc still works only via the ancestor fallback). Menu bar happens to win the race; the theme popover loses. Fix the primitive''s focus model so content''s own focus node reliably ends up focused (e.g. don''t force scope focus; or make the scope delegate to the autofocus child deterministically), then re-verify menu bar + anchored tests. +2. Follower content is not reliably mouse-tappable in the widget-test harness. The shared harness() uses Overlay(canSizeOverlay) + a zero-size MediaQuery, so an anchored follower positions the panel off-screen (observed tap offsets like (680,-45) and (680,654)) and tester.tap misses. Also autoFlip reads MediaQuery.size (zero in harness) so it can''t flip. Mitigations: have autoFlip use View.physicalSize (more correct anyway); position content without the inner Align (the panel shrink-wraps); and in tests drive selection via keyboard (as menu_bar does) or provide a real viewport. Consider a test helper for anchored-overlay content. + +Then migrate, one surface per commit, keeping each existing test green (theme_picker widget_test, team_chat_sidebar_test, claude_composer_test, quick_open_overlay_test). Quick-open is centered (no follower) and the weakest-fit — it only needs the lifecycle wrapper; migrate last or skip. + +Refs: D-88, T-286, T-275 (done), menu bar (done, commit e1d51eb).', 'Follow-up to the D-88 sweep. The primitive (ClideAnchoredOverlay + ClideMenu) shipped and the menu bar (T-286) + T-275 picker are migrated. The theme-picker migration was attempted and REVERTED after hitting two real blockers that affect all the remaining anchored surfaces; resolve these first, then migrate theme picker, @-mention typeahead, slash typeahead, and quick-open. + +Blockers found (theme picker attempt): +1. Focus-capture race. ClideAnchoredOverlay.captureFocus does a post-frame _scope.requestFocus(); ClideMenu also autofocuses its own node. Depending on mount timing (e.g. an extra ListenableBuilder wrapper) the scope wins and STEALS focus from ClideMenu''s node, so arrow/enter never reach the menu (Esc still works only via the ancestor fallback). Menu bar happens to win the race; the theme popover loses. Fix the primitive''s focus model so content''s own focus node reliably ends up focused (e.g. don''t force scope focus; or make the scope delegate to the autofocus child deterministically), then re-verify menu bar + anchored tests. +2. Follower content is not reliably mouse-tappable in the widget-test harness. The shared harness() uses Overlay(canSizeOverlay) + a zero-size MediaQuery, so an anchored follower positions the panel off-screen (observed tap offsets like (680,-45) and (680,654)) and tester.tap misses. Also autoFlip reads MediaQuery.size (zero in harness) so it can''t flip. Mitigations: have autoFlip use View.physicalSize (more correct anyway); position content without the inner Align (the panel shrink-wraps); and in tests drive selection via keyboard (as menu_bar does) or provide a real viewport. Consider a test helper for anchored-overlay content. + +Then migrate, one surface per commit, keeping each existing test green (theme_picker widget_test, team_chat_sidebar_test, claude_composer_test, quick_open_overlay_test). Quick-open is centered (no follower) and the weakest-fit — it only needs the lifecycle wrapper; migrate last or skip. + +Refs: D-88, T-286, T-275 (done), menu bar (done, commit e1d51eb). + +RE-SCOPE (2026-06-08, per D-88 amendment): the goal is NOT ''migrate everything onto ClideMenu''. The shared base is ClideAnchoredOverlay; content matches the surface: +- ClideMenu stays scoped to selectable-list menus (menu bar + permission picker — done). +- Add a NEW shared content component ClideTypeahead for the slash + @ typeaheads (near-duplicate caret-anchored completion surfaces) — they share this, not ClideMenu. This dedups two hand-rolls. +- Quick-open keeps bespoke content (centred filter + fuzzy + two-column rows); uses ClideAnchoredOverlay base only. Weakest fit; last or skip. +- Theme picker: its own small toggle+list content on the base (or ClideMenu if it cleanly fits). +Order: (1) fix the base blockers — focus capture racing content autofocus, and follower content untappable in the canSizeOverlay/zero-MediaQuery test harness (use View size for autoFlip; add a test path for anchored content) — with tests; THEN (2) ClideTypeahead + migrate slash + @; (3) theme picker; (4) quick-open last.', NULL, '2026-06-08 20:12:31', '2026-06-08 20:12:31', '2026-06-08 20:12:31', NULL, '4b5c423678c8c672ead79427b78e6448', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index 4459a6b3..e1e9a554 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -2321,3 +2321,19 @@ Blockers found (theme picker attempt): Then migrate, one surface per commit, keeping each existing test green (theme_picker widget_test, team_chat_sidebar_test, claude_composer_test, quick_open_overlay_test). Quick-open is centered (no follower) and the weakest-fit — it only needs the lifecycle wrapper; migrate last or skip. Refs: D-88, T-286, T-275 (done), menu bar (done, commit e1d51eb).', 'backlog', 'medium', NULL, NULL, NULL, '2026-06-08 17:52:51', '2026-06-08 17:52:51', NULL, '52f760eec6f108ce83041d7fd98f192e', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-288', 'task', 'T-286', 'Migrate remaining anchored surfaces onto ClideAnchoredOverlay (theme picker, typeaheads, quick-open)', 'Follow-up to the D-88 sweep. The primitive (ClideAnchoredOverlay + ClideMenu) shipped and the menu bar (T-286) + T-275 picker are migrated. The theme-picker migration was attempted and REVERTED after hitting two real blockers that affect all the remaining anchored surfaces; resolve these first, then migrate theme picker, @-mention typeahead, slash typeahead, and quick-open. + +Blockers found (theme picker attempt): +1. Focus-capture race. ClideAnchoredOverlay.captureFocus does a post-frame _scope.requestFocus(); ClideMenu also autofocuses its own node. Depending on mount timing (e.g. an extra ListenableBuilder wrapper) the scope wins and STEALS focus from ClideMenu''s node, so arrow/enter never reach the menu (Esc still works only via the ancestor fallback). Menu bar happens to win the race; the theme popover loses. Fix the primitive''s focus model so content''s own focus node reliably ends up focused (e.g. don''t force scope focus; or make the scope delegate to the autofocus child deterministically), then re-verify menu bar + anchored tests. +2. Follower content is not reliably mouse-tappable in the widget-test harness. The shared harness() uses Overlay(canSizeOverlay) + a zero-size MediaQuery, so an anchored follower positions the panel off-screen (observed tap offsets like (680,-45) and (680,654)) and tester.tap misses. Also autoFlip reads MediaQuery.size (zero in harness) so it can''t flip. Mitigations: have autoFlip use View.physicalSize (more correct anyway); position content without the inner Align (the panel shrink-wraps); and in tests drive selection via keyboard (as menu_bar does) or provide a real viewport. Consider a test helper for anchored-overlay content. + +Then migrate, one surface per commit, keeping each existing test green (theme_picker widget_test, team_chat_sidebar_test, claude_composer_test, quick_open_overlay_test). Quick-open is centered (no follower) and the weakest-fit — it only needs the lifecycle wrapper; migrate last or skip. + +Refs: D-88, T-286, T-275 (done), menu bar (done, commit e1d51eb). + +RE-SCOPE (2026-06-08, per D-88 amendment): the goal is NOT ''migrate everything onto ClideMenu''. The shared base is ClideAnchoredOverlay; content matches the surface: +- ClideMenu stays scoped to selectable-list menus (menu bar + permission picker — done). +- Add a NEW shared content component ClideTypeahead for the slash + @ typeaheads (near-duplicate caret-anchored completion surfaces) — they share this, not ClideMenu. This dedups two hand-rolls. +- Quick-open keeps bespoke content (centred filter + fuzzy + two-column rows); uses ClideAnchoredOverlay base only. Weakest fit; last or skip. +- Theme picker: its own small toggle+list content on the base (or ClideMenu if it cleanly fits). +Order: (1) fix the base blockers — focus capture racing content autofocus, and follower content untappable in the canSizeOverlay/zero-MediaQuery test harness (use View size for autoFlip; add a test path for anchored content) — with tests; THEN (2) ClideTypeahead + migrate slash + @; (3) theme picker; (4) quick-open last.', 'backlog', 'medium', NULL, NULL, NULL, '2026-06-08 17:52:51', '2026-06-08 20:12:31', NULL, '254250a0c8666da7273e17f4a9c0d8fb', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/governance/decisions/design.md b/governance/decisions/design.md index f432afc9..be485fd8 100644 --- a/governance/decisions/design.md +++ b/governance/decisions/design.md @@ -11,3 +11,9 @@ widget primitives. - **Rationale:** Nine surfaces had hand-rolled the same anchored-overlay + row-list + barrier + keyboard-nav pattern (menu-bar dropdowns, theme picker, slash + @ typeaheads, quick-open, three modal pickers), each re-deriving positioning, dismissal, and nav — divergent a11y, inconsistent dismissal, and a `pumpAndSettle`-hostile spread of ad-hoc overlays. Owning one primitive (per "own the rendering stack", [D-5](architecture.md#d-5-dart-core-sidecar-dissolved-ptyc-as-pql-peer)) makes the behaviour uniform and testable once, and turns the tenth surface (the T-275 permission-mode picker) into a few lines instead of another hand-roll. - **Cost:** A migration sweep across the existing surfaces (menu bar, theme picker, typeaheads, quick-open); the typeaheads keep their text-completion/key pipeline and only delegate anchoring + body, so the primitive must stay composable (a bare lifecycle wrapper + an optional turnkey menu), not a monolith. New UI authors must reach for the primitive rather than rolling another overlay. - **Raised by:** 2026-06-08 — user, while building the T-275 permission-mode picker: "since we don't do Material doesn't mean we can't make components of our own." Realises the "own the rendering stack" guardrail at the component level. Tracked by epic T-286. +- **Amendment (2026-06-08):** The shared piece is `ClideAnchoredOverlay` (positioning + lifecycle) — that is what every anchored surface adopts. Content is **NOT** universally `ClideMenu`; it matches the surface's shape, to avoid bending divergent surfaces into a menu (the theme-picker migration was reverted for exactly this friction): + - **`ClideMenu`** — selectable-list menus only (menu bar, permission picker). + - **`ClideTypeahead`** (a second shared content component, to add) — the slash and @ typeaheads are near-duplicate caret-anchored completion surfaces; they share *this*, not `ClideMenu`. + - **Quick-open / command palette** — bespoke content (centred filter + fuzzy + two-column rows); uses only the `ClideAnchoredOverlay` base. + - **Theme picker** — toggle + live-apply list; its own small content (or `ClideMenu` if it ever fits cleanly), on the base. + Revised goal: *everything anchored shares `ClideAnchoredOverlay`; content components match the surface.* The blockers to clear first (focus capture racing content autofocus; follower content untappable in the `canSizeOverlay` test harness) live in the base, not in any content component. Tracked by T-288. Raised by the user: "should those then not just be a different shared component from the others?"