-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Design Tools: Fix last ToolsPanelItem styling #56536
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.
+1 Nice debugging everyone, great work coming up with a suggested fix @t-hamano and thanks for putting up a PR @aaronrobertshaw!
This is testing really well for me, and I haven't been able to break it. The gap is consistently resolved for me, and otherwise the ToolsPanel and its items appears to behave just as on trunk.
Before
After
LGTM! ✨
I think this PR might have had a small but noticeable impact on the performance of "block select" and "typing within container". Do you think there's something to be done here? |
@ciampo do you have any ideas on how to avoid the I'm coming up short on options other than perhaps adding a means of filtering the items on which the |
Maybe useLayoutEffect is fine, maybe we're doing some heavy stuff within it. I have no idea here, just sharing random thoughts |
Yeah I agree with Riad — I don't think the issue is about using |
Fixes: #56470
What?
Updates
ToolsPanelItem
to useuseLayoutEffect
for its registration hook to prevent a race condition causing a rendering glitch in a few browsers for the last panel item.Kudos to @t-hamano for the suggested fix and the debugging efforts of @ciampo and @andrewserong 🙇
Why?
Prevents a rendering glitch causing the appearance of a broken layout in the color panel.
How?
Testing Instructions
Screenshots or screencast