-
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
Global Styles → Blocks: Radius control very slow and laggy, possibly due to inline preview #47352
Comments
@madhusudhand I think you worked on the block preview feature in #46727, do you think it could be related? Thank you for looking! |
Thank you for looking! |
@jasmussen I have added a debounce of 1ms to Here are the results- Beforebefore.movAfterafter.mov |
Thanks for that, do you have a PR for it? It looks better but still not entirely perfect, so I'm curious what might be going on under the hood. I'll add a label and hopefully we can get some additional eyes. |
I did quick profiling for this. Changing block global styles settings rerenders the whole Site Editor, which can be expensive and make UI feel unresponsive. |
Could we the inline previewe update instantly, but defer updating the site editor until mouseup, or something along those lines? |
@jasmussen, I think that would be the right direction. We only want instant feedback in previews. |
The problem I can see with only updating in the editor on mouseup or blur is that a user may be looking at a button in the editor as they update the control, rather than the preview, and they may wonder why that button is not changing as that is the expected UX for all other global styles panel controls. I put a draft PR up with the debounce option mentioned by @abhi3315, which seems to improve things - I wonder if it is worth doing this for 6.2 and exploring a wider solution to the preview/site edit perf issues after that? |
It seems like this would fall in bugfix territory which allows us more time than feb 7 no? But which technical solution to go with, I'll refer to you all. It should feel completely fluid when dragging the slider, I guess we can also update the canvas when you stop moving the mouse? Or when a value is fixed for a short while? |
In theory, but at a quick glance it seems like a robust/permanent solution to this could involve some major replumbing of how these controls work. Currently, the preview and the editor are reacting to the same data updates. The slider position and value input box are also reacting to this same data update (this is why the debounce can't be wound up any higher than about 5ms or the slider and input box updates get very sluggish), so these would all need to be decoupled somehow. If we go down this path it probably makes sense to look at it from a wider global styles perspective rather than just the border radius, so could turn into more than a simple bug fix. I would still push for adding the debounce to improve the situation slightly and circle back after 6.2 to revisit how these controls in general interact with the preview and editor canvas ... but I may be overlooking another quick and easy fix for this. |
@glendaviesnz are you able to progress on #47532? Trying to see if we can indeed get some iteration in place for 6.2 or whether the entire thing needs to be punted for now. |
@jasmussen This seems much more performant now on trunk: radius.mp4Are you still seeing the performance issues? |
Hmm, yes, not sure what happened, but it is indeed much better. I still feel like it could be even better still, but for now I'm going to close this one as fixed. CC: @youknowriad for awareness, I know he's done some perf work recently. |
If you go into Global Styles, then Blocks, then for example Button, the border radius control is very slow and laggy:
That same control is plenty responsive in the editor view:
It seems possible that the inline block preview can be related.
The text was updated successfully, but these errors were encountered: