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

[curator-viewer] enabled toast instead of alert for copy paste, and fixed streaming toast #165

Conversation

CharlieJCJ
Copy link
Contributor

@CharlieJCJ CharlieJCJ commented Nov 21, 2024

Addressed #153
Changes:

  • Reduced the TOAST_REMOVE_DELAY from 1000000ms to 3000ms for quicker dismissal of toast notifications.
  • Enabled updates toast in DatasetViewer page
  • Enabled toast updates in DetailsSidebar view
  • Added component to the root layout to enable toast notifications.

Testings:

  • tested on all litellm examples, openai examples.
  • on both streaming and non streaming.

Quick demo:
video

Note: This PR only affects UI, doesn't affect the copy paste behaviors in colab embedded view. The issue with colab embedded view is an common issue with <iframe> for security reasons.

@CharlieJCJ CharlieJCJ changed the title enabled toast instead of alert, and fixed streaming toast enabled toast instead of alert for copy paste, and fixed streaming toast Nov 21, 2024
@CharlieJCJ CharlieJCJ changed the title enabled toast instead of alert for copy paste, and fixed streaming toast [curator-viewer] enabled toast instead of alert for copy paste, and fixed streaming toast Nov 21, 2024
@CharlieJCJ CharlieJCJ marked this pull request as ready for review November 23, 2024 22:01
@CharlieJCJ CharlieJCJ marked this pull request as draft November 23, 2024 22:09
@CharlieJCJ
Copy link
Contributor Author

Need additional testing on colab after a built UI.

@madiator
Copy link
Contributor

Is this still a draft PR?
Also, the video is a bit unclear: are you clicking "Copy" several times or a single click is triggering those many toasts?

@CharlieJCJ CharlieJCJ marked this pull request as ready for review November 25, 2024 04:42
@CharlieJCJ
Copy link
Contributor Author

Made it to ready.

Video is clicking copy paste multiple times.

@CharlieJCJ
Copy link
Contributor Author

Added a note in the PR description: Note: This PR only affects UI, doesn't affect the copy paste behaviors in colab embedded view. The issue with colab embedded view is an common issue with <iframe> for security reasons

@vutrung96
Copy link
Contributor

sorry missed this PR. LGTM

@vutrung96 vutrung96 merged commit 6f527e2 into dev Dec 7, 2024
2 checks passed
@vutrung96 vutrung96 deleted the CURATOR-48-curator-viewer-ux-change-the-copy-paste-pop-up-to-something-else-153 branch December 7, 2024 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[curator-viewer UX] change the copy paste pop up to something else
3 participants