-
Notifications
You must be signed in to change notification settings - Fork 49.6k
[SuspenseTab] Scuffed version of Suspense rects #34188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think divs are probably better. We'll likely put various UI inside of them too. They're also better for view transitions.
Yeah, it's just easier to get the initial scaling and I probably need to move around how they're placed depending on future UI. A naive absolute placement of divs wouldn't work either if we want to include the name in the rects because those might be hidden. |
continue; | ||
} | ||
|
||
const rects = shell.rects; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only picks the rects from the roots but the suspense boundaries within the root can overflow the root and the inner suspense boundaries themselves and overflow.
For example, if you just have an absolutely positioned child inside body it's likely that the body element is smaller than the child.
So if the last boundary is below the size of the root then it's not clickable in this mode. This should really be the max of every rect recursively.
However, another way is to just make it like:
<div className="document" onClick={clickedDocument} style={{ width: '100%', height: '100%', overflow: 'auto' }}>
<div className="root" style={{ width: root1.width, height: root1.height, left: root1.x, top: root1.y, position: 'absolute' }}></div>
<div className="root" style={{ width: root2.width, height: root2.height, left: root2.x, top: root2.y, position: 'absolute' }}></div>
<div className="suspense" style={{ width: suspense1.width, height: suspense1.height, left: suspense1.x, top: suspense1.y, position: 'absolute' }} onClick={clickedSuspense.bind(null, suspense1.id)}></div>
...
</div>
Since then the overflow of the "document" div will account for the max of all the children automatically.
Another reason to maybe favor divs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although maybe we need to keep track of the min/max of the whole document anyway to allow the UI to scale down to fit the width of the smaller devtools window automatically.
DiffTrain build for [02a8811](facebook@02a8811)
DiffTrain build for [02a8811](facebook@02a8811)
For now we scale the width of the bounding box to the available Suspense tab width. The scaling lacks nuance but basic cases should be viewable. svg might not be the best choice for the final scaling heuristics.
Next up is investigating backend renderer bugs so that we can start testing it on non-trivial surfaces
CleanShot.2025-08-12.at.20.08.05.mp4