bugfix(modal): stack body portals by open order, not mount order (CUI-43) by JeanMarcMilletScality · Pull Request #1186 · scality/core-ui

JeanMarcMilletScality · 2026-08-26T08:11:31Z

TL;DR

Two open Modals stacked by the order they mounted in, not the order they were opened in, so the modal a user opened last could be painted underneath another one with its buttons unclickable. The portal host is now attached when the modal opens — and appended rather than prepended — so <body> order is open order. Drawer had the identical defect and is fixed the same way.

Context

Reported by a consuming application that composes two independently-owned Module Federation remotes, each auto-opening a modal on load: the modal that opened second painted underneath the first and swallowed clicks on its own close button. Long-standing bug, not a regression from any recent dependency bump — the bump only shifted remote-load timing enough to lose a pre-existing race consistently. The consumer worked around it in its own test suite; this fixes the library. See CUI-43 for the internal report.

Approach

Every ModalContainer gets the same z-index: 8500, so paint order among open modals is decided purely by <body> order — and the host was prepended at mount, which puts the newest host first, where it paints below its siblings. Net effect: the earliest-mounted modal always won, and a mounted-but-closed modal still reserved a slot it never used.

Before:

const modalContainer = useRef(document.createElement('div'));

useLayoutEffect(() => {
  document.body && document.body.prepend(modalContainer.current);  // ←
  return () => {
    document.body && document.body.removeChild(modalContainer.current);
  };
}, [modalContainer]);   // ← stable ref ⇒ runs once, at mount, never gated on isOpen

After:

const modalContainer = useRef(document.createElement('div'));

useLayoutEffect(() => {
  if (!isOpen) {                    // ←
    return;
  }
  const host = modalContainer.current;
  document.body?.append(host);      // ←
  return () => {
    host.remove();
  };
}, [isOpen]);                       // ←

Drawer is gated on mounted rather than isOpen, because mounted spans the closing transition — gating on isOpen would yank the node out mid-animation. document.body.removeChild(host) also became host.remove(), which does not throw if the node has already moved.

Why not the incrementing z-index the ticket recommended

CUI-43 proposed three fixes and called the third — assign an incrementing z-index when a modal opens — the most robust. It isn't, for this bug: a module-level counter in core-ui is not shared across Module Federation remotes. A federated host typically shares most dependencies non-singleton and lists only a chosen few as singletons; @scality/core-ui is not among the singletons in the host configuration checked here. So two remotes can each hold their own core-ui instance with its own counter, both starting at the same value — colliding straight back into DOM order, which is exactly the reported scenario. The DOM is the only coordination surface genuinely shared across remotes, so ordering stays keyed on <body> order and zIndex.modal remains a single shared constant.

Measured

Real browser, elementFromPoint on each footer button. A always mounts first:

	`<body>` order	paints on top	last-opened's `Confirm`
before, open A→B	`[B, A]`	A (mounted first)	❌ intercepted by A
after, open A→B	`[A, B]`	B	✅ clickable
after, open B→A	`[B, A]`	A	✅ clickable

Tracking both directions is the point: it shows the order follows open order, not merely a reversed mount order.

The sharpest user-facing symptom isn't the visual one. The focus effect is keyed on isOpen, so focus went to the modal that opened last while paint went to the one that mounted first — and both containers carry aria-modal="true", so assistive tech announced a dialog the user could not see and Tab walked controls hidden behind another modal's overlay:

open A then B	painted on top	holds focus	agree
before	A	B	❌
after	B	B	✅

Audit of the other `document.body` portals

CUI-43 asked for this. Tooltip needs no fix: its overlay portals straight to document.body and only while visible, so React appends it at show time (already the safe polarity) at z-index: 9990, above modal and drawer. The ticket's concern that Button wraps every button in a Tooltip is accurate (Buttonv2.component.tsx:370, unconditional) but harmless — with no overlay nothing is ever portalled, and TooltipContainer renders inline.

Usage

No API change. The guarantee the fix adds, as the new StackedModals story exercises it:

// A is declared first, so it always mounts first — stacking now follows
// the order the modals were opened in instead.
<Modal isOpen={isAOpen} close={() => setIsAOpen(false)} title="Modal A" actions={…}>…</Modal>
<Modal isOpen={isBOpen} close={() => setIsBOpen(false)} title="Modal B" actions={…}>…</Modal>

Open B then A, and A is on top with its buttons live. Before, A was on top either way.

Review focus

🟡 src/lib/components/modal/Modal.component.tsx › the useLayoutEffect — a behaviour change on every Modal in every consumer. The bit worth confirming is effect ordering: the layout effect must attach the host before the isOpen focus effect runs, which is what keeps auto-focus working (layout effects run first, so it holds — but it's load-bearing).
🟡 src/lib/components/drawer/Drawer.component.tsx › the useLayoutEffect — gated on mounted, not isOpen, deliberately. If that call is wrong, the drawer loses its node mid closing-transition.
⚪ stories/Modal/modal.stories.tsx › StackedModals — new story; doubles as the manual repro.

How to test

No screenshot: the change is a stacking order, so the meaningful evidence is which element takes the click. Reproduce it directly —

npm run storybook → Components / Feedback / Modal / Stacked Modals.
Click Open A, then Open B. B is on top and Confirm B takes the click.
Reload, click Open B, then Open A. Now A is on top and Confirm A takes the click.
On development/1.0, both orders leave A on top, and in step 2 Confirm B does nothing.
With both modals closed, document.body holds no leftover empty host divs — before, there was one per mounted Modal whether or not it ever opened.

Follow-up

Nothing enforces "one modal at a time", and in the reported case no consumer could: the two modals come from different MF remotes owned by different teams, both auto-opening on load. A dev-only warning is the sane guard, and it would have to count .sc-modal nodes in the document rather than use module state — same cross-remote reason that ruled out the incrementing z-index above. Not in this PR.
src/lib/components/charts/common/ChartTooltip.tsx:178 creates its portal host with appendChild at mount — mount-time rather than show-time, but append is the safe polarity and it declares no z-index of its own. Left alone.
Modal registers its Esc handler on document per open modal, so Esc closes every open modal at once. Pre-existing, untouched here.

References

CUI-43 — "Modal stacking is inverted and keyed on mount order, not open order". Bug / Severity Major / Impact Internal. Carries the downstream report and the consumer-side workaround this replaces at the library level.

What changed

Modal.component.tsx and Drawer.component.tsx each move their portal-host insertion out of a mount-time effect and into one gated on being open, switching prepend for append. Nothing else in either component changes — no props, no styles, no z-index values.

Modal.test.tsx is new and asserts the four cases in DOM terms (jsdom cannot paint): no host while closed, open-beats-mounted-first, two opens ordered by open order, and re-opening raising a modal back to the top. Three equivalent cases were added to the existing Drawer.component.test.tsx. All seven were confirmed to fail against development/1.0 before the fix.

Deliberately not in this PR: making zIndex.modal per-instance (unnecessary once DOM order encodes open order), any enforcement of a single-modal rule, and the ChartTooltip mount-time host.

🤖 Generated with Claude Code

bert-e · 2026-08-26T08:11:36Z

Hello jeanmarcmilletscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request.
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2026-08-26T08:11:42Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

…-43) Modal created its portal host on mount and `prepend`ed it to <body>, so with one shared z-index the earliest-mounted modal always painted on top — and a mounted-but-closed modal reserved a stacking slot it never used. Host the portal only while open and `append` it, so <body> order is open order. Drawer had the identical defect; fixed the same way, gated on `mounted` so the node survives the closing transition. Keyed on the DOM rather than the ticket's suggested incrementing z-index: a module-level counter in core-ui is not shared across Module Federation remotes, since federated hosts share it non-singleton, so two remotes would each start their counter at the same value and collide — which is the reported case. zIndex.modal stays a single constant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

bert-e · 2026-08-27T15:01:58Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

JeanMarcMilletScality · 2026-08-27T15:22:53Z

/approve

bert-e · 2026-08-27T15:23:05Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/1.0

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2026-08-27T15:24:13Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/1.0

Please check the status of the associated issue CUI-43.

Goodbye jeanmarcmilletscality.

JeanMarcMilletScality force-pushed the bugfix/CUI-43-modal-stacking branch from c282d3d to 09e1cda Compare August 26, 2026 14:22

JeanMarcMilletScality force-pushed the bugfix/CUI-43-modal-stacking branch from 09e1cda to 3ab17ac Compare August 26, 2026 16:07

JeanMarcMilletScality marked this pull request as ready for review August 26, 2026 17:09

damiengillesscality approved these changes Aug 27, 2026

View reviewed changes

bert-e merged commit 0d28305 into development/1.0 Aug 27, 2026
8 checks passed

Navigation Menu

Uh oh!

bugfix(modal): stack body portals by open order, not mount order (CUI-43) - #1186

bugfix(modal): stack body portals by open order, not mount order (CUI-43)#1186
bert-e merged 1 commit into
development/1.0from
bugfix/CUI-43-modal-stacking

JeanMarcMilletScality commented Aug 26, 2026 •

edited

Loading

Uh oh!

bert-e commented Aug 26, 2026

Uh oh!

bert-e commented Aug 26, 2026

Uh oh!

bert-e commented Aug 27, 2026

Uh oh!

JeanMarcMilletScality commented Aug 27, 2026

Uh oh!

bert-e commented Aug 27, 2026

Uh oh!

bert-e commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

JeanMarcMilletScality commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

Context

Approach

Why not the incrementing z-index the ticket recommended

Measured

Audit of the other document.body portals

Usage

Review focus

How to test

Follow-up

References

Uh oh!

bert-e commented Aug 26, 2026

Hello jeanmarcmilletscality,

Uh oh!

bert-e commented Aug 26, 2026

Waiting for approval

Uh oh!

bert-e commented Aug 27, 2026

Waiting for approval

Uh oh!

JeanMarcMilletScality commented Aug 27, 2026

Uh oh!

bert-e commented Aug 27, 2026

In the queue

Uh oh!

bert-e commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JeanMarcMilletScality commented Aug 26, 2026 •

edited

Loading

Audit of the other `document.body` portals