fix(staged): layer dialogs and floating UI above legacy overlays#770
Open
matt2e wants to merge 4 commits into
Open
fix(staged): layer dialogs and floating UI above legacy overlays#770matt2e wants to merge 4 commits into
matt2e wants to merge 4 commits into
Conversation
The shadcn migration hardcoded dialog and alert-dialog overlays/content to z-50, but these render through a Portal at document.body. Legacy full-screen overlays (e.g. .diff-modal-backdrop at z-index 1000) sit above them, so any shadcn dialog opened over a legacy overlay — such as the commit/note dialog from a review comment in the diff viewer — painted underneath and was invisible/unclickable. Bump the dialog and alert-dialog overlay/content layers to z-[10000], above the legacy max (9999), so the design system owns the top layer and the whole class of bug is resolved. Re: #note:9bcdcc33-7dc9-4bdc-998c-12ba433e54db Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…itives The prior fix hardcoded dialog/alert-dialog overlays and content to z-[10000] to clear the legacy overlay max (9999), but left every other portaled primitive — dropdown-menu, context-menu, popover, select, tooltip, and the HashtagInput dropdown — at z-50 (or a literal 9999). Any of those opened from inside a dialog then painted behind the dialog content, re-introducing the same layering bug one level up. Introduce two semantic tokens in a non-inline @theme block so they emit real :root custom properties: --z-index-overlay (10000) for the dialog layer that owns the top above legacy overlays, and --z-index-floating (10001) for portaled primitives, one above overlay so they always paint over dialog content. Swap the dialog/alert-dialog z-[10000] to z-overlay, the floating primitives' z-50 to z-floating, and HashtagInput's literal 9999 to var(--z-index-floating) so the whole stacking order is driven from one place. Re: #note:c3640d8c-bab7-4f59-b759-9c04faf10a71 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
bits-ui sets `body { pointer-events: none }` while a dialog is open to
scope interaction to the dialog. The HashtagInput dropdown portals to
document.body, so it inherited the dead pointer events and was visible
but mouse-unresponsive when opened from inside a dialog — items could
not be clicked or hovered.
Set `pointer-events: auto` on .hashtag-dropdown so the portaled dropdown
remains interactive regardless of the body-level lock.
Re: #note:9a12d28e-b4e5-45fa-96bb-1ae8b03c7a23
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5ad469596
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+346
to
+347
| --z-index-overlay: 10000; | ||
| --z-index-floating: 10001; |
There was a problem hiding this comment.
Tailwind's z-index utilities are z-<number>, z-[<value>], or z-(<custom-property>); the theme namespace list does not include --z-index-*, so these variables do not create z-overlay/z-floating classes. In the inspected dialog/dropdown/select/popover/etc. components, replacing z-50 with those class names leaves the portaled elements with no generated z-index rule, so they can still render behind the existing DiffModal layers (.diff-modal-backdrop is z-index 1000) instead of above them. Use z-(--z-index-overlay)/z-(--z-index-floating) or arbitrary values so the classes actually emit CSS.
Useful? React with 👍 / 👎.
matt2e
changed the title
…rimitives The prior fix relied on Tailwind generating `z-overlay` / `z-floating` utilities from the `--z-index-*` theme tokens. That works on the resolved Tailwind version (4.2.3, where the `z` utility reads the `--z-index` namespace), but the package range is pinned `^4.0.0`, so the namespace-resolution behavior isn't guaranteed across every version the range permits. Switch the nine portaled primitives (dialog, alert-dialog, dropdown, select, popover, tooltip, context-menu overlays/content) from the namespaced `z-overlay` / `z-floating` classes to the explicit arbitrary-property form `z-(--z-index-overlay)` / `z-(--z-index-floating)`. Both compile to identical `z-index: var(--z-index-…)` output, but the arbitrary-property utilities emit regardless of namespace support, so the top-layer stacking order is immune to any future Tailwind change. Update the app.css token comment to match the new utility form. Re: #note:3d96f212-9c09-4231-899a-dcc437197b80 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.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
Fixes stacking and interaction issues that surfaced after the shadcn-svelte migration, where dialogs and floating primitives could render beneath legacy overlays and the hashtag dropdown became unclickable inside dialogs.
app.cssand apply it to shadcn dialog, alert-dialog, popover, select, dropdown-menu, context-menu, and tooltip content/overlay components so they render above legacy overlays.HashtagInput.svelte) so it remains interactive when rendered inside dialogs.Test plan