Conversation
|
Size Change: +40 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
|
This fixed the issue for me, but I wonder if instead we should tighten up that effect as it shouldn't be setting that attribute when the block is first added as there is no background at that point to determine if it |
|
Thanks for the review, @glendaviesnz. Unfortunately, conditionally updating attribute inside effect will still create an extra undo level. Since |
I thought it didn't in my testing yesterday, but I agree that we would be just duplicating all the dependency checks of the I wonder if this would all be more efficient if the isDark checking and attribute setting was only run as part of an explicit background change action instead of with hooks and effects ... but I think we should worry about that later after fixing the immediate issue with the |
glendaviesnz
left a comment
There was a problem hiding this comment.
This tested well for me and fixed the double undo without affecting any of the other functionality for changing text color based on dark or light backgrounds.
As noted here, it is probably worth a follow up to see if there is a more efficient way of setting the isDark attribute.
|
Thanks, @glendaviesnz.
I think |
Description
PR fixes the Cover block "undo trap" by marking attribute update inside
useEffectas non-persistent.Fixes #36781.
How has this been tested?
Types of changes
Bugfix
Checklist:
*.native.jsfiles for terms that need renaming or removal).