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

Adds optional confetti to incoming payments in wallet #2231

Merged
merged 9 commits into from
Feb 8, 2024
Merged

Conversation

arcbtc
Copy link
Member

@arcbtc arcbtc commented Feb 2, 2024

2024-02-02.02-25-48.mp4

The confetti websocket can be added to any page with eventReactionWebocket(event_id, event_function)

For the wallet example event_id is the wallet ID, and event_function is the function eventReactionBothSidesConfetti (eventReactionFireworksConfetti also exists but is not used)

It made sense to leave room for multiple animations the developer could summon using eventReactionWebocket, or the user could specify in the profile page.

@arcbtc arcbtc requested review from dni and motorina0 February 2, 2024 02:26
@prusnak
Copy link
Collaborator

prusnak commented Feb 2, 2024

This is awesome!!!!

Does it look good on mobile too (vertical aspect ratio)?

@prusnak
Copy link
Collaborator

prusnak commented Feb 2, 2024

Needs make prettier and make bundle.

@dni
Copy link
Member

dni commented Feb 5, 2024

why are you creating an extra websocket connection? it should be enough to trigger the animation when we also close the popup after a successful payment. we should keep those connections to a minimum.

i don't think that the animation should be disabled by default its such a good feature and every wallet has it, i don't even think its should be configurable at all!

@arcbtc
Copy link
Member Author

arcbtc commented Feb 5, 2024

This is awesome!!!!

Does it look good on mobile too (vertical aspect ratio)?

2024-02-05.12-11-33.mp4

@arcbtc
Copy link
Member Author

arcbtc commented Feb 5, 2024

why are you creating an extra websocket connection? it should be enough to trigger the animation when we also close the popup after a successful payment. we should keep those connections to a minimum.

i don't think that the animation should be disabled by default its such a good feature and every wallet has it, i don't even think its should be configurable at all!

The reason I am adding the websocket is it is far superior to the SSE (which often times out). Later I will propose we use for updating the wallet balance and doing things like easily have balances in the drawer updating

Its enabled by default.

People like customising stuff, the toggle switch will later turn into a select box, where a user could select from various animations. The admin could then set what animations are available, same flow as themes really

Copy link

codecov bot commented Feb 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a73eb56) 59.23% compared to head (0cbf4ab) 59.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #2231      +/-   ##
==========================================
+ Coverage   59.23%   59.24%   +0.01%     
==========================================
  Files          60       60              
  Lines        8992     8992              
==========================================
+ Hits         5326     5327       +1     
+ Misses       3666     3665       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@talvasconcelos
Copy link
Collaborator

How does one create (and add) a new animation?

@arcbtc
Copy link
Member Author

arcbtc commented Feb 7, 2024

For now in the reactions.js file, but later I will add gif support and uploading gifs.

lnbits/static/js/base.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@motorina0 motorina0 left a comment

Choose a reason for hiding this comment

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

tACK

@motorina0 motorina0 added this to the 0.12.1 milestone Feb 8, 2024
@dni
Copy link
Member

dni commented Feb 8, 2024

can you add the library via npm and into our build?
is it this library? https://www.npmjs.com/package/js-confetti

@arcbtc
Copy link
Member Author

arcbtc commented Feb 8, 2024

can you add the library via npm and into our build? is it this library? https://www.npmjs.com/package/js-confetti

Nien! Its been changed, its also very small, so I think this approach is unnecessary

@arcbtc arcbtc merged commit c1c4eda into dev Feb 8, 2024
22 checks passed
@arcbtc arcbtc deleted the eventreactions branch February 8, 2024 13:34
@dni dni modified the milestones: 0.12.2, 0.12.1 Feb 15, 2024
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.

None yet

5 participants