Skip to content
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

feat(ui): toasts rework #6415

Merged
merged 12 commits into from
May 21, 2024
Merged

feat(ui): toasts rework #6415

merged 12 commits into from
May 21, 2024

Conversation

psychedelicious
Copy link
Collaborator

Summary

Add a utility for managing toasts. A single method toast is exported. It can be used to both create a new toast or update an existing one, if provided the ID of an existing toast. All chakra toast options are supported, plus one new option: withCount.

withCount defaults to true, and handles appending a couple to the title of the toast when mutltiples are fired:

toast({ title: 'Hello', withCount: true });
// first toast title: 'Hello'
// second toast title: 'Hello (2)'
// third toast title: 'Hello (3)'

When updating a toast, the new title, description and other params will immediately override the existing toast.

If set to false, no count is appended. Note that if the id changes each time the toast is fired, that is considered a different toast and it will create a new one instead of updating.

All toasts across the app have been revised to use this utility. There's no more "toast queue" - toasts are fired the same way whether in a component, listener or wherever.

Other changes:

  • Fixed recalling seed not showing a toast
  • Consolidated some model install logic that used toasts, was duplicated like several times

Related Issues / Discussions

QA Instructions

Note: There is some minor jank with how the toasts resize if you update them rapidly. This is a chakra issue - will need to dig into chakra theming to fix it.

  • Do something that makes a toast many times in a row. For example, spam one of the parameter recall buttons.
  • Create a graph with a single divide integer node. Leave the values at zero to create a divide-by-zero error. Increase iterations to 5 or so and invoke. You should see one batch queued toast and one error toast that rapidly updates.

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable) This needs proper e2e tests to test, but I don't think this particular functionality is super important to test.
  • Documentation added / updated (if applicable)

@github-actions github-actions bot added the frontend PRs that change frontend files label May 21, 2024
@maryhipp
Copy link
Collaborator

This looks good but it will be really hard to copy the session ID if you have a long queue and they are all failing because it basically flickers through each one as the socket error fires. I played around a bit to see if I could easily to just show the first, or a list but no luck. What do you think would be the best UX?

@psychedelicious
Copy link
Collaborator Author

@maryhipp I've made the suggested changes, and updated the logic for what the toast displays per our discussion

Small wrapper around chakra's toast system simplifies creating and updating toasts. See comments in toast.ts for details.
This was duplicated like 7 times or so
With the model install logic cleaned up the enum is less useful
If false, when updating a toast, the description is left alone. The count will still tick up.
Only display the session if local. Otherwise, just display the error message.
@psychedelicious psychedelicious enabled auto-merge (rebase) May 21, 2024 23:38
@psychedelicious psychedelicious merged commit 0ba57d6 into main May 21, 2024
14 checks passed
@psychedelicious psychedelicious deleted the psyche/feat/ui/toasts-rework branch May 21, 2024 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend PRs that change frontend files
Projects
None yet
2 participants