Skip to content

[#6346, #6477, #6421] Platform: Implemented toaster component for visual feedback #7078

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Feb 19, 2021
Merged

[#6346, #6477, #6421] Platform: Implemented toaster component for visual feedback #7078

merged 13 commits into from
Feb 19, 2021

Conversation

gaurav061
Copy link
Contributor

@gaurav061 gaurav061 commented Feb 3, 2021 •
edited
Loading

Description: Added toaster for having visual feedback for user action.
example:

  1. When the user adds/delete the KMS configuration.
  2. When duplicate region code is added for on-prem Provider.
  3. Api error response in case of edit universe.

@gaurav061
Copy link
Contributor Author

Please add your comments.

@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2021

This pull request introduces 4 alerts when merging a6b69c1 into 3ce9d33 - view on LGTM.com

new alerts:

  • 3 for Unused variable, import, function or class
  • 1 for Potentially inconsistent state update

nishantSharma459
nishantSharma459 reviewed Feb 4, 2021
View reviewed changes
nishantSharma459
nishantSharma459 reviewed Feb 4, 2021
View reviewed changes
nishantSharma459
nishantSharma459 reviewed Feb 4, 2021
View reviewed changes
nishantSharma459
nishantSharma459 reviewed Feb 4, 2021
View reviewed changes
nishantSharma459
nishantSharma459 reviewed Feb 4, 2021
View reviewed changes
nishantSharma459
nishantSharma459 reviewed Feb 4, 2021
View reviewed changes
nishantSharma459
nishantSharma459 reviewed Feb 4, 2021
View reviewed changes
nishantSharma459
nishantSharma459 reviewed Feb 4, 2021
View reviewed changes
nishantSharma459
nishantSharma459 reviewed Feb 4, 2021
View reviewed changes
Copy link
Contributor

@nishantSharma459 nishantSharma459 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address all the comments.

@gaurav061 gaurav061 requested a review from sshev February 8, 2021 11:48
@bhavin192 bhavin192 mentioned this pull request Feb 9, 2021
Closed
sshev
sshev reviewed Feb 10, 2021
View reviewed changes
Copy link

@sshev sshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • why would you need redux integration for toasts? to me it seems totally unnecessary
  • is there a reason for implementing your own toasts solution? I'd strongly suggest reusing some available solution like react-toastify for example

@sshev sshev requested a review from andrewc-dev February 10, 2021 05:26
@gaurav061
Copy link
Contributor Author

Sure @sshev I will use react-toastify for this, I thought it wasn't acceptable to use any other library for this.

sshev
sshev suggested changes Feb 17, 2021
View reviewed changes
Copy link

@sshev sshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check my comments pls. Also, we are using a prettier tool to automatically format code as per unified code style, so I'd suggest running prettier --write src/**/* before submitting PR for review.

@gaurav061 gaurav061 changed the title [#6346, #6477, #6392] Platform: Implemented toaster component for visual feebback. [#6346, #6477] Platform: Implemented toaster component for visual feebback. Feb 17, 2021
@gaurav061 gaurav061 requested a review from sshev February 17, 2021 11:24
@sshev sshev changed the title [#6346, #6477] Platform: Implemented toaster component for visual feebback. [#6346, #6477] Platform: Implemented toaster component for visual feedback Feb 18, 2021
sshev
sshev reviewed Feb 18, 2021
View reviewed changes
This was referenced Feb 18, 2021
Closed
Closed
@gaurav061 gaurav061 changed the title [#6346, #6477] Platform: Implemented toaster component for visual feedback [#6346, #6477, #6421] Platform: Implemented toaster component for visual feedback Feb 18, 2021
@gaurav061 gaurav061 requested a review from sshev February 18, 2021 06:37
@gaurav061 gaurav061 mentioned this pull request Feb 18, 2021
Closed
sshev
sshev approved these changes Feb 19, 2021
View reviewed changes
Copy link

@sshev sshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@gaurav061 gaurav061 merged commit f1f0218 into yugabyte:master Feb 19, 2021
@danielisen
Copy link
Contributor

@gaurav061 I am seeing an error popup show up in the UI when creating a KMS config even though the creation succeeds. The error seems to be JS related.

It is something like: dispatch(…).then is not a function

@gaurav061
Copy link
Contributor Author

@daniel-yb Yes, I reproduced the same it seems that while calling the API to setKMSConfig there is a response dispatcher once the API is successful that is not thenable thus this error is coming, I will fix this as a part of the follow-up ticket. Thanks for pointing it out.

polarweasel pushed a commit to lizayugabyte/yugabyte-db that referenced this pull request Mar 9, 2021
@gaurav061
[yugabyte#6346, yugabyte#6477, yugabyte#6421] Platform: Implemented t…
6e07ea1 
…oaster component for visual feedback (yugabyte#7078)

Description: Added toaster for having visual feedback for user action.
cases covered:
1. When the user adds/delete the KMS configuration.
2. When duplicate region code is added for on-prem Provider.
3. Api error response in case of edit universe.

Test scenario:
- Adding duplicate region code for the on-prem provider notifies the user of an appropriate toaster.
-  Adding/Deleting the KMS have visual feedback to the users.
- Any error encountered during edit universe have visual feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaydeepkumara jaydeepkumara Awaiting requested review from jaydeepkumara

@andrewc-dev andrewc-dev Awaiting requested review from andrewc-dev

@nishantSharma459 nishantSharma459 Awaiting requested review from nishantSharma459

1 more reviewer

@sshev sshev sshev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gaurav061 @danielisen @sshev @nishantSharma459