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.

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

feat: add browser notification #528

Merged
merged 18 commits into from
May 23, 2024
Merged

feat: add browser notification #528

merged 18 commits into from
May 23, 2024

Conversation

dni
Copy link
Contributor

@dni dni commented Mar 22, 2024 •
edited
Loading

used when a swap is claimed or refunded.

screenshot-1711525563

screenshot-1714982106

now it is opt in only via settings

@vercel Vercel
Copy link

vercel bot commented Mar 22, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
webapp ✅ Ready (Inspect) Visit Preview May 23, 2024 3:19pm
webapp-mainnet ✅ Ready (Inspect) Visit Preview May 23, 2024 3:19pm

@dni dni requested review from michael1011 and kilrau March 27, 2024 07:47
@dni dni marked this pull request as ready for review March 27, 2024 07:48
dni added a commit that referenced this pull request Mar 27, 2024
@dni
feat: play sound on successful swaps
0224532 
PoC :D still a random sound. but it could be played when wo do the notifications aswell. like here: #528

closes #338
@dni dni mentioned this pull request Mar 27, 2024
Merged
dni added a commit that referenced this pull request Mar 27, 2024
@dni
feat: play sound on successful swaps
b8c11d9 
PoC :D still a random sound. but it could be played when wo do the notifications aswell. like here: #528

closes #338
kilrau
kilrau requested changes Mar 30, 2024
View reviewed changes
Copy link
Member

@kilrau kilrau left a comment

Choose a reason for hiding this comment

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

I open the landing page and it immediately asks for notification permissions - NO
image

Then again on "Create Swap"...

Please read the issue again! We only want notification permissions if user toggles 0-conf off: #346 so that should be the trigger.

@kilrau kilrau unassigned dni Apr 30, 2024
dni added a commit that referenced this pull request May 6, 2024
@dni
feat: play sound on successful swaps
3d6c899 
PoC :D still a random sound. but it could be played when wo do the notifications aswell. like here: #528

closes #338
@dni dni force-pushed the notifications branch from 57c0939 to dcfba62 Compare May 6, 2024 07:25
@dni dni changed the base branch from main to sound May 6, 2024 07:49
@dni dni force-pushed the notifications branch from dcfba62 to fbc3860 Compare May 6, 2024 07:50
@dni dni requested a review from kilrau May 6, 2024 07:50
dni added a commit that referenced this pull request May 8, 2024
@dni
feat: play sound on successful swaps
7f911ce 
PoC :D still a random sound. but it could be played when wo do the notifications aswell. like here: #528

closes #338

add to settings

css opsi

play sound on enabling

remove i18n
dni and others added 16 commits May 23, 2024 15:09
@dni
feat: add browser notification
b0cc2b5 
used when a swap is claimed or refunded.
initially tries to enable notfication but wont popup because it needs
user permission.
added one ask a create swap click and one on refund. could be added to
amount input or
address input aswell but needs a userevent to popup
@dni
add sound
a953ce5
@dni
notify on submarine swap aswell
03fb497
@dni
fixup!
daf81c6
@kilrau @dni
fix: pay_invoice_to string EN
c272ccd
@dni
prettier
f2e4532
@dni
error message if notifications are denied
8a6d67a
@kilrau @dni
fix: strings
d859418
@kilrau @dni
chore: run prettier
9f5d472
@kilrau @dni
fix: align wording for audio notifications
cf01fd2
@kilrau @dni
fix: align logging tooltip strings
e3fd1c5
@dni @michael1011
Update src/utils/notification.ts
2d1e079 
Co-authored-by: michael1011 <[email protected]>
@dni @michael1011
Update src/components/BrowserNotification.tsx
53ec2d5 
Co-authored-by: michael1011 <[email protected]>
@dni @michael1011
Update src/components/BrowserNotification.tsx
888ba08 
Co-authored-by: michael1011 <[email protected]>
@dni @michael1011
Update tests/setup.ts
0343c14 
Co-authored-by: michael1011 <[email protected]>
@dni @michael1011
Update src/context/Global.tsx
3d66523 
Co-authored-by: michael1011 <[email protected]>
@dni dni force-pushed the notifications branch from c280e63 to 3d66523 Compare May 23, 2024 13:09
@kilrau
Copy link
Member

kilrau commented May 23, 2024 •
edited
Loading

2. Notifications for all failed swaps, right now it seems there are no notifications for failed swap states:

Did you add notifications for failed swaps? I didn't test this earlier and now it looks like I broke my regtest again and can't test rn

@kilrau kilrau mentioned this pull request May 23, 2024
Open
@kilrau
Copy link
Member

kilrau commented May 23, 2024

2. Notifications for all failed swaps, right now it seems there are no notifications for failed swap states:

Moved into separate issue #601, from my side ok to merge this without

michael1011
michael1011 approved these changes May 23, 2024
View reviewed changes
src/i18n/i18n.ts Outdated Show resolved Hide resolved
@kilrau kilrau merged commit e5461f7 into main May 23, 2024
4 checks passed
@kilrau kilrau deleted the notifications branch May 23, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kilrau kilrau kilrau approved these changes

@michael1011 michael1011 michael1011 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dni @kilrau @michael1011