UI refactor P1: OverlayManager - one owner for every popup/dialog lifecycle#98
Merged
Merged
Conversation
…a permanent gate First phase of the UI refactor plan (doc/UI_QA_AUDIT.md section 7). Foundation - graphlink_styles.py is THE token source (recorded decision: extending the existing THEME_TOKENS/runtime-injection pipeline instead of introducing a parallel JSON file that would fork truth): - STRUCTURE_TOKENS: theme-independent scales - spacing on a 4px grid (--gl-space-1..8), radius sm/md/lg, type ramp 12/13/14/16/20 (nothing under 12px - audit finding D2), three elevation shadow levels (finding B9), motion 150/200ms + easing (finding F4). Exported to every island and registered under Tailwind v4's PROPER namespaces (spacing/radius/ text/font-weight/shadow/duration/ease - not bogus color utilities), with Qt-side integer mirrors (SPACE_PX/RADIUS_PX/TEXT_PX/ELEVATION_PARAMS) parsed from the same dict so the two representations cannot drift. - New per-theme "surface" group: the clean neutral role vocabulary (window/node_body/inset/inset_deep/field/border/border_strong/divider/ handle/handle_hover/chrome_inactive + a 6-tier text ramp). Dark values equal the exact hexes that dominated the swept files, so the migration is essentially byte-neutral in the default theme; mono/muted derive from each theme's existing neutrals. - New per-theme "syntax" group: PythonHighlighter's palette relocated from graphlink_pycoder.py so code-node colors theme with everything else. - FONT_FAMILY_NAME (bare name for QFont) beside the existing QSS stack; get_surface_color()/get_syntax_color() lookups in graphlink_config. Sweep - ~500 hardcoded hex literals and ~40 string-pasted "Segoe UI" occurrences across ~35 UI files replaced with token lookups (4 parallel sweep passes + a centrally-adjudicated finalization: near-role neutrals deduped onto the role scale within a sanctioned sub-perceptual tolerance; semantic/data colors either matched byte-exact to existing semantic roles, promoted into the new groups, or explicitly allowlisted). Module-level scrollbar style constants became functions so stylesheets resolve the LIVE theme instead of freezing at import. Persisted scene-data defaults (container color) deduped onto one documented constant. Gate - tests/test_ui_token_acceptance.py codifies the P0 acceptance criterion permanently: no hex literal and no "Segoe UI" outside graphlink_styles.py, excepting a small reasoned allowlist (export CSS, user-pickable presets, persisted data defaults, the per-theme QMenu table that phase P8 retires); plus structure-scale conformance (4px grid, 12px type floor, 3 elevation levels). Verified: full pytest 1742 passed (acceptance gate green, export round-trip/count/shape tests updated for the new groups); vitest 533 passed; all 19 islands rebuilt against the extended token sheet; real app launch clean (window up, zero log errors). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ecycle Second phase of the UI refactor plan (doc/UI_QA_AUDIT.md section 7). The audit's section-2 findings were a class, not incidents: this phase removes the class. New graphlink_overlay_manager.py (OverlayManager + DialogScrim): - Named-surface registry with POPOVER/DIALOG tiers; single-open policy across both tiers - opening anything closes whatever else is open (audit B1: Plugins+Controls+Settings+Library used to stack freely). - One QApplication event filter: Escape closes the open surface; a mouse press outside an open popover light-dismisses it without eating the click (audit B2). Dialogs dismiss via close button/scrim/Escape only, preserving the legacy stay-open-during-scans rationale. - DialogScrim: translucent full-window layer under dialogs - visible modality, guaranteed z-order, click-to-dismiss. - visibility_changed signal -> toolbar payload's new activeSurface field: chips render active from REAL visibility, never local latched click state (audit B6; web side: App.tsx reads payload, local latch removed, schema regenerated, island rebuilt). - Two manager bugs found by this phase's own acceptance tests and fixed: close() captures state BEFORE close_fn() because hide delivers Show/HideEvents synchronously through the manager's own app filter; and the filter never mutates manager state for the same reason. Settings + Library converted from escaping top-level windows to clamped embedded children (audit B4): - SettingsWebHost: Tool flags + screen-coordinate show_for_anchor DELETED; now wrapped in the new token-styled DialogFrame (title + close button - audit B5) and centered/clamped by the manager. - ChatLibraryDialog: frameless stay-on-top per-open window -> cached embedded child (explicit Qt.Widget flags: QDialog defaults to a Dialog WINDOW flag even with a parent, which briefly made it an invisible translucent top-level - found live). The fresh-window-per-open construction race WAS audit B3's first-click no-op; first click now opens reliably (live-verified). THE BIG ONE - audit B7 root-caused and fixed: the library's "black void" body was the QGraphicsDropShadowEffect on its shell. A QGraphicsEffect on any ancestor forces software rendering that the GPU-composited QWebEngineView cannot join, so the entire web region painted as a solid black rectangle. Same effect removed from DialogFrame (where it initially reproduced identically on Settings). Live-verified: library now renders its full chat list (19 chats, search, CRUD row) on first open; settings renders all sections. No ancestor of a webview may carry a QGraphicsEffect. Escape delivery over web content: QtWebEngine feeds keys straight into the render process - app filter, QShortcuts, and ShortcutOverride interception never fire (each verified live). Added a host-injected page-side relay (capture-phase keydown -> titleChanged -> escape_pressed signal, zero bridge/schema/rebuild cost) wired to close_all for every island host. End-to-end verified in-process (synthetic page keydown closes the surface); real-OS-key confirmation pending one human keystroke - the automation tool's synthetic Escape also failed against native Qt menus pre-refactor, implicating the tool's key delivery rather than app code. All toolbar handlers (library/settings/about/help/plugins/pins/controls) route through manager.toggle(); resize re-clamps the open dialog + scrim. Tests: 22 new acceptance tests including a REAL ChatWindow drive (single-open across real surfaces, Escape per surface, no dialog pixel outside the window at 3 window sizes, library-first-open). Two stale settings-host tests rewritten to pin the embedded contract. Full pytest 1766 passed; vitest 535 passed; live drive on Windows verified all section-2 behaviors visually. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase P1 of the UI refactor plan. Contains P0 (#97) - GitHub refused the stacked base; merge #97 first and this PR auto-shrinks to the single P1 commit.
Every transient surface's open/dismiss lifecycle now routes through one
OverlayManager- killing the audit's entire section-2 defect class:keydown→titleChanged→ signal; QtWebEngine feeds keys straight to the render process, so filters/QShortcuts/ShortcutOverride never fire over web focus - each verified live). In-process end-to-end proof green; one human keystroke pending for real-OS confirmation (the automation tool's synthetic Escape also failed against native Qt menus pre-refactor, implicating the tool).DialogFrameprovides the mandatory title + close button.activeSurfacepayload field; toolbar island renders active from payload, local latch deleted.QGraphicsDropShadowEffecton its shell - a QGraphicsEffect on any webview ancestor forces software rendering the GPU-composited view can't join → solid black rectangle. Removed here and in DialogFrame (where it reproduced on Settings). Library now renders its full 19-chat list on first open; Settings renders all sections - live-verified.Test plan
Phase P2 (island sizing contract) follows.