Skip to content

Fix schedule builder Radix regressions#4669

Draft
marcoacierno wants to merge 3 commits into
mainfrom
fix/schedule-builder-radix-regressions
Draft

Fix schedule builder Radix regressions#4669
marcoacierno wants to merge 3 commits into
mainfrom
fix/schedule-builder-radix-regressions

Conversation

@marcoacierno

@marcoacierno marcoacierno commented Jun 10, 2026

Copy link
Copy Markdown
Member

Summary

Fixes regressions introduced by #4664 (Radix UI migration) in the schedule builder.

1. Items longer than their slot no longer stretched for the entire duration

The old markup painted the full grid-row span via bg-slate-200 on both the wrapper div and the inner <ul>. The migration removed the wrapper background and replaced the <ul> with a Radix Card, which only takes its natural content height — so a multi-slot item rendered as a short card at the top of its span.

Fix: make the Card fill its grid area with height: 100%. The other ScheduleItemCard usage (pending items basket) is unaffected since its parent has auto height.

This also removes the invisible dead zone: the item wrapper spans the full duration with z-50, so the placeholders visible "through" it were not clickable.

2. Drop/Add cells sometimes unresponsive (no hover, no click)

DjangoAdminEditorModal returned null as soon as url was cleared, unmounting the open Radix Dialog mid-close. A modal Radix Dialog sets pointer-events: none on <body>, and an abrupt unmount can leave it stuck — after closing an "Edit" dialog the whole page stops responding to hover/clicks until reload.

Fix: keep Dialog.Root mounted and drive it via the open prop (same pattern AddItemModal already uses), guarding only the iframe on url.

3. "Type" dropdown options in the "Add event to schedule" modal open behind the dialog

Before the migration this was a native <select>, which the browser always renders on top. The Radix Select content is portalled with no z-index (Radix Themes stacks portals by DOM order), while custom-styles.css gives the dialog overlay an explicit z-index: 30 — so the options painted behind the modal and could not be clicked.

Fix: raise the dialog overlay to z-index: 500 (the old shared Modal level, also covering the schedule grid's z-[100] sticky headers, which otherwise paint over the open dialog) and give popper content (Select, DropdownMenu, Popover, Tooltip) z-index: 510 so it stacks above open dialogs. The popper wrapper copies the content's computed z-index, so class rules are sufficient.

🤖 Generated with Claude Code

PR #4664 replaced the item's <ul> with a Radix Card and dropped
bg-slate-200 from the grid wrapper, so items longer than their slot
rendered at content height instead of filling the spanned rows.
Make the Card fill its grid area with height: 100%.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pycon Error Error Jun 11, 2026 12:03am

@claude

claude Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Fixes three UI regressions from the Radix migration (#4664): card height in the schedule grid, stuck pointer-events: none on <body> after modal close, and Select options rendering behind the dialog overlay.

z-index escalation scope

.rt-PopperContent / .rt-SelectContent / .rt-TooltipContent at z-index: 510 in custom-styles.css are global selectors — they raise every Radix popper in the entire app, not just those opened from inside a dialog. This is the canonical Radix Themes workaround, but it's worth auditing whether any popper-using components outside the schedule builder now float above things they shouldn't (e.g., sticky table headers, other modals, sidebars). If all admin popper usage happens in flat stacking contexts this is fine; if not, scoping to .rt-DialogContent .rt-PopperContent would be safer — though Radix portals to <body> so descendant selectors won't work there without a data-attribute approach.

baseUrl computed when url is null

After removing the early-return in DjangoAdminEditorModal, baseUrl is evaluated on every render even when the dialog is closed. It's a cheap document.location read so there's no real cost, but moving it inside the {url && ...} block (or memoising with useMemo) keeps intent clearer.

No new tests — the three regressions are all visual/interaction bugs (layout, pointer-events, z-index) that are hard to cover with unit tests, so this is acceptable provided manual QA covers the schedule builder end-to-end (drag items, open/close Edit dialogs multiple times, verify Add-event type dropdown).

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.52%. Comparing base (2832fe6) to head (e4e5843).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4669   +/-   ##
=======================================
  Coverage   92.52%   92.52%           
=======================================
  Files         359      359           
  Lines       10800    10800           
  Branches      821      821           
=======================================
  Hits         9993     9993           
  Misses        696      696           
  Partials      111      111           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

DjangoAdminEditorModal returned null as soon as url was cleared, which
unmounts the open Radix Dialog mid-close. A modal Radix Dialog sets
pointer-events: none on <body> and an abrupt unmount can leave it
stuck, making the whole schedule builder (e.g. the Drop/Add cells)
unresponsive to hover and clicks until reload.

Keep Dialog.Root mounted and drive it via the open prop — same pattern
AddItemModal already uses — guarding only the iframe on url.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "Type" select in the "Add event to schedule" modal opened its
options behind the dialog: the Radix Select content is portalled with
no z-index (stacking by DOM order), while custom-styles.css gives the
dialog overlay an explicit z-index. Before #4664 this was a native
<select>, which the browser always renders on top.

Raise the overlay to 500 (the old shared Modal level) so dialogs also
cover the schedule grid's z-[100] sticky headers, and give popper
content (Select, DropdownMenu, Popover, Tooltip) z-index 510 so it
stacks above open dialogs. The popper wrapper copies the content's
computed z-index, so class rules are sufficient.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant