Skip to content

fix(ui-modal): restore Modal open/close transition animation#2599

Open
matyasf wants to merge 1 commit into
masterfrom
fix/INSTUI-5072-modal-transition
Open

fix(ui-modal): restore Modal open/close transition animation#2599
matyasf wants to merge 1 commit into
masterfrom
fix/INSTUI-5072-modal-transition

Conversation

@matyasf

@matyasf matyasf commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Move ModalContext.Provider outside Transition (v1 + v2) so the dialog/Mask is Transition's direct child again and the fade lifecycle runs.
  • v2: pass open to the Dialog unconditionally (was gated on !transitioning) so the window renders during the transition and fades together with the Mask.
  • v1 + v2: release focus from the Dialog at the start of the exit transition so focus isn't retained inside the aria-hidden fading subtree (fixes the retained-focus console warning).
  • docs: fix the Mask z-index theme-override example in the v2 README.

Test Plan

  • On the Modal docs page, open and close the first example: both the Mask and the window should fade in and out together (~300ms), not appear/disappear instantly.
  • Close the Modal via the Close button (or Esc / click-away) with focus inside it and confirm no aria-hidden/retained-focus warning in the console, and that focus returns to the trigger.
  • Spot-check with a screen reader that focus moves into the Modal on open and back to the trigger on close.

Fixes INSTUI-5072

🤖 Generated with Claude Code

The Modal mounted and unmounted instantly with no fade. Transition drives its
enter/exit lifecycle on its single direct child, but that child was a
ModalContext.Provider (no DOM node), so the dialog was never animated. Move the
Provider to wrap Transition (v1 and v2) so the dialog/Mask is the transitioned
element again.

In v2 the Dialog was additionally gated with `open={open && !transitioning}`, so
the window was not rendered during the transition and could not fade with the
Mask. Pass `open` unconditionally (matching v1) so the window fades together
with the Mask, and release focus from the Dialog at the start of the exit
transition so focus is not retained inside the aria-hidden, fading subtree.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@matyasf matyasf self-assigned this Jun 19, 2026
@matyasf matyasf requested a review from balzss June 19, 2026 14:58
Comment on lines +621 to +626
themeOverride={{
Mask: {
zIndex: 555
}
}}
>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Note: This token does not exist yet in v2, I've asked Adam to create it

@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://instructure.design/pr-preview/pr-2599/

Built to branch gh-pages at 2026-06-19 15:04 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

Copy link
Copy Markdown

Visual regression report

No changes.

Status Count
Unchanged 32
Changed 0
New 0
Removed 0

📊 View full report

Baselines come from the visual-baselines branch. They refresh on every merge to master.

github-actions Bot pushed a commit that referenced this pull request Jun 19, 2026
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