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

Replace default alerts with Toast #543

Merged
3 commits merged into from
Aug 4, 2022
Merged

Replace default alerts with Toast #543

3 commits merged into from
Aug 4, 2022

Conversation

HenrikeW
Copy link
Contributor

@HenrikeW HenrikeW commented Aug 4, 2022

Related issue(s) and PR(s)

There were some places left where we used alert. All of them are now replaced with our own Toast, type "warning".

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

List of changes made

  • because the list of toasts is handled in Report.tsx, a handler function was created and passed on to the child component that needed it (QuickAdd.tsx)
  • alerts in QuickAdd and Report were replaced with toasts.

Screenshot of the fix

Just one example.
image

Testing

Most of the cases are error handling for API requests failing etc, so hard to test.
The one that can be tested is the one on the screenshot above:
Try to add an issue/activity pair that already is part of your list of recent issues.
You should see the toast (was alert before).

Definition of Done checklist

  • I have made an effort making the commit history understandable
  • I have performed a self-review of my own code and commented any hard-to-understand areas
  • Tests and lint/format validations are passing
  • My changes generate no new warnings

@HenrikeW HenrikeW requested review from jonandernovella and a user August 4, 2022 07:56
@HenrikeW HenrikeW marked this pull request as draft August 4, 2022 07:57
@HenrikeW HenrikeW force-pushed the dev/replace-alerts branch from ebb9aa6 to bc7857e Compare August 4, 2022 07:59
@HenrikeW HenrikeW marked this pull request as ready for review August 4, 2022 08:05
@ghost ghost merged commit f1df7ac into develop Aug 4, 2022
@ghost ghost deleted the dev/replace-alerts branch August 4, 2022 08:18
@jonandernovella jonandernovella mentioned this pull request Aug 8, 2022
This pull request was closed.
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.

1 participant