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

Me: 2fa Reauth Required Improvements #1386

Merged
merged 3 commits into from
Dec 8, 2015
Merged

Conversation

ebinnion
Copy link
Contributor

@ebinnion ebinnion commented Dec 8, 2015

Fixes #401
Fixes #400

This PR allows auto focusing of the 2fa code input when the reauth required dialog opens in /me.

reauth-autofocus

cc @v18 and @lezama for review

To test:

  • Checkout fix/me-reauth-autofocus
  • Make sure to test with an account that has 2fa enabled
  • Go to WordPress.com
  • Delete twostep_auth cookie
  • Go to /me in Calypso
  • In the dialog that pops up, the input should focus automagically

@ebinnion ebinnion added [Type] Enhancement [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] User & Account Settings (/me) Settings and tools for managing your WordPress.com user account. labels Dec 8, 2015
@ebinnion ebinnion self-assigned this Dec 8, 2015
@rickybanister
Copy link

While you're in there, what about addressing #400

  • using mid-points instead of 123456 for the placeholder
  • disabling the submit button if the field is empty

@ebinnion
Copy link
Contributor Author

ebinnion commented Dec 8, 2015

I just pushed up a commit that addresses disabling the "Verify" button when a code is not entered. I'm not sure what we're going to do with the placeholder text just yet, so I'd prefer to handle that in another PR.

reauth-verify-focus

@ebinnion ebinnion changed the title Me: Auto focus on reauth code input Me: Auto focus on reauth code input and disable button unless code entered Dec 8, 2015
@ebinnion ebinnion changed the title Me: Auto focus on reauth code input and disable button unless code entered Me: 2fa Reauth Required Improvements Dec 8, 2015
@ebinnion
Copy link
Contributor Author

ebinnion commented Dec 8, 2015

After some clarification in Slack, I went ahead and updated the placeholder text to be e.g. 123456:

screen shot

}

return (
<Dialog
autoFocus={ false }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this prop necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my testing it is necessary.

My understanding is the the Dialog component auto focuses on the content by default. So passing in autoFocus={false} short circuits that and allows us to set our own autoFocus.

See this block.

@rickybanister
Copy link

Looks good to me design-wise.

@lezama
Copy link
Contributor

lezama commented Dec 8, 2015

Works as expected 🚢

@lezama lezama added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 8, 2015
ebinnion added a commit that referenced this pull request Dec 8, 2015
@ebinnion ebinnion merged commit 888c217 into master Dec 8, 2015
@ebinnion ebinnion deleted the fix/me-reauth-autofocus branch December 8, 2015 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] User & Account Settings (/me) Settings and tools for managing your WordPress.com user account. [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Me: focus on two-step response field when authenticating access Me: 2FA: Improve the 2FA popup dialog
4 participants