search

MetaMask / snaps

Extend the functionality of MetaMask using Snaps
https://metamask.io/snaps/
Other
712 stars 549 forks source link

Site specific permissions are not shown in the popup UI #305

Closed holantonela closed 2 years ago

holantonela commented 2 years ago

We currently exposed which site is connected to the Snap accessible via home.html#settings/snaps-list.

It seems expected that we also reflect that interaction in the popover UI.

Screenshot 2022-03-21 at 12 30 52
Gtonizuka commented 2 years ago

As @holantonela said we should show a green dot when the extension is connected (atm it says ‘Not connected’ as per screenshot above) and have a easy way to disconnect like we do with normal dapps

FrederikBolding commented 2 years ago

We can easily detect that the origin is connected to a snap (I already wrote code for this). My only question would be what to show in the popover then and is it confusing that we use this UI which also normally shows whether an account is connected to a page.

cc @rekmarks

FrederikBolding commented 2 years ago

Also @holantonela are you on an old build? On the current version I see the permissions: image

rekmarks commented 2 years ago

So @holantonela's screenshot might be from prod or a slightly older build, because we did have a bug where the list was not properly populated. I think we definitely want to show the complete list of permissions for the website, and the original extension designs for this component were made with exactly this use case (multiple different permissions) in mind.

IMO, the unresolved question is whether we want the "connected" indicator to become green for any permission or just eth_accounts. This is a design question that I will leave for @holantonela and @rachelcope.

FrederikBolding commented 2 years ago

The copy is interesting then. It seems weird to me to show "MetaMask is not connected to this site....." and then show the permissions the origin has. It might just be me though 🤷‍♂️

rekmarks commented 2 years ago

We may want to switch the copy, but IIRC the meaning of "connected" is flexible enough to support our use case. Either way, that's another thing for our friends in design to weigh in on!

rekmarks commented 2 years ago

We will close this issue because snap permissions are displayed in the list. There are other unresolved questions regarding snap permissions in the current extension UI, but those will be covered in other issues.