fix(#5250): let settings search results escape the menu clip - #5254
2 commits merged into
Conversation
|
| Filename | Overview |
|---|---|
| static/index.html | Wraps the eight settings section buttons in a new .settings-menu-items div; structurally clean, preserves all data-settings-section and onclick attributes. |
| static/style.css | Overrides .side-menu's overflow-y:auto on #settingsMenu with overflow:visible, delegates scrolling to #settingsMenu .settings-menu-items; cascade and specificity are correct. |
| tests/test_3850_settings_search.py | Adds test_settings_menu_layout_ownership_contract with properly anchored full-selector regex patterns for both #settingsMenu and #settingsMenu .settings-menu-items. |
| tests/test_issue5250_settings_search_dropdown_escape.py | New Playwright regression test that renders a minimal reproduction page and uses elementFromPoint to verify the dropdown is hit-testable below the menu's clipping boundary; gracefully skips if Playwright is unavailable. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["#settingsMenu.side-menu\n(overflow: visible ← new)"] --> B[".settings-search\n(position: relative)"]
A --> C[".settings-menu-items\n(overflow-y: auto ← new)"]
B --> D["input#settingsSearch"]
B --> E["#settingsSearchResults\n.settings-search-results\n(position: absolute; top:100%; z-index:200)"]
C --> F["button.side-menu-item × 8\n(scroll within .settings-menu-items)"]
E -. "escapes clip boundary" .-> G["Visible below #settingsMenu"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["#settingsMenu.side-menu\n(overflow: visible ← new)"] --> B[".settings-search\n(position: relative)"]
A --> C[".settings-menu-items\n(overflow-y: auto ← new)"]
B --> D["input#settingsSearch"]
B --> E["#settingsSearchResults\n.settings-search-results\n(position: absolute; top:100%; z-index:200)"]
C --> F["button.side-menu-item × 8\n(scroll within .settings-menu-items)"]
E -. "escapes clip boundary" .-> G["Visible below #settingsMenu"]
Reviews (2): Last reviewed commit: "test(#5250): harden settings menu select..." | Re-trigger Greptile
🎬 Cutter preview — PR #5254 |
🔬 Gate certification — GREEN ✅ (visible CSS fix; clip-escape verified live via getComputedStyle)Certified head: What I ran (isolated worktree
|
| Gate | Result |
|---|---|
| Codex (reproduce) | SAFE TO SHIP (no findings — clip-escape works, no menu-scroll/layout/selector regression, z-index correct) |
Full pytest suite (-p no:xdist) |
11157 passed, 0 failed |
PR's tests (test_issue5250 + test_3850_settings_search) |
24 passed |
| Live verification (getComputedStyle) | #settingsMenu overflow:visible ✓ · .settings-menu-items overflow-y:auto ✓ · .settings-search-results position:absolute, z-index:200, display:block, renders results ✓ |
Findings (clean)
- ✅ Correct clip-escape pattern — the fix wraps the settings side-menu buttons in a new
.settings-menu-itemsflex container that takes theoverflow-y:autoscrolling, and sets#settingsMenutooverflow:visible. So the absolutely-positioned.settings-search-resultsdropdown (top:100%, z-index:200) now overlays outside the scroll container instead of being clipped by it. Verified live: menuoverflow:visible, inner wrapperoverflow-y:auto, dropdownposition:absolute/z-index:200/display:blockrendering results. - ✅ No menu-scroll regression — scrolling moved cleanly to the inner
.settings-menu-itemscontainer (the side-menu still scrolls; only the clip boundary moved). Codex confirmed no layout regression. - ✅ DOM reparent safe — the
side-menu-itembuttons moved into the new wrapper retain theirdata-settings-section+switchSettingsSection(...)handlers; the PR's test hardens the selector contract so this can't silently break. Codex confirmedswitchSettingsSection/selectors intact. - ✅ Fresh-based on current master; 24 settings-search tests pass.
Note: I attempted a headless screenshot of the open dropdown, but the settings modal didn't paint in the headless-driven state (0×0 rects / no PNG). The fix is a low-visual-risk CSS clip-escape (the "after" simply shows the previously-clipped dropdown rows), and I verified the exact overflow/position contract live via getComputedStyle + Codex. If a visual glance is wanted before merge, opening Settings → typing in the search box on a running instance shows the dropdown overflowing the menu cleanly.
Concept: 4/5 — fixes a real usability bug (#5250: settings-search results clipped/hidden by the menu's overflow); existing-flow polish.
Recommendation to the next agent
Ready to merge — cert fresh for sha:c2a0472514d7. crit=3 CSS clip-escape for the settings-search dropdown; Codex SAFE, full suite green, 24 tests, the overflow restructure (menu visible + inner-wrapper auto + dropdown absolute/z-200) verified live via getComputedStyle, DOM reparent preserves switchSettingsSection/selectors. Fresh-based. Low visual risk (clip-escape). Concept 4/5. Cert valid only at sha:c2a0472514d7.
Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy/close. Cert valid only at sha:c2a0472514d7; a new push invalidates it → re-gate.
8d5ff4c


Thinking Path
.side-menumakes#settingsMenuscroll, and the absolute dropdown is clipped by that boundary.What Changed
static/index.html: wraps the Settings section buttons in a.settings-menu-itemscontainer below the existing search shell.static/style.css: keeps the shared.side-menurule unchanged, makes#settingsMenuoverflow visibly, and gives only.settings-menu-itemsvertical scrolling.tests/test_3850_settings_search.py: updates the searchable-settings layout contract without changing the search semantics it already covers.tests/test_issue5250_settings_search_dropdown_escape.py: adds a focused browser-level regression proof for the clipped dropdown case.Why It Matters
Users can now see and click Settings search results even when the dropdown extends past the visible menu boundary. The existing match list, result cap, and click-to-setting flow stay intact.
Verification
pytest tests/test_3850_settings_search.py tests/test_issue5250_settings_search_dropdown_escape.py -v --timeout=60If present after restacking with the ranking work from #5211:
pytest tests/test_5149_settings_search_ranking.py -v --timeout=60Manual browser proof fallback if Playwright is unavailable locally: open Settings, type a query with multiple matches, confirm a result remains visible and hit-testable below the menu boundary, then click it and confirm the target section switches and highlights the field.
Full-suite CI context, not a required local check unless explicitly run:
pytest tests/ -v --timeout=60.Upstream
Closes #5250.
Model Used
GPT 5.5 via Codex CLI