fix(suggestion): add isLoading state to suggestion popup #5542
Conversation
🦋 Changeset detectedLatest commit: 2742b77 The changes in this PR will be included in the next version bump. This PR includes changesets to release 54 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| props.isLoading = true | ||
| Promise.resolve(items({ editor, query: state.query })).then(result => { | ||
| if (props == null) { | ||
| return | ||
| } | ||
| props.isLoading = false | ||
| props.items = result | ||
| renderer?.onUpdate?.(props) |
There was a problem hiding this comment.
This is doing more than just exposing the loading state, it should still be awaited, you are changing the behavior of how this works.
There was a problem hiding this comment.
Ah I see apologies - this is kind of a naive question, why does it need to be awaited?
There was a problem hiding this comment.
By awaiting the function, you are suspending it's execution of the lines after it. If the goal truly is to only expose an isLoading flag, then you should not be messing with the order of execution. By doing a Promise.resolve & not awaiting it, you are immediately executing the next lines synchronously.
There was a problem hiding this comment.
That makes sense! Though wouldn't having await defeat the purpose of having an up-to-date isLoading value if the component is only going to be re-rendered after the promise returns?
I imagined it like this:
- update
isLoading-> rerender (i.e. immediately executing next lines) - when promise is resolved -> update
isLoading-> rerender
and re: messing up the next lines: maybe something like this might work?
Might be missing a bit of context and need a bit of hand-holding here
Changes Overview
Fixes issue #5533 by exposing an
isLoadingprop for the suggestion popup.Implementation Approach
Not sure if this is the way to go, took the least resistant path. This re-renders the popup after resolving the
itemspromise with the updated result.Testing Done
Here's a recording, though I'm not sure how it should be presented on the demo example.
https://www.loom.com/share/11fb168e6f0e4e6eb2853472b4f90842
Verification Steps
As described in the video above.
Additional Notes
Checklist
Related Issues