Split TOTP instruction into two steps #100

renintw · 2023-03-28T19:09:39Z

Fixes #93

This PR

Adds Intro component for totp introduction.
Fixes the screen height of each step so when switching step, no jitter would happen.
Adds AutoTabbingInput component
- Only number can be submitted
- Supports Auto Tabbing and 'backspace'
Aligns styles with design

Screencast

totp.mp4

TODO

Lint after merging.
Is there a better place for the Error Notice, or the current layout is acceptable? (screencast 0:40) @jasmussen @iandunn

Update

renintw · 2023-03-28T20:12:57Z

settings/src/components/totp.scss

+		background: #3858E9 !important;
+		border-color: #3858E9 !important;
+		color: $white !important;
+		margin-right: 0 !important;


Probably not a good idea to use that many !important, but as I don't want to build a button from scratch here, I just overrode the styles of Button from Gutenberg.

renintw · 2023-03-28T20:15:25Z

settings/src/components/auto-tabbing-input.scss

+			margin-right: 24px;
+		}
+	}
+}


In this PR, there are still some files that haven't been well formatted yet. I will use the linter to fix them all at once later in a separate commit.

StevenDufresne · 2023-03-28T23:45:25Z

settings/src/components/totp.js

+					setInputs={setInputs}
+				/>
+
+				<Button variant="link" onClick={ handleClick }>Previous</Button>


How does this differ from the "cancel" button under the 6 inputs?

Oh, right! Originally, when the cancel was clicked, it would return to the account status screen. Replacing previous for that would make the look better. Thanks!

I found that it makes sense to me to use the Cancel button to clear all 6 inputs and keep the previous button.
Commit added: Let Cancel clear all the inputs

StevenDufresne · 2023-03-28T23:59:56Z

I don't find the introduction tab does enough for it to warrant it's own tab to be honest. If it did a better job helping the setup, I would be interested in it, but since it just lists a few links, it seems like an extra interaction. I personally like how github does it:

I do very much like the input updates however. Does it make sense to split this up into those two features? One were we just update the code scan inputs and one that includes this extra "step"?

jasmussen · 2023-03-29T08:07:23Z

Thanks for the ping, thank you for the video, that's useful as I'm not able to review the checked out branch. Thank you also for the attention to detail.

First off, I think that longer term we should move towards something like this.

That is:

A new notice component
The new colors, including the blueberry and pomegrade.

For the near term, it looks a bit off to combine the blueberry with the classic WordPress blue. So you can think of the blueberry as a "spot color" — it should always be the spot color of the surrounding template. That surrounding template should eventually be updated to use blueberry, the sooner the better of course. But it's okay to keep the colors the same in the mean time.

Same for the componentry — outside of an updated notice component, it's okay to use the existing notice component. Something like this:

renintw · 2023-03-29T19:42:09Z

I do very much like the input updates however. Does it make sense to split this up into those two features? One were we just update the code scan inputs and one that includes this extra "step"?

It was suggested to splitting this up into two steps in that the original look was considered a bit packed together. I guess the main difference between us and Github is that we try to provide a few explanations and introductions about 2FA for .org here, whereas Github has a whole section dedicated to it. If we can provide more information on the introduction page, then that page may have more value. However, I personally feel that we can also simply provide the feature like Github does, without any introductory text in that maybe 2fa is common enough (maybe?) This way, we can have things on one page.

@jasmussen Do you have any thoughts on what @StevenDufresne brought up ^?
And thanks for the feedback and the clear clarification! I think I'll follow your idea that as the whole website is still in the classic WP style, and a new style may seem a bit abrupt, I'm not going to do extra adjustments going forward. However, I will add a red border when the input is wrong.

Regarding the repositioning of the notice, what's your thought on the things to see when there is no notice?
I'm thinking it may look something like when there's no notice, the QR code and input fields are still kept close together, and once it appears, the input field is pushed down. If this is the case, an animation might be needed to help make this transition smoother.

jasmussen · 2023-03-30T07:25:30Z

Do you have any thoughts on what StevenDufresne brought up ^?

If the point is to combine two steps into one, I think that's fine. I'd suggest that also gives an opportunity to do a pass on the text, reduce when there's overlap. Less is always more when it comes to that. Doesn't have to be this PR, just noting.

