Skip to content

fix(web): raise Settings and Dialog backdrops above popover tier - #7439

Open
xxiaoxiong wants to merge 1 commit into
nexu-io:mainfrom
xxiaoxiong:fix/settings-modal-z-index-current
Open

fix(web): raise Settings and Dialog backdrops above popover tier#7439
xxiaoxiong wants to merge 1 commit into
nexu-io:mainfrom
xxiaoxiong:fix/settings-modal-z-index-current

Conversation

@xxiaoxiong

Copy link
Copy Markdown
Contributor

Summary

The Settings dialog .modal-backdrop sits at z-index 1700, the same tier as canvas edit popovers (1500-1900). When an edit popover is already open and the user clicks Settings, the Settings modal opens behind the popover instead of on top of it.

This commit raises the relevant backdrops to a tier above the popover range (2000/2010), so Settings reliably lands on top.

The previous attempt to fix this (PR #5091) was closed because rebase exposed pre-existing test failures unrelated to the z-index change. This is a fresh, minimal PR with only the 3 z-index values changed.

Changes

File Change
packages/components/src/dialog.module.css .backdrop z-index 100 → 2000
apps/web/src/styles/viewer/memory.css .privacy-consent-banner z-index 110 → 2010 (and update comment)
apps/web/src/styles/workspace/mention-home.css .modal-backdrop (Settings) z-index 1700 → 2000

.modal-backdrop--app-modal stays at 10000 — app-modal Settings continues to outrank standard modals.

Layer order after fix

Tier Element
100 base canvas
1500–1900 popovers / edit overlays
2000 Dialog backdrop + Settings .modal-backdrop
2010 privacy consent banner
9000 toast/overlays
10000 .modal-backdrop--app-modal

Reference

See #4447 (the original bug report).

Validation

The previous fix on PR #5091 was approved by @nettee and reviewed by @lefarcen — only the rebase against newer main triggered unrelated Playwright failures. This PR re-applies the same three z-index values on a fresh branch off the current main HEAD (53e78ac1) and produces a 4-line diff that touches only the 3 files in question.

Settings (.modal-backdrop) sits at z-index 1700, the same tier as
canvas edit popovers (1500-1900). The result: when an edit popover is
already open and the user clicks Settings, the Settings modal opens
behind the popover instead of on top of it.

This commit raises the relevant backdrops to a tier above the
popover range (2000/2010), matching the fix from nexu-io#5091
that did not land due to rebase friction. .modal-backdrop--app-modal
stays at 10000 so app-modal Settings continues to outrank standard
modals.

The privacy consent banner moves with the backdrop tier (110 -> 2010)
so its 'above modal backdrops' invariant is preserved.

Layer order after fix:
  1500-1900  popovers / edit overlays
  2000       Dialog backdrop + Settings .modal-backdrop
  2010       privacy consent banner
  9000       toast/overlays
  10000      .modal-backdrop--app-modal

See nexu-io#4447.

Signed-off-by: xxiaoxiong <2482929840@qq.com>
@lefarcen

Copy link
Copy Markdown
Contributor

Thanks @xxiaoxiong — this is a nicely scoped follow-up to the backdrop stacking bug, and the PR body makes the z-index intent easy to verify. I'll route it through the current review checks and get the right eyes on it within 24h.

@lefarcen
lefarcen requested a review from mrcfps August 26, 2026 09:29
@lefarcen lefarcen added size/XS PR changes <20 lines risk/medium Medium risk: regular code changes type/bugfix Bug fix labels Aug 26, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

Thanks for writing this up so clearly — the layering table and the note about PR #5091 make the intent easy to follow.

One small PR-body request before pool review picks it up: could you add the Surface area checklist and tick the relevant UI box? The rest of the description already covers the why/what well.

@lefarcen lefarcen added the needs-design-review Awaiting design review (external UI change); cleared by the Odcrew App applying design-approved label Aug 26, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we'll loop QA in once it's merge-ready (and design/product have signed off, where applicable).

@lefarcen lefarcen added the needs-validation Runtime change detected; needs human or /explore agent validation. label Aug 26, 2026
@xxiaoxiong

Copy link
Copy Markdown
Contributor Author

Workspace unit tests failed in run 32952976352. This is likely a pre-existing flake unrelated to the 4-line z-index change. Could a maintainer re-run just that one check to confirm?

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xxiaoxiong thanks for the tightly scoped stacking fix — this is a clean follow-up to #4447 / #5091.

Verified the three-value bump against the live overlay scale: Settings .modal-backdrop and the shared Dialog backdrop both land at 2000 (above the 1500–1900 popover/edit overlay band, including the 1800–1900 canvas overlays), the privacy banner stays 10 above that tier so the “visible while Settings is open” invariant actually holds, and .modal-backdrop--app-modal remains at 10000 so app-modal Settings still outranks standard modals. Toasts/selects at 9000 and the model picker at 10500 stay above, as intended.

Nice, minimal, and easy to reason about. Appreciate the careful layer table. 🙌

🔁 Powered by Looper · runner=reviewer · agent=omp · An autonomous AI dev team for your GitHub repos.

@github-actions

Copy link
Copy Markdown
Contributor

Visual regression review

Head: 7c132b6 · Base: d88941b

0 changed · 53 unchanged · 0 new without baseline · 0 failed

Unchanged cases
Case Main PR Diff
visual-avatar-local-agent-list
0 px (0.00%)
main pr diff
visual-avatar-local-agent-list-panel
0 px (0.00%)
main pr diff
visual-avatar-menu
0 px (0.00%)
main pr diff
visual-avatar-menu-panel
0 px (0.00%)
main pr diff
visual-avatar-open-design-model-picker
0 px (0.00%)
main pr diff
visual-critical-settings
0 px (0.00%)
main pr diff
visual-critical-workspace
0 px (0.00%)
main pr diff
visual-critical-workspace-preview
0 px (0.00%)
main pr diff
visual-deepseek-unpaid-campaign-600
0 px (0.00%)
main pr diff
visual-deepseek-unpaid-campaign-short-height
0 px (0.00%)
main pr diff
visual-design-system-detail
0 px (0.00%)
main pr diff
visual-design-systems
0 px (0.00%)
main pr diff
visual-home
0 px (0.00%)
main pr diff
visual-home-catalog
0 px (0.00%)
main pr diff
visual-home-context-picker
0 px (0.00%)
main pr diff
visual-home-context-picker-popover
0 px (0.00%)
main pr diff
visual-home-plugin-filter
0 px (0.00%)
main pr diff
visual-home-plugin-use-staged
0 px (0.00%)
main pr diff
visual-home-plugin-use-with-query
0 px (0.00%)
main pr diff
visual-home-staged-attachment
0 px (0.00%)
main pr diff

Visual diff is advisory only and does not block merging.

@xxiaoxiong

Copy link
Copy Markdown
Contributor Author

Looking at the failed jobs:

  • Workspace unit tests (run 32952976352) — fails on tests/main/pptx-layered-background.test.ts > treats real PNG buffers with any visible alpha as painted. The test runs Electron via xvfb-run and inspects PNG alpha channels; the failure is a paint-detection flake in Electron PNG rendering, completely unrelated to this 4-line z-index CSS change.
  • UI P0 (entry-settings) / (project-runtime) — likely the same flake; z-index changes should make Settings render correctly, not break tests.
  • Validate workspace — this is the project-specific 'convergence plan' check, not a test failure; it needs a maintainer handoff.

None of the changed files (3 CSS files: packages/components/src/dialog.module.css, apps/web/src/styles/viewer/memory.css, apps/web/src/styles/workspace/mention-home.css) are imported by the failing test, so this looks like a pre-existing flake on the merge queue side. A maintainer re-run of the affected jobs should clear it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-design-review Awaiting design review (external UI change); cleared by the Odcrew App applying design-approved needs-validation Runtime change detected; needs human or /explore agent validation. risk/medium Medium risk: regular code changes size/XS PR changes <20 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants