fix(structure): make sync document toast dismisable #7209
Merged
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
No changes to documentation |
|
Component Testing Report Updated Aug 21, 2024 7:20 AM (UTC) ✅ All Tests Passed -- expand for details
|
pedrobonamin
commented
Jul 20, 2024
Comment on lines
70
to
81
| const conditionalToastParams = useMemo( | ||
| () => ({ | ||
| id: `sync-lock-${documentId}`, | ||
| status: 'warning' as const, | ||
| enabled: isLocked, | ||
| title: t('document-view.form-view.sync-lock-toast.title'), | ||
| description: t('document-view.form-view.sync-lock-toast.description'), | ||
| closable: true, | ||
| }), | ||
| [documentId, isLocked, t], | ||
| ) | ||
|
|
There was a problem hiding this comment.
Memoizing this params to avoid unnecessary calls to the useEffect in the useConditionalToast hook, which uses the params as a dependency.
pedrobonamin
commented
Jul 20, 2024
Comment on lines
35
to
27
| return () => { | ||
| // Close the toast when the component unmounts, even if the condition is still true | ||
| if (wasEnabled && params.enabled) { | ||
| toast.push({ | ||
| ...params, | ||
| // Note: @sanity/ui fallbacks to the default duration of 4s in case of falsey values | ||
| duration: 0.01, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
This will close the toast when the component unmounts, preventing it to be forever present
pedrobonamin
force-pushed
the
dismissable-document-sync-toast
branch
2 times, most recently
from
cd83cf1 to
759b387
Compare
pedrobonamin
commented
Jul 20, 2024
| @@ -18,19 +10,20 @@ const LONG_ENOUGH_BUT_NOT_TOO_LONG = 1000 * 60 * 60 * 24 * 24 | |||
| export function useConditionalToast(params: ToastParams & {id: string; enabled?: boolean}) { | |||
| const toast = useToast() | |||
|
|
|||
| const wasEnabled = usePrevious(params.enabled) | |||
There was a problem hiding this comment.
This is not necessary now that the cleanup function was added to the useEffect.
If the params.enabled condition was true, then the cleanup will take care of closing the toast when params.enabled changes or when the component is unmounted
pedrobonamin
force-pushed
the
dismissable-document-sync-toast
branch
from
dac1623 to
ff02fc6
Compare
ricokahler
approved these changes
Aug 22, 2024
jordanl17
pushed a commit
that referenced
this pull request
Aug 29, 2024
* fix(structure): make sync document toast dismissable * fix(structure): close conditional toast when component unmounts * chore(structure): remove the usePrevious hook in useConditionalToast * fix(structure): update sync lock toast id to make it unique across documents
This was referenced Sep 20, 2024
This was referenced Sep 25, 2024
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.
Description
An issue is present in structure in which sync toasts are forever present.
For what was discussed with the internal team, is that the toast is persisted even though the condition for it is gone or and even if the document is not in view anymore.
This seems to be caused because when the
FormViewcomponent is unmounted, the toast doesn't trigger a close action, to solve this a cleanup function was added to the useEffect to close the toast if it is still shown.This seems to be happening even if the
FormViewis not visually unmounted (document is still open), this seems to be caused because theFormViewcomponent takes a keykey={documentId + (ready ? '_ready' : '_pending')}, so when the key changes, the form is unmounted and mounted again, but given there are no clean up functions in the conditional toast hook, the toast will be kept forever.In this video two buttons were added to force the states that show or hide the toast.
Screen.Recording.2024-07-20.at.10.58.38.mov
It also adds support to close the syncing document toast.
What to review
Testing
Notes for release