ubiquity / pay.ubq.fi

Generate and claim spender permits (EIP-2612)
https://pay.ubq.fi
8 stars 36 forks source link

better mobile feedback #220

Closed Keyrxng closed 4 months ago

Keyrxng commented 5 months ago

Non-Web3 friendly mobile browsers need better feedback.

additional context

Had already given up. On Brave browser it doesn't even throw an error it just loads for ever.

Originally posted by @jordan-ae in https://github.com/ubiquity/devpool-directory-bounties/issues/24#issuecomment-2040538335

0x4007 commented 5 months ago

The intended solution was actually to hide the claim button if the permit is not claimable due to no web3 support.

It's simpler and cleaner (just a couple lines of code)

I thought that was already implemented, so this proposal doesn't seem to add value beyond that.

Keyrxng commented 5 months ago

checkout the PR QA screenshots Pav, the claim buttons are hidden (besides extra details). I went for persistent over repeatable as yeah it seemed like installing a fake door or something 😂

The value that this adds is that the toast never expires so that the feedback is persistent. Should they blink and the miss the first toast they'd have no further feedback than to either wait on the loading state changing (which it never would) or refreshing the page to fire another toast.

The value the PR adds is that now on non-web3 browsers the logic does not hang/throw because of window.ethereum whereas before this PR, attaching a listener to it or using it in checks & effects was causing the infinite loader.

Had already given up. On Brave browser it doesn't even throw an error it just loads for ever.

The above was true for any non-web3 browser (desktop or mobile) prior to this PR, I have tested on mobile Safari, Brave & MM as well as Brave on desktop (icognito with no wallet installed)

ubiquibot[bot] commented 4 months ago
+ Evaluating results. Please wait...
ubiquibot[bot] commented 4 months ago

[ 20.3 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment14.9
ReviewComment315.4
Conversation Incentives
CommentFormattingRelevanceReward
The intended solution was actually to hide the claim button if t...
4.90.684.9
> | Preview Deployment | > | ------------------ | > | [469161d6d...
3
a:
  count: 1
  score: "1"
  words: 1
td:
  count: 1
  score: "1"
  words: 4
0.263
Hey @Keyrxng thanks for your pull. Apologies for being pedant...
9.30.1059.3
@gentlementlegen the simple solution is to hard code a permit ma...
3.10.673.1

[ 16.1 WXDAI ]

@gentlementlegen
Contributions Overview
ViewContributionCountReward
ReviewComment116.1
Conversation Incentives
CommentFormattingRelevanceReward
> > Preview Deployment > > > > > > > > > > 469161d6d9e1...
16.1
a:
  count: 2
  score: "2"
  words: 2
li:
  count: 2
  score: "2"
  words: 89
code:
  count: 2
  score: "2"
  words: 3
0.4816.1

[ 217.2 WXDAI ]

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueSpecification16.8
IssueTask150
IssueComment133.2
IssueComment10
ReviewComment384.8
ReviewComment342.4
Conversation Incentives
CommentFormattingRelevanceReward
Non-Web3 friendly mobile browsers need better feedback. - it ...
6.8
li:
  count: 2
  score: "2"
  words: 16
16.8
checkout the PR QA screenshots Pav, the claim buttons are hidden...
33.2
code:
  count: 1
  score: "1"
  words: 2
0.933.2
checkout the PR QA screenshots Pav, the claim buttons are hidden...
-
code:
  count: 1
  score: "0"
  words: 2
0.9-
I spent a little bit last night trying to hack together a soluti...
51
li:
  count: 2
  score: "4"
  words: 21
code:
  count: 3
  score: "6"
  words: 3
0.251
> Apologies for being pedantic No I appreciate it, it can't b...
5.40.435.4
1. Persistent non-expiring toast, 5 secs is a long time but fore...
28.4
li:
  count: 2
  score: "4"
  words: 23
code:
  count: 2
  score: "4"
  words: 4
0.4528.4
I spent a little bit last night trying to hack together a soluti...
25.5
li:
  count: 2
  score: "2"
  words: 21
code:
  count: 3
  score: "3"
  words: 3
0.225.5
> Apologies for being pedantic No I appreciate it, it can't b...
2.70.432.7
1. Persistent non-expiring toast, 5 secs is a long time but fore...
14.2
li:
  count: 2
  score: "2"
  words: 23
code:
  count: 2
  score: "2"
  words: 4
0.4514.2

[ 7.2 WXDAI ]

@rndquu
Contributions Overview
ViewContributionCountReward
ReviewComment17.2
Conversation Incentives
CommentFormattingRelevanceReward
@Keyrxng Questions: 1. This PR adds a toast "Please use a web3 ...
7.2
li:
  count: 3
  score: "3"
  words: 42
0.667.2
gentlementlegen commented 4 months ago

Did anyone else had an error on collect? It worked, but got a toast with an RPC error afterwards.

rndquu commented 4 months ago

Did anyone else had an error on collect? It worked, but got a toast with an RPC error afterwards.

@Keyrxng

Keyrxng commented 4 months ago

@gentlementlegen I never ran into an error toast when claiming right there, all went smooth.

If the claim worked and the TX succeeded it's hard to guess what caused it, do you remember anything about the error?


I often have logs open when claiming, it typically logs a small error for duplicate attempts to change the wallet network if a request is already pending, then another if you change the network something along the lines of underlying network has changed.

My network was already on Gnosis so I wasn't prompted this time but I'm not sure beyond that as a guess.

gentlementlegen commented 4 months ago

@Keyrxng Not much logs except saying that the RPC provider gave a "No network" error. Might have been unlucky on the selected RPC. Nonetheless the transaction worked, if it is just me then I'll blame the RPC for being slow / down.

Keyrxng commented 4 months ago

That's the thing the handler should never ever return a faulty/unreachable endpoint. It pops RPCs that don't reply and then supplies the quickest to respond.

Could you try and capture more info if it happens again, I'll also try to repro with future permits.