Media editor: avoid double-mount flicker on open#77732
Open
Media editor: avoid double-mount flicker on open#77732
Conversation
Keying `<CropperProvider>` on `media?.id ?? 'none'` caused the provider (and the `<Modal>` it wraps) to remount when `media` resolved from `null` to the loaded record on open. That ran the modal's entry animation twice — once during loading, once after data arrived — visibly flickering through the underlying editor content. Key on the store-level `id` instead, which is set atomically with `isModalOpen=true` and stays stable across the load.
While the modal is waiting for media data, the bare `<Spinner />` sat at the top-left of the content area. Wrap it in a flex shell that fills the modal body and centers the spinner.
ramonjd
added
[Feature] Media
28 tasks
ramonjd
commented
Apr 28, 2026
Comment on lines
+18
to
+25
| // Center the loading spinner in the modal body while media data is | ||
| // resolving, before the InterfaceSkeleton mounts. | ||
| .media-editor-modal__loading { | ||
| flex: 1; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| } |
Member
Author
There was a problem hiding this comment.
Thought I'd piggy back of this small PR.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a subtle modal open flicker in the Media Editor by preventing an unnecessary remount of the cropper context provider while attachment data resolves, keeping the modal’s entry animation from running twice.
Changes:
- Change
CropperProviderkeying frommedia?.idto stable storeidto avoid remount during async record resolution. - Wrap the loading
Spinnerin a dedicated container and add styles intended to center it within the modal body.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/media-editor/src/components/media-editor-modal/index.tsx | Stabilizes CropperProvider mount by keying on store id; adds loading wrapper around spinner. |
| packages/media-editor/src/components/media-editor-modal/style.scss | Adds styles for the new loading wrapper to center the spinner during initial load. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`.media-editor-modal__loading` lives inside `.components-modal__children-container`, which isn't a flex container, so `flex: 1` was a no-op and the spinner stayed top-left. Use `height: 100%` to fill the body, matching the pattern used for `.interface-interface-skeleton` in the same file.
|
Size Change: +61 B (0%) Total Size: 7.77 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 2157cf4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25035450019
|
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
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.
What
<CropperProvider key={ media?.id ?? 'none' }>remounted whenmediaresolved fromnullto the loaded record on open, dragging<Modal>with it. The modal's entry animation ran twice — once with the spinner, once with the cropper. Key onidfrom the store instead, which is set atomically withisModalOpenand stays stable across the load. Should have listened to @andrewserong over at Media Editor: add cropper controls to the media editor modal #77540 (comment) 😄The animation bug is actually very subtle and hard to test without just eyeballing it.
I used the Animations inspector to slow things down in the video:
Before
Kapture.2026-04-28.at.15.10.34.mp4
After
Kapture.2026-04-28.at.15.11.00.mp4
Testing
Enable the experiment:
For the animation:
For the loader: