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

Fixes double account creation when using keyboard controls #15077

Merged
merged 3 commits into from
Sep 23, 2022
Merged

Conversation

ryanml
Copy link
Contributor

@ryanml ryanml commented Jun 29, 2022

Fixes: #15061

Explanation

Defined in issue

Manual Testing Steps

Follow the steps mentioned in the issue, ensure that only a single account is created when using keyboard shortcuts

@ryanml ryanml requested a review from a team as a code owner June 29, 2022 04:45
@ryanml ryanml self-assigned this Jun 29, 2022
@ryanml ryanml requested a review from hmalik88 June 29, 2022 04:45
@metamaskbot
Copy link
Collaborator

Builds ready [61bed96]
Page Load Metrics (1682 ± 49 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint851644182336161
domContentLoaded1554194216688842
load15551942168210349
domInteractive1554194216688842

highlights:

storybook

Copy link
Contributor

@digiwand digiwand left a comment

Choose a reason for hiding this comment

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

👍🏼

Copy link
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

I might be crazy but pressing [ENTER] doesn't submit the form; we may need to event type check here

@digiwand
Copy link
Contributor

This fix is working on my end on Chrome. I was able to see the double accounts being created before the fix, and then a single account being created with this fix.

@darkwing what browser were you using? To confirm, was the target element the "Confirm" button after pressing tab to get to the button? I can see this could be easily mistaken since the focus state could be mistaken for the default state

@darkwing
Copy link
Contributor

Just tested on Chrome + OSX. If I click the "Create" button, the account gets created. If click "Create account" and immediately press ENTER, nothing happens.

@metamaskbot
Copy link
Collaborator

Builds ready [52aa16a]
Page Load Metrics (1653 ± 28 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint95147111136
domContentLoaded1561176616515828
load1561176616535828
domInteractive1561176616515828

highlights:

storybook

@jpuri
Copy link
Contributor

jpuri commented Aug 24, 2022

I might be crazy but pressing [ENTER] doesn't submit the form; we may need to event type check here

This does not looks related to code changes in PR. Click event is fired only in case of mouse click.

@digiwand
Copy link
Contributor

digiwand commented Sep 7, 2022

Echoing what @jpuri was mentioning, this PR tests navigating user flow with a keyboard and not a mouse.

@darkwing

Just tested on Chrome + OSX. If I click the "Create" button, the account gets created. If click "Create account" and immediately press ENTER, nothing happens.

This is existing behavior though would be nice to add form submit on error. After clicking "Create account", the input is in focus. This PR is testing when we hit enter when the "Create" button is in focus, and not the input in focus. You will need to tab twice to focus on the create button from the input, and then click enter to trigger the action

@ryanml
Copy link
Contributor Author

ryanml commented Sep 23, 2022

@darkwing, would you be fine with a separate PR for adding submit on 'Enter' behavior?

@metamaskbot
Copy link
Collaborator

Builds ready [6b31b2d]
Page Load Metrics (2697 ± 133 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1021891342211
domContentLoaded212532022659259124
load212532022697277133
domInteractive212532022659259124

highlights:

storybook

@ryanml ryanml requested a review from darkwing September 23, 2022 19:56
@ryanml ryanml merged commit e74614d into develop Sep 23, 2022
@ryanml ryanml deleted the fix-15061 branch September 23, 2022 20:03
@github-actions github-actions bot locked and limited conversation to collaborators Sep 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Create account using keyboard generates 2 accounts instead of 1
6 participants