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

[Gutenboarding]: add autofocus to account creation email input field #39586

Merged
merged 1 commit into from
Feb 21, 2020

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Feb 21, 2020

Changes proposed in this Pull Request

This patch prevents the <Modal /> from focussing on itself, and instead forces focus on the email input field.

Feb-21-2020 13-16-25

❓What do folks think about the trade off in relation to accessibility? Would a better description in the label mitigate this?

Testing instructions

Trigger the signup modal in Gutenboarding.

Stare at the cursor as it blinks at you incessantly from within the email input field. Subtly mocking you. Daring you to blur it into oblivion.

…l input field.

This may affect accessibility, but would a better description in the label mitigate this?
@ramonjd ramonjd added [Type] Question [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Goal] New Onboarding previously called Gutenboarding labels Feb 21, 2020
@ramonjd ramonjd requested a review from a team February 21, 2020 02:18
@ramonjd ramonjd requested a review from a team as a code owner February 21, 2020 02:18
@ramonjd ramonjd self-assigned this Feb 21, 2020
@matticbot
Copy link
Contributor

@p-jackson
Copy link
Member

What do folks think about the trade off in relation to accessibility?

This is fine, in fact focusing on the text input is the right thing to do here according to WCAG.

Unless a condition where doing otherwise is advisable, focus is initially set on the first focusable element.
https://www.w3.org/TR/wai-aria-practices-1.1/#keyboard-interaction-7

Copy link
Member

@p-jackson p-jackson left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

The docs for the focusOnMount prop make it sound like it's supposed to do the right thing automatically.
https://developer.wordpress.org/block-editor/components/modal/#focusonmount

Problem is that the standard <Modal /> guff adds a <div role="dialog" tabIndex="0" />, which is treated as a focusable element. In which case the focusOnMount docs mean nothing; it's always just going to focus itself.

Anyway, I think the way you've done it is good. As the WCAG guidance points out, the correct element to focus is going to depend on the context, so it's going to be hard to get the <Modal> component to do the right thing by default all the time.

@ramonjd ramonjd merged commit 0cc43da into master Feb 21, 2020
@ramonjd ramonjd deleted the try/gutenboarding-autofocus-signup branch February 21, 2020 04:22
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] New Onboarding previously called Gutenboarding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants