feat: add a placeholder slot for the embed loading gap - #58
Open
sidgaikwad wants to merge 1 commit into
Open
Conversation
Between mount and the editor being ready the component rendered an empty container. On a cold CDN cache that is the full embed.js request plus the versioned-bundle hop showing as a blank box at minHeight 500, with no supported way to fill it: onLoad only fires after success, so consumers had to overlay their own absolutely-positioned spinner and know about the internal two-div layout. Render `placeholder` centred over the container while the editor is null. Two design points: - It is an overlay sibling, not a child of the mount container. The embed owns that container's DOM and React must not reconcile children in and out from under it. - It stays visible when the mount fails, so a failed load never ends on a blank box. Consumers pair it with onError to swap in a failure message. position: relative is only applied when a placeholder is actually being shown, so existing layouts are untouched.
|
@sidgaikwad is attempting to deploy a commit to the Unlayer Team on Vercel. A member of the Team first needs to authorize it. |
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.
Fixes #37.
Problem
Between mount and the editor being ready, the component rendered an empty container. On a cold CDN cache that's the full
embed.jsrequest plus the versioned-bundle hop showing as a blank box atminHeight: 500.There was no supported way to fill it:
onLoadonly fires after success, so consumers had to render their own absolutely-positioned overlay from first paint, tear it down inonLoad, know about the internal two-div layout — and it still didn't cover the failure path.Change
Two design decisions worth your attention:
div.image-editor-rootinto it), so React must not reconcile children in and out from under it.onErrorto swap in a failure message. Documented in the README and the prop's doc comment.position: relativeis applied to the wrapper only while a placeholder is actually showing, so existing layouts are untouched.Verification
Beyond the unit tests, I wired it into the demo and pointed
scriptUrlat a non-existent file to hold the failure state open.Before — embed fails to load, entire editor area is blank white:
After — same failure, with a spinner and "Loading the image editor…" centred in the area.
The demo scaffolding was reverted; this PR touches only
src/,test/and the README.lint,typecheck,buildcleanConflict note
Touches the same render block as #55, and the same README props table as #54.