Skip to content
This repository has been archived by the owner on May 12, 2022. It is now read-only.

Privacy mode by default UI #54

Closed
omnat opened this issue Apr 2, 2019 · 32 comments
Closed

Privacy mode by default UI #54

omnat opened this issue Apr 2, 2019 · 32 comments
Assignees
Labels
extension Related to extension

Comments

@omnat
Copy link
Collaborator

omnat commented Apr 2, 2019

Design: https://www.figma.com/file/foWeMNl8hQNDnxBjf2fFcbhd/privacy-mode-notice?node-id=0%3A1

Dev: MetaMask/metamask-extension#6352

@omnat omnat added the extension Related to extension label Apr 2, 2019
@omnat omnat added this to the Sprint 4.1 - 4.12 milestone Apr 2, 2019
@cjeria cjeria self-assigned this Apr 15, 2019
@bdresser
Copy link

@cjeria the mock you added to the figma looks good! (pasting below because i had to do some weird webGL thing to open the figma file)

I think we can make the copy a little clearer. Something like:

Having trouble connecting to this dapp? MetaMask now enables Privacy Mode by default. If this site doesn't support Privacy Mode, click below to share your address. (Read more...)

Button: Share my address

Could we also include an X in the corner or a dismiss option?

Screen Shot 2019-04-24 at 2 51 29 PM

@bdresser
Copy link

From design sync

  • have the CTA include context: "Share your address with cookie.com"
  • maybe move notification toast to the top?

@omnat
Copy link
Collaborator Author

omnat commented Apr 29, 2019

When the user can't connect to a dapp, but a dapp is trying to.. this prompt opens automatically, right? To pro-actively tell the user what's happening and guide them towards action.

As discussed earlier, I am assuming that the user doesn't have to self-initiate opening the Metamask modal.

@cjeria
Copy link
Contributor

cjeria commented Apr 29, 2019

@omnat Unfortunately, the user will have to open the MetaMask extension to see this prompt (it's not a popup). We've discussed other options like using a pop up instead (like the Connect pop up) which sounds like it's feasible to do, however, the reason we decided against it if I recall, was that it was too similar to the proper Connect Request UX.

Looking at all methods for accomplishing the same goal (connecting to a dapp) we have:

  • Connect Request
  • Signature Request
  • Force Connect

Seems redundant IMO and we should consider unifying them.

@cjeria
Copy link
Contributor

cjeria commented Apr 29, 2019

image

@omnat
Copy link
Collaborator Author

omnat commented Apr 29, 2019

Got it! Thanks @cjeria
Agree with the unifying effort related to #80

@cjeria
Copy link
Contributor

cjeria commented May 1, 2019

Adding updated design.

Changes include:

  • Slightly updated copy
  • Centered button link and added site URL

Any final feedback is welcomed.

@bdresser
Copy link

bdresser commented May 1, 2019

This looks really great @cjeria !

Maybe bold the first line rather than "Privacy Mode"? Even though it's big and yellow, good to be super clear about the topic.

@bdresser
Copy link

bdresser commented May 1, 2019

Could we also add a 1 to the metamask icon in the browser bar so users know there's something in there?

@bdresser
Copy link

bdresser commented May 1, 2019

cc @danfinlay @whymarrh this look okay to y'all?

@danfinlay
Copy link

danfinlay commented May 8, 2019

The current design is very wordy, and is tucked at the bottom of the window, and yellow, all of which I think combine towards making it seem unimportant, and easy to ignore.

Since MetaMask is an account manager, and its primary usage is logging into sites, I think we should treat this UI with more primacy.

I'd rather we embrace the patterns of usual account menus and just replace this whole yellow box with a "Log In to [SITENAME]" button, closer to the top of the popup.

login-pages-annotated (1)

Technical Qs

Also I'm not sure we are able to identify the currently open tab's URL without requesting a new permission, so we might also need to design a flow for how we ask for the new tabs permission on the fly. @whymarrh should be able to answer.

@danfinlay
Copy link

danfinlay commented May 8, 2019

All that said, if this is just MVP, and we're about to revisit this for login-per-site, then I would say go ahead with it.

At that point, we'll also want the ability to let the user select the account to sign in with, and so an account list might be the better "you're not logged in" view. Maybe something like this, from MetaMask/metamask-extension#3383

account list login view

@bdresser
Copy link

bdresser commented May 8, 2019

so @danfinlay you're ok with the design for MVP? with the suggestion that we make it more visually prominent if possible?

I do want to get this in production separately / before log-in per site

Also, we should consider the flow where the user has MetaMask locked -- would they type in their password, then click "share my address" ?

Needing to request a tab permission would be super not ideal - @danfinlay you saw whymarrh's response here (MetaMask/metamask-extension#6384) right?

@danfinlay
Copy link

you're ok with the design for MVP? with the suggestion that we make it more visually prominent if possible?

Yeah, particularly because:

I do want to get this in production separately / before log-in per site

Needing to request a tab permission would be super not ideal - @danfinlay you saw whymarrh's response here (MetaMask/metamask-extension#6384) right?

I hadn't seen the latest, but it still appears aligned with what I'm saying: While we can "detect if this site ever touched it", we don't know what the "current selected tab" is, so we can have a list of sites that have tried to access accounts (a method Whymarrh and I seem to agree is already imperfect, and has false positives), but we can't know which site a user is looking at when they open MetaMask.

@bdresser
Copy link

bdresser commented May 8, 2019

I think it'd be safe to just show the yellow notification if the user has any legacy dapp open in any tab to err on the side of over-communication & awareness

@danfinlay
Copy link

So if there are multiple "legacy detected" dapps (these are any dapps that check accounts before calling connect(), and include false positives), we should just stack these login boxes?

@cjeria
Copy link
Contributor

cjeria commented May 8, 2019

I have a hunch that this current design will work just fine, but we won't know until it's tested in the real world. I don't think @bdresser was suggesting that the notifications will stack up per legacy dapp, there would only be one if the tab you are currently on is a legacy dapp. If two tabs are legacy dapps still only show one notification but with the corresponding URL of the tab/dapp you're focused on. If that's not possible, we could instead show something like "3 dapps would like to connect to you're account" and expand to show which dapps with a ✔️ or ❌next to each. In any case, I'd push for a coded up prototype of this design first to test its functionality and iterate on the UX from there @whymarrh

@danfinlay
Copy link

danfinlay commented May 8, 2019

there would only be one if the tab you are currently on is a legacy dapp.

This was the point we were discussing, we can't detect what your current tab is without an additional permission, and requesting it might benefit from some extra design consideration (do we want to let the user know why they're about to be asked for a new permission?)

If that's not possible, we could instead show something like "3 dapps would like to connect to you're account" and expand to show which dapps with a ✔️ or ❌next to each.

This is more realistic, and we might want design for that, and the subsequent screen where the user selects which sites to log into.

However, (and I feel like a broken record) this legacy dapp detection is not perfect (it has false positives), and that means this warning will appear for sites that are "doing everything right", and I think that's unfortunate. It paves our basic dapp support into hard-to-avoid jank.

One way to make this whole thing feel less janky is to avoid terms like "privacy mode" and "this app doesn't support X", and just say Log In. This button applies to old and new sites alike, and just requires that we account for when to ask for the tabs permission.

@bdresser
Copy link

bdresser commented May 9, 2019

Really want to avoid an additional permission - our active installs took a big hit last time.

I'm in favor of loosening the copy, but still think it's wise to tell people this friction is because we're trying to protect their privacy. How about:

Having trouble logging in to this dapp? MetaMask now enables Privacy Mode by default. Click below to share your address

I don't think false positives are a huge risk because if everything works smoothly, the user will likely not decide to open the extension from the nav-bar and they'll never see this message.

@cjeria
Copy link
Contributor

cjeria commented May 28, 2019

Here's one of the latest ui design for this. Feedback is welcomed!
@bdresser @danfinlay
image

@bdresser
Copy link

  • would change text to "Share your address with [dappname]"
  • let's show a badge on the icon in the browser bar, maybe a ! instead of a number if that's possible

@cjeria
Copy link
Contributor

cjeria commented May 29, 2019

