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: adding toast notification #876

Conversation

CarolineLCa
Copy link
Contributor

Summary of Changes

Adding toast notifications to Decline action where they were missing:

  • Proof Request
  • Credential Offer

Before

Screen_Recording_20230707_124753_Aries.Bifold.mp4

Now

Screen_Recording_20230707_141004_Aries.Bifold.mp4

Related Issues

N/A

Pull Request Checklist

Tick all boxes below to demonstrate that you have completed the respective task. If the item does not apply to your this PR check it anyway to make it apparent that there's nothing to do.

  • All commits contain a DCO Signed-off-by line (we use the DCO GitHub app to enforce this);
  • Updated LICENSE-3RD-PARTY.md for any added dependencies or vendored components;
  • Updated documentation as needed for changed code and new or modified features;
  • Added sufficient tests so that overall code coverage is not reduced.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

Pro Tip 🤓

  • Read our contribution guide at least once; it will save you a few review cycles!
  • Your PR will likely not be reviewed until all the above boxes are checked and all automated tests have passed.

PR template adapted from the Python attrs project.

Signed-off-by: Caroline Lucas Calheirani <[email protected]>
…otification' into feat/adding-toast-notification

Signed-off-by: Caroline Lucas Calheirani <[email protected]>
@CarolineLCa CarolineLCa requested a review from a team as a code owner July 7, 2023 18:33
@jleach jleach changed the title feat:adding toast notification feat: adding toast notification Jul 10, 2023
@jleach
Copy link
Contributor

jleach commented Jul 20, 2023

@CarolineLCa If you update the base branch we can get this one merged in. You may need to write a test somewhere in the app if CodeCov complains. We can't merge any PRs that notably drop our code coverage. But any test would help I think.

@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Merging #876 (78e343c) into main (60c9ceb) will increase coverage by 2.55%.
Report is 1 commits behind head on main.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##             main     #876      +/-   ##
==========================================
+ Coverage   52.37%   54.93%   +2.55%     
==========================================
  Files         178      178              
  Lines        4987     5028      +41     
  Branches     1374     1393      +19     
==========================================
+ Hits         2612     2762     +150     
+ Misses       2355     2245     -110     
- Partials       20       21       +1     
Files Changed Coverage Δ
.../App/components/listItems/NotificationListItem.tsx 53.52% <0.00%> (-0.77%) ⬇️
packages/legacy/core/App/localization/en/index.ts 100.00% <ø> (ø)
packages/legacy/core/App/localization/fr/index.ts 100.00% <ø> (ø)
...ckages/legacy/core/App/localization/pt-br/index.ts 100.00% <ø> (ø)
...ckages/legacy/core/App/screens/CredentialOffer.tsx 72.41% <0.00%> (-0.85%) ⬇️
packages/legacy/core/App/screens/ProofRequest.tsx 65.73% <74.46%> (+25.45%) ⬆️
...gacy/core/App/components/misc/CredentialCard11.tsx 90.16% <100.00%> (+15.37%) ⬆️
packages/legacy/core/App/types/record.ts 100.00% <100.00%> (+25.00%) ⬆️

... and 2 files with indirect coverage changes

@bryce-mcmath
Copy link
Contributor

The UI/UX team thinks these toasts aren't entirely necessary, partially because the user can see the original notification disappear. Ok if we close this one?

@CarolineLCa CarolineLCa deleted the feat/adding-toast-notification branch August 31, 2023 15:27
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.

3 participants