[Block Library - Site Title, Site Tagline]: Add delay to content changes #37234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ntsekouras
added
[Package] Block library
|
Size Change: +1.22 kB (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
ntsekouras
requested review from
ockham,
mcsf,
jeyip,
youknowriad,
ellatrix and
Mamaduka
youknowriad
reviewed
Dec 9, 2021
There was a problem hiding this comment.
The problem with the debounce approach is that the changes are not sent to the data store right away, meaning things like "dirty checks"... can be delayed. Can we instead use a similar technique to RichText here? (if we're editing the same attribute over time, reuse the last "undo" state until the "mark persistent" is called)
To be honest I don't remember the code exactly but the principle should be the same here.
youknowriad
reviewed
Dec 9, 2021
| // Wait a bit as the Site Title's onChange is debounced. | ||
| await page.waitForSelector( | ||
| '.editor-post-publish-button__button.has-changes-dot' | ||
| ); |
There was a problem hiding this comment.
In other words, this change shouldn't be necessary for the test to pass.
The thing is that there is no special technique in RichText. This is handled in If I'm not mistaken of course, to handle this in a similar way, we need to implement something either too specific (like checking So if my above assumption about the alternative approach is correct, is it worth it to implement this in core store? |
I guess my question here is why is this handled on the block-editor store and not directly on the core store? |
|
Answering myself: probably because the attributes are internal to the "block-editor" store and not something that is accessed outside it. Is it possible to make this behavior opt-in or opt-out somehow in the core-data store as well? How can we make sure that this is not just an adhoc fix for the site title but could work for any property (site title, post title, site tagline, excerpt...) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: #37171
Currently undoing a site tagline or site title block change, reverts the text one character at a time, instead of in larger "chunks" like in the paragraph block.
This is happening because we have a specific handling for quick changes in the same block attribute here.
For now this PR just adds a little delay before applying these two blocks' changes.
Should we handle this in a similar way for
EDIT_ENTITY_RECORDaction? IMO it might not worth it a new API forentitiesto handle specific fields, as I feel like there will not be many other cases besides these two sitesettings. What do you think?