chore: enhance delete repo UX #364
Conversation
| @@ -50,14 +72,25 @@ function RepoLine({ repo, deletable, sharable, runtimeInfo, onDeleteRepo }) { | |||
| } | |||
| ); | |||
|
|
|||
| // haochen: any reason not using Loading state from useMutation? | |||
There was a problem hiding this comment.
Good catch. We should use the loading status from useMutation.
| refetchQueries: ["GetRepos"], | ||
| onCompleted() { | ||
| client.writeQuery({ | ||
| query: GET_REPOS, | ||
| data: { | ||
| myRepos: repos.filter((repo) => repo.id !== clickedRepo?.id), | ||
| }, | ||
| }); | ||
| enqueueSnackbar("Successfully deleted repo", { variant: "success" }); | ||
| }, | ||
| onError() { | ||
| enqueueSnackbar("Failed to delete repo", { variant: "error" }); | ||
| }, |
There was a problem hiding this comment.
I think we don't need to manually update a cache here. The deleting variable is enough to achieve the black-out effect in your gif.
In my understanding, the cache-update mechanism is most useful when we want to delete the repo instantly without waiting for deleteRepo mutation returns, which isn't the case here as you are still waiting for the completion of deleteRepo mutation. I can see that the waiting for the second GET_REPOS request is avoided, but that's like reducing from 2s to 1s; not that important. The cache-update is for reducing waiting time from 1s to 0s.
I'd rather do refetchQueries to reduce the complexity of the logic. What do you think?
There was a problem hiding this comment.
@lihebi Good point, managing apollo cache will add logic overhead. But if we rely on refetchQueries, can't achieve UI in gif. The deleted repo will not disappear right away when deleting become false. It will disappear after refetchQueries done. so there will be some noticeable delay between success toast show and the repo disappears.
I think this behavior can seem clumsy to the user, but not a big issue, up to us to see if it is worth fixing.
In my understanding, the cache-update mechanism is most useful when we want to delete the repo instantly without waiting for deleteRepo mutation returns
actually common practice with apollo cache is to wait till the mutation api succeeded then remove cache, because the mutation might fail.
There was a problem hiding this comment.
Thanks for the insight. Yep, let's use this manual cache-update here.
before:
after:
caveat:
Directly remove deleted repo from Apollo Client cache after mutation succeeded instead of refetching repo list to provide that instant UI feedback.
https://www.apollographql.com/docs/react/data/mutations/#updating-local-data