Took another pass at this notification and created a light version for comparison. They are both intended to stand out and encourage user engagement. Would like to hear feedback about which direction to move forward with @omnat @bdresser @danfinlay

image

@bdresser
Copy link

I don't have feelings on staying in the design system vs doing something outside of it, but I do think the user should be able to interact with the wallet behind/around the modal - so for that reason I favor the functionality shown in A @cjeria

@omnat
Copy link
Collaborator Author

omnat commented May 29, 2019

I am assuming this applies to current dapp that user is on (active tab). So if 2 legacy dapps are open on 2 tabs, only the active tab dapp connect request is shown in extension.

Also assuming these requests don't pile up ("3 dapps are requesting you to share your address") - only the one in which user has context for (i.e., active tab)

Are the above 2 assumptions correct?

  • In favor of Option B. If above assumptions are correct, user can't use metamask as intended in context of the dapp that's still waiting on permission to see address. So better to enforce response to this permission.

  • Like the tooltip. What's the copy for tooltip? I'd suggest to explain what the consequence of sharing address is. E.g.,
    Sharing your address with [dapp_name] will allow you to interact with this dapp. This permission is to protect your privacy by default. This doesn't allow [dapp_name] to view your wallet funds & transaction history.

@danfinlay
Copy link

I like the functionality of A, but could we do it while avoiding spending time extending the design system? This is going to be a temporary thing until login per site.

Also I’m curious what the x button or ignore should do. Is there another way to log in after dismissing these, or is it permanent? Kinda seems like we might want to make it non-dismissible.

@rekmarks
Copy link
Member

rekmarks commented May 30, 2019

I second @danfinlay re: login-per-site, and I also think we should keep in mind ocap permissions with respect to the amount of effort put into this. "Sharing an address" will be just one permission among many in the not-particularly-distant-future.

@bdresser
Copy link

bdresser commented May 30, 2019

@danfinlay @omnat given that only about 30% of DAU are completing on-chain tx, there is definitely a usecase for folks opening the extension without sharing their address ( I realize they may interact with dapps outside of just submitting tx, but point stands). Users may also need to switch accounts or networks. I think people should definitely be able to interact with the extension even when the popup is showing.

Are the above 2 assumptions correct?

@omnat yes!

good call on tooltip copy, how about

MetaMask only shares your address with sites you approve. Sharing your address with [dapp_name] will allow you to interact with this dapp.

Re: design system, I don't think the dev overhead to style the buttons differently is huge effort

@cjeria
Copy link
Contributor

cjeria commented May 30, 2019

I think there's a happy middle ground between A and B options per the feedback provided above (thank you team!).

The X was to dismiss this alert since I expect users will get annoyed seeing it if they decide not to connect.

Here's a simplified iteration using a flash alert component (defined in the design system) with a share button.

There no dismiss option and no new components to be created just for this. Alternatively, however, we could just do this same design in the dark mode style if that seems better for this specific use-case?

Screen Recording 2019-05-30 at 01 02 PM

@bdresser
Copy link

Love the update! Would also be cool with dark mode your call!

Still think the copy should remain "Share your address with dappname.com" – the inclusion of another verb makes it a litttttle bit more confusing imo. But it's a weakly held opinion so I'm cool w this if you want to keep.

@cjeria
Copy link
Contributor

cjeria commented May 30, 2019

I'm digging the dark version a bit more I think. I'd call this design done unless there are any final thoughts?

Screen Recording 2019-05-30 at 04 37 PM

@bdresser
Copy link

bdresser commented Jun 5, 2019

Only last thing is that we should remove the last line in the tooltip because although I get that it's saying our default protects your wallet activity, it sounds like clicking "share" will protect your wallet activity, which it doesn't.

Also @cjeria where does this final version live in the team Figma?

Other than that, good to go!

@cjeria
Copy link
Contributor

cjeria commented Jun 6, 2019

Updated tooltip language @bdresser
image

Link to the figma file
https://www.figma.com/file/foWeMNl8hQNDnxBjf2fFcbhd/privacy-mode-notice?node-id=82%3A590

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
extension Related to extension
Projects
None yet
Development

No branches or pull requests

6 participants
@danfinlay @cjeria @bdresser @rekmarks @omnat and others