-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Block Library: Fix Query block undo trap during creation #30203
Conversation
updateQuery( newQuery ); | ||
} | ||
}, [ query.perPage, query.inherit ] ); | ||
}, [ query.perPage ] ); |
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 is unrelated and just a leftover..
Size Change: +28 B (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
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.
Tested, and this fixes the undo trap as expected! Undo/redo and save flows seem to work as expected.
Unrelated to the change here, I noticed some errors in the console related to the post title block:
and opened a quick fix PR #30208
🤔 Noticing a small thing, although cannot consistently reproduce: If i insert query block -> select a pattern -> save the changes -> select 'undo': it seems "redo" is sometimes not an available option. It seems small and overall this feels like a big step up from where it was, but noticing that inconsistency at times. |
Oh, do you think this could work for the image block too? #30114 |
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.
Can't comment on the code but the experience worked great based on my testing! Thanks for getting this in place - it's come up a few times with the FSE outreach program:
query.mov
The only thing I noticed is that the site content was all out of wack in terms of width but I'm guessing that's due to the recent PR exploring changing how alignments are handled with FSE & block themes.
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 was able to replicate the problem, and in my tests this PR solves the issue.
I am seeing the same JS error mentioned above, but the redo is working well for me.
I noticed this error yesterday and meant to see the root cause of this tomorrow. In my quick check I think it has to do with
I guess so, it seems like a similar problem. I'll check the PR better tomorrow though and it's worth trying I think @ellatrix . |
Description
Fixes: #29494
In
Query
block there are some effects running where some initialization logic is happening and setting some values to some attributes (ex. queryId).These updates can cause an
undo trap
where undoing will result in resetting again, so we need to mark these changes as not persistent with__unstableMarkNextChangeAsNotPersistent
.Testing instructions
undo
is performed properly and everything else works as before and expected.Screenshots