Regarding the repositioning of the notice, what's your thought on the things to see when there is no notice?
I'm thinking it may look something like when there's no notice, the QR code and input fields are still kept close together, and once it appears, the input field is pushed down. If this is the case, an animation might be needed to help make this transition smoother.

It's fine that the notice pushes content around 👍 👍

But especially if two steps are combined into one, it'd be good to reduce elements (such as text) as much as possible, so we still mostly retain the same footprint and shape of the overall box. If you can try it and add a screenshot, I can give some suggestions on that if helpful.

renintw · 2023-04-13T09:30:09Z

I'm going to merge this first and #104 (New copy for TOTP), #105 (Notice reposition) were opened to address what we've discussed above.

renintw requested a review from iandunn March 28, 2023 19:09

renintw self-assigned this Mar 28, 2023

renintw added enhancement New feature or request ui Related to user interface labels Mar 28, 2023

renintw requested a review from StevenDufresne March 28, 2023 19:10

renintw force-pushed the enhance/split-2fa-intruction-into-two-steps branch from 6ee043a to e700642 Compare March 28, 2023 19:38

renintw commented Mar 28, 2023

View reviewed changes

StevenDufresne reviewed Mar 28, 2023

View reviewed changes

renintw added 11 commits April 13, 2023 16:38

Add totp intro

a778b2d

Fix screen height

ae7dfbd

Component: Add AutoTabbingInput

a8c8df4

Use AutoTabbingInput + Align totp styles w/ design

b82126a

Fix qr-code height to avoid jitter

1eae32d

Make class name consistent

c1a28dc

Align totp submit buttons styles with design

4e49064

Align manual setup styles with the design

49905f3

Fix is-error notice styles

4d2d2ef

Fix responsive styles

ca988da

Add error border and restore WP classic style

78797a6

renintw force-pushed the enhance/split-2fa-intruction-into-two-steps branch from d806f40 to 78797a6 Compare April 13, 2023 08:38

renintw mentioned this pull request Apr 13, 2023

New copy for TOTP #104

Closed

Let Cancel clear all the inputs

cba6a73

renintw force-pushed the enhance/split-2fa-intruction-into-two-steps branch from c4da34c to cba6a73 Compare April 13, 2023 09:33

renintw merged commit 35d524f into trunk Apr 13, 2023

renintw deleted the enhance/split-2fa-intruction-into-two-steps branch April 13, 2023 09:48

renintw mentioned this pull request Apr 18, 2023

Scan QR Code - Clearing input should remove error state. #106

Closed

iandunn mentioned this pull request May 5, 2023

Can't paste TOTP codes #151

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split TOTP instruction into two steps #100

Split TOTP instruction into two steps #100

renintw commented Mar 28, 2023 •

edited

Loading

renintw Mar 28, 2023

renintw Mar 28, 2023 •

edited

Loading

StevenDufresne Mar 28, 2023

renintw Mar 29, 2023

renintw Apr 13, 2023 •

edited

Loading

StevenDufresne commented Mar 28, 2023

jasmussen commented Mar 29, 2023

renintw commented Mar 29, 2023 •

edited

Loading

jasmussen commented Mar 30, 2023

renintw commented Apr 13, 2023

Split TOTP instruction into two steps #100

Split TOTP instruction into two steps #100

Conversation

renintw commented Mar 28, 2023 • edited Loading

Screencast

TODO

Update

renintw Mar 28, 2023

Choose a reason for hiding this comment

renintw Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

StevenDufresne Mar 28, 2023

Choose a reason for hiding this comment

renintw Mar 29, 2023

Choose a reason for hiding this comment

renintw Apr 13, 2023 • edited Loading

Choose a reason for hiding this comment

StevenDufresne commented Mar 28, 2023

jasmussen commented Mar 29, 2023

renintw commented Mar 29, 2023 • edited Loading

jasmussen commented Mar 30, 2023

renintw commented Apr 13, 2023

renintw commented Mar 28, 2023 •

edited

Loading

renintw Mar 28, 2023 •

edited

Loading

renintw Apr 13, 2023 •

edited

Loading

renintw commented Mar 29, 2023 •

edited

Loading