-
Notifications
You must be signed in to change notification settings - Fork 8
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
Get design review on UI and UX #5
Comments
One question will be where should the settings live? Probably not |
Just uploading a video to show where it's at right now. This did require a change to the upstream plugin, which I've PR'd. Screen.Recording.2022-11-16.at.4.01.31.pm.movSome notable things, some of which need taking upstream to two-factor, some of which will probably want to be changed on WordPress.org-only.
|
Yeah that activation flow definitely has to change for w.org. It shouldn't be presented as distinct settings to enable, but as a single flow: scan the QR code, generate your backup codes, confirm a backup code. |
I think the user should still be able to choose which providers they want to use, though. That doesn't matter in practice right now, since there's only two -- and Backup Codes is only suitable as a secondary provider -- but WebAuthn will be available in the future. |
@WordPress/meta-design, in this repo we're working on adding two factor authentication to wordpress.org. Joen suggested we base the design on WPCOM, so we're working off of that (see #18 for some screenshots). We've changed things a little bit to fit w.org, but not too much. We're getting close to launching the MVP, and I think the UI is ready for review. Does anyone have time for that? If so, let me know and I can add you to the beta testers list. That will let you play around with it live on wordpress.org (preferably with a test account, but you can use your real one if you want). Just let me know which account you want to use and I'll set that up and show you where to go etc. |
While not in meta-design myself, I'm open to help test as well as improving this flow is a long-standing irk I have with the plugin that I would :1000000000: :looooooove: to see happen. |
Thanks! 👍🏻 You should be able to see it now, by going to The UI is a bit tailored to w.org, so it's on the front end, and includes the email/password fields. I don't think there are any big barriers to adapting it for the plugin/wp-admin, though. The code is in is in https://github.com/WordPress/wporg-two-factor/tree/trunk/settings Let me know what you think! |
Thank you for the ping. Would it be possible to see a video of the flow like Dion shared earlier? That might be an easy light weight way to provide feedback. Otherwise, I should have username |
No problem: 2fa.design.review.mov |
Thank you, that's incredibly useful! Let me give a ton of feedback, just to track it here, but I don't know that we need all of this addressed for the first version — some of the feedback I'll give is likely going to be related to the larger Support forum redesign to come down the line. So I'll also refer to your input on what is useful to implement first.
Generally works well. Thanks for using the new icon set. It also looks like you are using the colors from the library:
Any preference? Let me know and I'll provide the SVGs. On a separate note, the block editor is moving towards 40px inputs and buttons by default:
Small detail, the focus style here is a bit awkward.
The "Generate strong password" also untoggles the "hide password". Which I suppose makes sense, otherwise you wouldn't know what a strong password looks like. But it reinforces the idea that we need a visible toggle state for the eye, such as the icon change. This one is a bit awkward in layout. I'm also not convinced that notice needs to be red. Block editor specific, sure, but this PR adds a different visual treatment for the notice, which I think would work well here:
It's also a bit confusing that you remain on the "password" page and have to click "back". I would expect to be taken to a new step in the wizard that says "Password changed", with a "Done" or "Okay" button that then takes me to the main security checkup screen again.
And one step for the non-QR code version: Another for the text version: Hope that's useful! |
|
I agree that "enable it" is too short to stand out visually, and "it" is vague for screen readers. Linking the entire sentence slows down screen reader users, though, and has a similar effect on sighted users. More: https://www.nngroup.com/articles/better-link-labels/, https://www.nngroup.com/articles/writing-links/ I've changed it to "Your account requires extra security. Please enable two-factor authentication", but I think that could probably be refined more. It doesn't explain why the account requires extra security (because it has elevated permissions, maintains a popular plugin, etc), and it doesn't explain that their permissions have been temporarily disabled until they enable 2FA. I played around with several variations, but couldn't find anything that communicated everything they need to know without being too verbose. Do you have any thoughts? |
Er, it actually wasn't, but I've updated it to use them 👍🏻 I also updated the greys to do the same.
Done 👍🏻
Good point. It can't just be We could move the title to the left, but that wouldn't guarantee that it'd never be overridden. That might be good enough in practice, but it might also look worse on the left. What do you think? |
I don't know if @WordPress/marketing has bandwidth to look at phrasing, but I imagine they might have ideas. One of the opportunities for reduction is to put some of that text, i.e. the why, on the destination screen instead of the notice.
Snackbars in general seem to me like they should all be in a consistent fixed place, like button center or bottom left, a la the block editor. But I think there's a simpler question of whether we need a snackbar here at all. When you are successful in enabling one time 2fa, that takes you to a new page already, right? I.e. you see Backup Codes after you are succesful. That way, we could simply weave the confirmation into the prose, rather than snackbar it. |
🤔 I tried that out, but it seems a bit odd having two notices on the same screen that reference each other, but are in separate locations. (ignore the imperfect styles, this is just a rough sketch to illustrate having both on the same screen) Were you thinking of doing it differently than that? |
Oh hi! 👋 Thought I'd jump in here and see if I can help.
I can see how this one's a challenge. There's a lot that needs to be understood. Figured I'd offer a first attempt. It's not as short as the two sentences you currently have, but it is shorter than the original shown above: "Your account has elevated privileges and requires extra security before you can continue. Enable two-factor authentication now." |
I like that, thanks! |
Decision needed: what are the blocking issues here needed for the MVP launch? |
Some of these are already done, and I think I'll have time to get a few more in before we launch. The launch is just for beta users, so I think it's fine for the rest to get done in the next milestone. I went ahead and moved it to the next milestone, but LMK if anyone has any objections and we can reconsider. |
I think @renintw represented the remaining work in the following issues:
If there's nothing else, can we consider closing this ticket or removing it from the MVP milestone? |
Alright, closing this, open a new issue if some nuance was missed in the issue extraction. Thanks ya'll!! |
Having a designer review the UI and flows would help refine them, and decrease confusion for setup/recovery/etc. Google has a nice wizard when setting up hardware keys; that might provide some inspiration.
This should probably be broken into multiple tickets once a design is made. Ideally we'd contribute improvements upstream, to improve the plugin for everyone, and also because building a custom UI could lead to vulns in the future when upstream code changes.
IIRC @jeffpaul was also looking into this, so it'd be good to coordinate with him.
Related:
The text was updated successfully, but these errors were encountered: