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

🦋 Disable pay button as long as the sum up / stripe iframe is not loa… #1665

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ithiame
Copy link
Contributor

@ithiame ithiame commented Feb 7, 2025

…ded #1631

@ithiame ithiame requested a review from coyotte508 February 7, 2025 11:55
Copy link
Collaborator

@coyotte508 coyotte508 left a comment

Choose a reason for hiding this comment

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

There is not a single await between stripeLoading = true and stripeLoading = false

image

So it doesn't do anything.

You need to find a way to wait for a callback or something

Comment on lines +221 to +223
setTimeout(() => {
stripeLoading = false;
}, 3000);
Copy link
Collaborator

@coyotte508 coyotte508 Feb 7, 2025

Choose a reason for hiding this comment

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

Ideally you can listen to the DOM instead of hardcoding a delay. There is the load event for the document, and for the iframe too if you can find it.

It's what we do to download invoices or bulk invoices, listen to the dom event. For example here: https://github.com/B2Bitcoin/beBOP/pull/1660/files#diff-633ad8803814b1c6ca182165aabc08d46569cba063f15d1fc09e816072366bb0R184-R193

Searching on google I also get some answers: https://stackoverflow.com/questions/52954937/when-are-stripe-elements-ready-in-the-dom https://docs.stripe.com/js/element/events it seems there's a builtin event for that

Please take the time to look for good solutions

Copy link
Contributor

@Tirodem Tirodem Feb 7, 2025

Choose a reason for hiding this comment

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

The "blame" is on me here.
I proposed to make a 2 step implementation because this issue costs us (PVH Editions) money with issues on card payment.
I don't mind having a 3-seconds wait asap and a smart solution after.

Copy link
Collaborator

@coyotte508 coyotte508 left a comment

Choose a reason for hiding this comment

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

ok for the quickfix then I guess cc @Tirodem but please make a follow-up PR to clean that up :)

@Tirodem
Copy link
Contributor

Tirodem commented Feb 7, 2025

Yes cc @AmadouAgiltoo (but I'd be more into keeping #1631 open as the 2 version were listed on it, we can do 2 checkbox to say first part is done)

@AmadouAgiltoo
Copy link
Contributor

OK @Tirodem I'll indicate on the 331 that the quick fix by adding some seconds before display the buttom is done.
Then after we'll work for a clean implementation.

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.

🦋 Disable "pay" button as long as the sum up / stripe iframe is not loaded
4 participants