hubmapconsortium / portal-ui

HuBMAP Data Portal front end
https://portal.hubmapconsortium.org
MIT License
12 stars 2 forks source link

Austenem/CAT-867 update workspace toasts #3526

Closed austenem closed 1 month ago

austenem commented 2 months ago

Summary

Updates workspace success toasts to indicate whether the workspace has launched in a new tab. Additionally, updates logic to show this toast not just when a workspace is created, but any time a workspace is launched.

Design Documentation/Original Tickets

CAT-867 Jira ticket

Testing

Tested various entry points for the toast:

Screenshots/Video

Screenshot 2024-08-29 at 3 24 19 PM Screenshot 2024-08-29 at 3 24 08 PM

Checklist

john-conroy commented 2 months ago

Does it make sense to make these all into hooks and move the calls to useSnackbarActions?

austenem commented 2 months ago

Does it make sense to make these all into hooks and move the calls to useSnackbarActions?

So something like:

const useWorkspaceUpdateSuccessToast = () => {
    return () => toastSuccess('Workspace successfully updated.');
  };

for each of these in useSnackbarActions()?

john-conroy commented 2 months ago

Yeah, but likely using useCallback with useSnackbarActions in its dependency array. What do you think? It would push all of the logic into the one file, but we don't necessarily have to address it in this PR.

austenem commented 2 months ago

Would having the toast functions be in the dependency array be cleaner? For example, having the following in useSnackbarActions():

  const useWorkspacesDeleteErrorToast =  useCallback(() => {
    return (names: string) => toastError(`Error deleting workspaces: ${names}`);
  }, [toastError]);

So that they can be all be pulled out of useSnackbarActions() instead of imported individually?

john-conroy commented 2 months ago

Yep! That's what I meant, but struggled to communicate. Thanks!

john-conroy commented 2 months ago

Something like:

function useWorkspacesDeleteErrorToast() {
  const {toastError} = useSnackbarActions();
  return useCallback( (names: string)  => {
      toastError(`Error deleting workspaces: ${names}`);
    }, [toastError]);
 }
austenem commented 2 months ago

Ah, ok - so just to clarify, in the example you gave it looks like these hooks would be defined outside of useSnackbarActions() and imported individually?

john-conroy commented 2 months ago

I think I prefer your suggestion. A central hook which returns the various functions. Thanks! Sorry for the confusion.

austenem commented 1 month ago

Thanks for reviewing!