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

Refactored Onboarding #223

Merged
merged 21 commits into from
Oct 31, 2023
Merged

Refactored Onboarding #223

merged 21 commits into from
Oct 31, 2023

Conversation

overheadhunter
Copy link
Member

@overheadhunter overheadhunter commented Aug 21, 2023

This updates the "setup" wizard, following feedback from usability tests. It is now a single stage wizard emphasizing the importance of the setup code.

Bildschirmfoto 2023-08-21 um 17 26 15

While it still allows changing the browser name, it applies a sensible default based on the user agent string:

var match = navigator.userAgent.toLowerCase().match(/(android|iphone|opr|edge|chrome|safari|firefox)/) || [''];
switch (match[0]) {
case 'android': return 'Android';
case 'iphone': return 'iPhone';
case 'opr': return 'Opera';
case 'edge': return 'Edge';
case 'chrome': return 'Chrome';
case 'safari': return 'Safari';
case 'firefox': return 'Firefox';
default: return 'Browser';
}

Furthermore it introduces a checkbox to raise more awareness of the significance of the setup code.

All labels are still WiP, most notably the setupCode = 'Account Key'. We're still looking for a good phrase which satisfies these requirements:

  • it is concise
  • it transports the message that it should be kept secret
  • it explains what it is used for (linking apps to the user account)
  • it does not contain "password" or "passphrase" to avoid confusion with the IDP's account password

This also fixes an "invalid state" where the browser still has a key pair but got deleted from the user's profile. Re-authorizing the same browser does now work again.

Tasks

Edit tasklist title
Beta Give feedback Tasklist Tasks, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. one can still open /app/setup despite the browser already being set up
    Options
  2. remove unique constraint for device names
    Options
  3. add "not you? change user" link
    Options

@overheadhunter overheadhunter added type:enhancement Improvements on an existing feature misc:frontend labels Aug 21, 2023
@overheadhunter overheadhunter added this to the 1.3.0 milestone Aug 21, 2023
@overheadhunter
Copy link
Member Author

overheadhunter commented Aug 21, 2023

German translation is mostly still missing, but we should agree on the English labels first.

@overheadhunter overheadhunter changed the base branch from feature/refactored-access-grant to develop August 22, 2023 14:23
Copy link
Member

@tobihagemann tobihagemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to test this, but I can't get past the following error with a fresh installation:

Fetching data failed
Cannot read properties of undefined (reading 'publicKey')

The error occurs here:

https://github.com/cryptomator/hub/pull/223/files#diff-3e773d478d54a16170ac8b7b79f6156a1da1548a34a76de0062c18c517bdbcebR221

Console log:

Retrieving setup information failed. TypeError: Cannot read properties of undefined (reading 'publicKey')
    at BrowserKeys.id (crypto.ts:491:89)
    at fetchData (InitialSetup.vue:221:41)

@overheadhunter
Copy link
Member Author

Cannot read properties of undefined

I guess we need some "create or load" logic here... Looking into it tomorrow!

@infeo
Copy link
Member

infeo commented Aug 29, 2023

I looked at it from UX side (no real code review). What came to my mind is:

  • Print button for account key
  • If there is a warning sign, it should also be a warning color imho. Either
    • replace by info sign
    • change color
  • first paragraph should be shortened:

This is your first login to Hub. We created your personal Account key:
[KEY]
Bla bla store it safely...

  • Maybe underline the Browsername to let the users notice its clickable?

Also, what do you think about a redirect to the user profile page after onboarding? That way the user also finds the account key.

@overheadhunter
Copy link
Member Author

I looked at it from UX side (no real code review). What came to my mind is:

  • Print button for account key

Can be a follow-up PR. Keep in mind that other than a recovery key, the account key can be changed any time. Not sure if a print-out is the best storage option. Instead it would be nice to optimize the page for password managers to pick up the password and offer to store it?

  • If there is a warning sign, it should also be a warning color imho. Either

    • replace by info sign
    • change color

I'm still looking for a completely different iconography here. Ideally, we should put the focus on "what the setup code is used for". Maybe we can add some illustration of various devices and change the text to something like:

You need your Account Key to authorize access to your account from other browsers, apps or devices. You can see a list of authorized devices on your profile page. This browser will appear under the name Chrome 🖊️.

  • first paragraph should be shortened:

This is your first login to Hub. We created your personal Account key:
[KEY]
Bla bla store it safely...

Experience tells me that there is a maximum amount of text that users can handle before they switch to a "just click next" behaviour. In order to split up all relevant information into digestible chunks, I decided to use the space above the setup code to explain WHAT it is and the part below to explain its PURPOSE.

"Hi, this is a Plumbus. [picture] It can be used for ..."

  • Maybe underline the Browsername to let the users notice its clickable?

Since we want to put the focus purely on the account key (as per feedback from usability tests with the old form), it was my intention to find a balance between completely hiding this feature and still offering users looking for it a way to easily recognize it. I tried underline before but imo it was too much. So I deemed monospace + pen icon sufficient. But this calls for another round of usability testing, specifically asking users to rename their browser during initial setup.

Also, what do you think about a redirect to the user profile page after onboarding? That way the user also finds the account key.

Yes, probably a good idea.

@overheadhunter
Copy link
Member Author

Next revision, breaking the text down in even smaller chunks of information:

Bildschirmfoto 2023-08-30 um 16 36 07

@infeo
Copy link
Member

infeo commented Aug 31, 2023

ilikeit

Looks good! My thoughts:

  • Should we mention, that the account key can be viewed from authorized devices in the profile page? This is quite the lengthy information, don't know if there is space for that
  • "... list of authorized apps and browsers on your profile ..."
  • icon of third item should match the device type browser icon
  • Wording for the first sentece. I would stick to directly address the user, how about "This is your first login to Cryptomator Hub. We created your unique, secret account key for you:" ?

@tobihagemann
Copy link
Member

Is this intentional that you're moving away from the "device" wording? It has somehow been replaced with "app" (e.g., other apps or authorized apps). On the profile page, the heading is "Authorized Devices and Browsers". Do you think that "Authorized Apps and Browsers" makes more sense?

I actually like the first sentence with "every user gets a unique ...". It's short and gives you a broader context that you're not the only one.

@overheadhunter
Copy link
Member Author

Is this intentional that you're moving away from the "device" wording? It has somehow been replaced with "app" (e.g., other apps or authorized apps). On the profile page, the heading is "Authorized Devices and Browsers". Do you think that "Authorized Apps and Browsers" makes more sense?

I'm still experimenting with the wording. "Device" was great when it was reasonable to assume there will only ever be one app (namely the Cryptomator app) per device. When we added browsers, this concept crumbles. Let's assume we stick with "device", a user might name their browser "MacBook". Later, when attempting to add the desktop app, they may run into naming conflicts.

And yes, if we agree on a good name, we should rename it in all places (but maybe keep the device-based name presets in the Cryptomator apps).

@tobihagemann
Copy link
Member

Some questions on the open tasks:

one can still open /app/setup despite the browser already being set up

You added // TODO: move "already set up" case to setup component. Is there any reason why we shouldn't solve this like before, using a beforeEnter?

if we keep the unique constraint for device names, we need a proper error text

I'm inclined to remove the constraint. What do you think?

redirect to profile?

I'm against this. Users should be redirected to the "main" page after setup so that they can get familiar with managing vaults. If we find out that users are still confused by the account key after this refactoring, we can discuss this further.

@overheadhunter
Copy link
Member Author

one can still open /app/setup despite the browser already being set up

You added // TODO: move "already set up" case to setup component. Is there any reason why we shouldn't solve this like before, using a beforeEnter?

iirc, the beforeEnter caused overhead on every single routing event. But there is lots of potential for improvement here (i.e. keeping certain user-related state in-memory).

if we keep the unique constraint for device names, we need a proper error text

I'm inclined to remove the constraint. What do you think?

I agree. @infeo opposed, maybe he can add some arguments? 😉

redirect to profile?

I'm against this. Users should be redirected to the "main" page after setup so that they can get familiar with managing vaults. If we find out that users are still confused by the account key after this refactoring, we can discuss this further.

ack

@infeo
Copy link
Member

infeo commented Sep 8, 2023

I agree. @infeo opposed, maybe he can add some arguments? 😉

Not technically. It is just the feeling, as a user looking at my devices, i could see several entries named "Browser XY". But we have for distinction the creation date, right? If so, we can drop it imho.

@tobihagemann
Copy link
Member

In my opinion, we need more device metadata for better distinction (e.g., last access date, last IP address, operating system). But yeah, then let's remove the name constraint and think about this later.

tobihagemann and others added 4 commits September 8, 2023 11:27
Copy link
Member

@SailReal SailReal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't merge yet, found some bugs I'm currently fixing, see e.g.

Screencast.from.2023-10-30.14-48-35.webm

* we need to load the user with it's devices
* recovering the key must always be done if the device id is not known to Hub
@SailReal
Copy link
Member

Before I adjust this, what do you think, but IMO the "register first device" screen and the "register another device" screen should look more similar?

@SailReal SailReal merged commit 6ed5c63 into develop Oct 31, 2023
5 checks passed
@SailReal SailReal deleted the feature/onboarding branch October 31, 2023 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
misc:frontend type:enhancement Improvements on an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants