-
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
Editor: Trigger reusable blocks autocomplete when options generated #14915
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.
LGTM 👍 I verified that before as soon as I focused paragraph a blocks request was immediately made, now the request is only made when the inserter or the autocomplete opens.
* been true, but relied upon the fact the user would be delayed in typing an | ||
* autocompleter search query. Once implemented using resolvers, the status of | ||
* this request could be subscribed to as part of a promised return value using | ||
* the result of `hasFinishedResolution`. There is currently reliable way to |
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.
There is currently (no) reliable way to
Thanks for the review @jorgefilipecosta ! While this does serve as a fix for #14766, that issue was also separately fixed by #14916 . I still think this pull request can be considered an improvement to avoid the fetch until necessary, but not as urgent as it once was. I think I may wait until after the next plugin release to consider merging it. Either that, or give some more time to consider what might be necessary to support autocompleters which update options asynchronously. |
Going to merge this as it brings more stability to e2e tests. |
Fixes #14766
This pull request changes the behavior of the default block autocompleter to avoid triggering a fetch of reusable blocks until the time at which the user begins entering a search query which would prompt the block autocompleter to generate its own options set. Prior to this change, a fetch would occur immediately upon selecting a paragraph block. While this had the benefit of preloading data for improved likelihood could be reflected in the search query, it had the downsides of (a) not guaranteeing this likelihood, (b) incurring the overhead of a network request, and (c) causing changes in state. The last of these manifests itself in the bug report described in #14766.
For transparency's sake, I should be clear that the implementation here does not solve the underlying issue that receiving reusable blocks into editor state has the dual effect of creating an undo level and marking the post as having unsaved changes. This is tracked more thoroughly at #14910, with a separate improvement planned. Thus, it is still expected that unexpected undo history will accumulate in response to reusable blocks fetching (e.g. clicking the Inserter when on a new post will add an Undo level). Technically speaking, the full fix considered for #14910 at #14910 (comment) would resolve both #14910 and #14766, at which point this pull request could be considered an optional enhancement / change in behavior to defer fetching of reusable blocks.
Separately, I anticipate that we should explore further refactoring of autocompleters to avoid "fetches" as explicit actions, and consider what solutions might exist for being able to express autocompleter options via subscribed selectors (or
withSelect
-bound components) and complementing resolvers.Testing Instructions:
Repeat Steps to Reproduce from #14766, observing that no undo level is created.