Skip to content

Conversation

@chedieck
Copy link
Collaborator

@chedieck chedieck commented Apr 15, 2025

Related to #300

Description

Changes the logic in order to make the onSuccess function still fire for a closed PayButton, as long as it has been opened once since page load.

Test plan

Make sure not only #300 is working well, but also sideshift altpayments & Widgets, since the changes also affect them.

@chedieck chedieck marked this pull request as draft April 15, 2025 19:15
@lissavxo
Copy link
Collaborator

lissavxo commented Apr 16, 2025

I tested these cases ->

  1. Button any amount
    trigger success when opened [yes]
    trigger success when closed [no]
  2. Button with amount set
    trigger success when opened [yes]
    trigger success when closed [no]
  3. Widget any amount
    trigger success [yes]
  4. Widget with amount set
    trigger success [yes]

I also noticed altpayment is not working properly It freezes on loading step
Screenshot 2025-04-16 at 14 35 01

@chedieck chedieck marked this pull request as ready for review April 16, 2025 18:46
@chedieck chedieck force-pushed the fix/keep-socket-open-on-closed-button branch from 5d2730e to 9f6c90a Compare April 16, 2025 18:47
@Klakurka Klakurka requested review from Klakurka and lissavxo April 17, 2025 04:44
@lissavxo
Copy link
Collaborator

It is not working properly, I was able to trigger the payment success behavior once but the other times I tried I can only hear the sound the dialog doesn't open

lissavxo
lissavxo previously approved these changes May 15, 2025
Copy link
Member

@Klakurka Klakurka left a comment

Choose a reason for hiding this comment

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

We need to make a decision around how to handle the user clicking multiple buttons on the page before paying.

If I open one button dialog, close it, then open a 2nd different one and pay to that, the first one will open up on top of my active button dialog.

One option is to only ever keep the last connection open. If somebody opens a new dialog, kill the old one.

Another option is to continue having it work as-is but if I pay to button #2, don't trigger #1 to open in my face.

Either option is good with me.

Klakurka
Klakurka previously approved these changes May 26, 2025
@Klakurka
Copy link
Member

Conflicts.

@chedieck chedieck merged commit a317df4 into master May 28, 2025
1 check passed
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.

4 participants