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

Add pop-up notifs + show a notif when an address has been copied #1012

Merged
merged 3 commits into from
Oct 13, 2022

Conversation

csillag
Copy link
Contributor

@csillag csillag commented Sep 22, 2022

out2

Show a notification when address is copied.

The bigger chunk of this PR is actually integrating the notification widget (from Grommet) in such a way that it's convenient to use anywhere inside the app.

The method of integration is almost exactly the same how Modal windows are integrated.dialogs.

Fixes #271.

@csillag csillag changed the title Csillag/indicate address copied Add a visual indication that an address has been copied Sep 22, 2022
@csillag csillag force-pushed the csillag/indicate-address-copied branch from 252cc73 to ac018e9 Compare September 22, 2022 08:52
@github-actions
Copy link

github-actions bot commented Sep 22, 2022

MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ EDITORCONFIG editorconfig-checker 8 0 0.02s
✅ GIT git_diff yes no 0.01s
✅ JSON eslint-plugin-jsonc 1 0 0 0.91s
✅ JSON jsonlint 1 0 0.26s
✅ JSON prettier 1 0 0 0.55s
✅ JSON v8r 1 0 1.37s
✅ TSX eslint 4 0 0 5.51s

See errors details in artifact MegaLinter reports on CI Job page
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

@csillag csillag force-pushed the csillag/indicate-address-copied branch from ac018e9 to 51e3963 Compare September 22, 2022 10:58
@codecov
Copy link

codecov bot commented Sep 22, 2022

Codecov Report

Merging #1012 (3378393) into master (188d779) will increase coverage by 0.10%.
The diff coverage is 89.47%.

❗ Current head 3378393 differs from pull request most recent head bdd587b. Consider uploading reports for the commit bdd587b to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1012      +/-   ##
==========================================
+ Coverage   88.59%   88.69%   +0.10%     
==========================================
  Files         102      102              
  Lines        1762     1778      +16     
  Branches      406      412       +6     
==========================================
+ Hits         1561     1577      +16     
  Misses        201      201              
Flag Coverage Δ
cypress 60.57% <21.05%> (-0.33%) ⬇️
jest 78.80% <89.47%> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/styles/theme/ThemeProvider.tsx 100.00% <ø> (ø)
src/app/components/Transaction/InfoBox.tsx 84.61% <80.00%> (-15.39%) ⬇️
src/app/components/AddressBox/index.tsx 100.00% <100.00%> (+33.33%) ⬆️

@csillag csillag force-pushed the csillag/indicate-address-copied branch from 51e3963 to c208e1c Compare September 22, 2022 21:20
@kostko
Copy link
Member

kostko commented Sep 23, 2022

@csillag csillag force-pushed the csillag/indicate-address-copied branch from c208e1c to 41b6fbe Compare September 23, 2022 22:41
@csillag csillag changed the title Add a visual indication that an address has been copied Add pop-up notifs + show a notif when an address has been copied Sep 26, 2022
src/app/components/Notification/index.tsx Outdated Show resolved Hide resolved
src/app/components/AddressBox/index.tsx Outdated Show resolved Hide resolved
@csillag csillag force-pushed the csillag/indicate-address-copied branch 3 times, most recently from 803ed36 to b879291 Compare September 26, 2022 16:32
src/app/components/AddressBox/__tests__/index.tsx Outdated Show resolved Hide resolved
src/app/components/AddressBox/index.tsx Outdated Show resolved Hide resolved
src/app/components/AddressBox/index.tsx Outdated Show resolved Hide resolved
src/app/components/Notification/index.tsx Outdated Show resolved Hide resolved
src/app/components/Notification/index.tsx Outdated Show resolved Hide resolved
@csillag csillag force-pushed the csillag/indicate-address-copied branch 3 times, most recently from 2206eeb to b069a53 Compare September 27, 2022 08:59
@csillag
Copy link
Contributor Author

csillag commented Sep 27, 2022

@lukaw3d Thanks for all the input. I believe I have now addressed everything you mentioned.

@csillag csillag force-pushed the csillag/indicate-address-copied branch from 516e5c4 to 200a018 Compare September 27, 2022 13:58
@csillag csillag force-pushed the csillag/indicate-address-copied branch from 200a018 to 1836405 Compare September 28, 2022 17:52
Comment on lines 24 to 25
expect(copy).toHaveBeenCalledWith(testAddress)
})
Copy link
Member

Choose a reason for hiding this comment

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

I think you meant to check that notification shows up here :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I wanted to test both the actual clipboard copy and the notification, since AddressBox had no tests before.
Now I have added a new test case below, specifically for the notification.

src/app/components/AddressBox/index.tsx Show resolved Hide resolved
@csillag csillag force-pushed the csillag/indicate-address-copied branch from e99a1b4 to aa38444 Compare September 28, 2022 22:30
@csillag csillag force-pushed the csillag/indicate-address-copied branch 2 times, most recently from b58ce05 to ac45682 Compare September 29, 2022 10:39
@csillag csillag force-pushed the csillag/indicate-address-copied branch from ddb184b to 974ffa0 Compare September 29, 2022 10:41
@csillag csillag force-pushed the csillag/indicate-address-copied branch from 974ffa0 to d5e7492 Compare September 29, 2022 17:26
@csillag csillag force-pushed the csillag/indicate-address-copied branch from d5e7492 to 3378393 Compare October 13, 2022 11:36
@csillag csillag force-pushed the csillag/indicate-address-copied branch from 3378393 to bdd587b Compare October 13, 2022 11:43
@csillag csillag merged commit 6bd2f70 into master Oct 13, 2022
@csillag csillag deleted the csillag/indicate-address-copied branch October 13, 2022 11:50
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.

a visual indication that you clicked the button to copy an address
4 participants