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

better mobile feedback #220

Closed
Keyrxng opened this issue Apr 11, 2024 · 9 comments · Fixed by #221
Closed

better mobile feedback #220

Keyrxng opened this issue Apr 11, 2024 · 9 comments · Fixed by #221

Comments

@Keyrxng
Copy link
Member

Keyrxng commented Apr 11, 2024

Non-Web3 friendly mobile browsers need better feedback.

  • it should not be stuck in a loading state
  • feedback should be either repeatable or persistent

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 ubiquity/devpool-directory-tasks#24 (comment)

@0x4007
Copy link
Member

0x4007 commented Apr 13, 2024

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
Copy link
Member Author

Keyrxng commented Apr 13, 2024

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)

Copy link

ubiquibot bot commented May 27, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot bot commented May 27, 2024

[ 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
Copy link
Member

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

@rndquu
Copy link
Member

rndquu commented May 27, 2024

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

@Keyrxng

@Keyrxng
Copy link
Member Author

Keyrxng commented May 27, 2024

@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
Copy link
Member

@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
Copy link
Member Author

Keyrxng commented May 27, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants