-
Notifications
You must be signed in to change notification settings - Fork 38
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
Rework Dapp Connection Screen #3340
Comments
Dapp Connection Screen CASE 1. The dApp name can be fetched, the URL can be fetched, dApp favicon/logo can be fetched and displayed. CASE 2. The dApp name can not be fetched, dApp favicon/logo can not be fetched and displayed so we are replacing it with the generic label "DAPP" and generic icon. CASE 3. Unknown URL! Rare case that can still occur. URL cannot be fetched by Superhero Wallet. In this case neither the favicon nor the dApp name will be available. Most probably it's a risky app with basically "unknown source". User should be warned about potential risks. |
@sotiris @CedrikNikita IMO there's an issue with the way we grant permission to third party dApp to view not only the current account but also all "current active accounts" in the future. In other words the dApp asks only once for permission to view current address but it will be able to view all "currently active addresses" until Superhero Wallet is connected. @CedrikNikita correct me if I'm wrong here. I am concerned that the user is not aware what's happening. As a user I'd expect to give permission affecting only my current account address when connecting, logging in, providing permission to a dApp or platform. I might want to use different accounts for different platforms and purposes. I would not expect to grant access to the dApp to view all my wallet account addresses and that's what we are currently doing. I think we should reconsider this behavior and ask for permission each time the currently active account is changed. |
@onvisions dapp requests access to one address is not this the case? |
As far as I understood this is not the case and that's why I raised my concerns from user perspective. Let's sync with @CedrikNikita to be on the same page if it's needed. As far as the current task is concerned I have created the design according to request: |
@onvisions , @smaroudasunicorn - I confirm that the DAPPs are actually connecting with the wallet, not the addresses. For AE if a DAPP is connected whenever user changes the active account the DAPP knows about that and can make an action based on this action. The best example is our testing DAPP: https://docs.aeternity.com/aepp-sdk-js/v13.3.2/examples/browser/aepp/ Try to connect and open the "Basic functionality" tab. Then open the extension and try to switch accounts. You'll see that the testing DAPP will update the active account on the fly. This is why I agree with Tsvetan that this connect modal is not fully informative. MetaMask is asking you to list the accounts you want to use for connection: |
I suggest to not put the dapp name (eg.: Superhero Dex) as a tag above the arrow. There is a limited space in this area and in my opinion putting the name there gives more risk of breaking the layout than a real value to the user. Also regarding the icon: should we create our own list of icons we can display? I'm asking because the situation is the same as with the app browser: we are not able to obtain the dapp icon.
The tags above the arrow are uppercased, so we are not able to display "dAPP". What do you think about using "DAPP"? Also the "dApp" string displayed as the name is our own label which should be used when there is no name?
Not sure if this is possible. We always have the access to the URL of the app that tries to connect. @CedrikNikita am I right? |
I agree about this one we might get rid of:
|
I agree with Bart Comments
|
OK, so now I'm waiting for updated designs. Here's working POC of my changes: #3385 |
Below an image is presented about dapp connection
We need to rework dapp connection screen. This screen is about dapp requesting from the wallet to provide address of active account. Should not seem like a transaction.
The text was updated successfully, but these errors were encountered: