search

anza-xyz / solana-pay

A new standard for decentralized payments.
https://solanapay.com
Apache License 2.0
1.29k stars 450 forks source link

BackButton should be disabled once clicked #175

Closed flodef closed 1 year ago

flodef commented 1 year ago

In order to avoid double click and to be clear that the button has been pressed, it should be disabled once clicked on.

vercel[bot] commented 1 year ago

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

Name Status Preview Updated
solana-pay ✅ Ready (Inspect) Visit Preview Dec 11, 2022 at 10:45PM (UTC)
solana-pay-docs ✅ Ready (Inspect) Visit Preview Dec 11, 2022 at 10:45PM (UTC)
jordaaash commented 1 year ago

Can you describe what issue this is fixing?

flodef commented 1 year ago

Can you describe what issue this is fixing?

This is a basically a UI problem, it should not be possible to click twice on the button and ask for getting back (or whatever action the button is doing). That does not generate any real problem here, but having customized the app, it can generate issues later on.

jordaaash commented 1 year ago

If it isn't causing a problem, I don't see why we want to change it in an opinionated way. Perhaps more narrowly, we could just allow Button props to be passed to the BackButton component and leave this behavior up to the caller.

flodef commented 1 year ago

If it isn't causing a problem, I don't see why we want to change it in an opinionated way. Perhaps more narrowly, we could just allow Button props to be passed to the BackButton component and leave this behavior up to the caller.

Because it's not causing a problem right now does not mean it won't cause a problem in the future! It's only good UI practice. Steven Luscher just taught me about the size of the button, in order to be compliant with Apple guidelines. Same here.

For example, look at the behavior of github button when you press "Comment" below. If it was not disabled at first press, you could publish your comment 5 times thinking is was not taken into account the first time. Are you even using this "point of sale" ?? Because I am, and I can observe user's behavior everyday.

jordaaash commented 1 year ago

Hi @flodef, you opened quite a few PRs! I reviewed each PR on its own merits, and where it made sense, I asked questions about changes. I appreciate the time it takes to contribute to open source repos. In turn, I hope that you appreciate that it also takes time to review PRs and determine whether to accept a change.

This determination is based on many factors, one of which (in this case) is that the point of sale example is intended as a proof of concept example, not a production application, and is, effectively, feature complete. This means that it won't capture all use cases, present or future, it isn't intended to, and we may not accept certain kinds of changes at all.

I can see you're upset, and I accept that you may not want to open any more PRs to this repo. If you do want to, but you want to know if a change is likely to be accepted first, please open an issue and we can discuss. For the PRs you still have open, I'm happy to continue to review and accept changes that make sense.

flodef commented 1 year ago

Yes, I really do apreciate that you're taking the time to review PR. Many thanks for that. Until you said it, I didn't get it was a proof of concept, otherwise, I wouldn't have taking the time to suggest any PR. As a proof of concept, it's already good enough how it is. However, I would strongly advice you to write it in BOLD in the README.md of the Point of Sale! That could save time to other devs in the future and that costs you no time to do. As a Solana user, dev and advocate that try to onboard people, I think it's a shame that it stays as only a proof of concept. But that's not my decision to take, so let's be it.

Coming back to the bug, and to prove my point, go on the app, then recent transaction, double click on Back and TADA ! See for yourself. Screen recording 2022-12-13 02.45.21.webm