Skip to content

LG-14754: Avoid focus loss on submit button when submitting form#11482

Merged
aduth merged 6 commits intomainfrom
aduth-lg-14754-submit-disable
Nov 12, 2024
Merged

LG-14754: Avoid focus loss on submit button when submitting form#11482
aduth merged 6 commits intomainfrom
aduth-lg-14754-submit-disable

Conversation

@aduth
Copy link
Copy Markdown
Contributor

@aduth aduth commented Nov 8, 2024

🎫 Ticket

LG-14754

🛠 Summary of changes

Updates the behavior of submit buttons to avoid disabling the button using disabled attribute, instead using a combination of aria-disabled and event handlers to prevent duplicate submission when the button is already activated.

This fixes an issue where focus loss can occur if the page's submit button is the active element when submitting the form. While this is usually not a problem because it's expected that the user will be navigated away after submitting the form, this can cause issues in some screen readers which announce the loss of focus (e.g. JAWS announcing "Unavailable").

📜 Testing Plan

Verify that focus is maintained when submitting a form, and that you cannot submit twice:

SpinnerButtonComponent:

Apply diff to force a pause in submission and to validate duplicate submission handling, since form submission normally happens quickly:

diff --git a/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts b/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts
index 45f6e33afe..700789d12c 100644
--- a/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts
+++ b/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts
@@ -69,5 +69,6 @@ class CaptchaSubmitButtonElement extends HTMLElement {
     if (this.shouldInvokeChallenge()) {
       event.preventDefault();
-      this.invokeChallenge();
+      debugger;
+      // this.invokeChallenge();
     }
   };
  1. Go to http://localhost:3000
  2. Open browser developer tools
  3. Fill in email and password
  4. Press Tab to shift focus to "Sign in" button
  5. Press Enter
  6. Observe that breakpoint is triggered
  7. Dismiss breakpoint
  8. Observe that the focus highlight remains on the button while submitting
  9. Press Enter
  10. Observe that breakpoint is not triggered

SubmitButtonComponent:

Apply diff to force a pause in submission and to validate duplicate submission handling, since form submission normally happens quickly:

diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb
index a0dd702f04..db73956e9d 100644
--- a/app/controllers/sign_up/registrations_controller.rb
+++ b/app/controllers/sign_up/registrations_controller.rb
@@ -21,4 +21,5 @@ module SignUp
 
     def create
+      binding.pry
       @register_user_email_form = RegisterUserEmailForm.new(analytics:)
  1. Go to http://localhost:3000
  2. Click "Create an account"
  3. Fill in email address
  4. Check Rules of Use checkbox
  5. Press Tab to shift focus to "Submit" button
  6. Press Enter
  7. Observe that the focus highlight remains on the button while submitting
  8. Press Enter one or more times, and/or click the submit button
  9. Change to terminal process where server is running
  10. Observe that breakpoint is triggered
  11. Press Ctrl + D to dismiss breakpoint
  12. Observe that the breakpoint is not re-triggered, indicating that the server only received one form submission.
  13. Observe that you're redirected in the browser to the signup email confirmation page

@aduth aduth requested a review from a team November 8, 2024 15:12
}

handleLongWait() {
#handleLongWait = () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Taking more advantage of private class members, for clearer separate of public/private interface, and better bundle optimization (related Slack thread).


.usa-button:disabled.usa-button--active,
[aria-disabled='true'].usa-button--active {
.usa-button[aria-disabled='true'].usa-button--active {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without this change, the styles aren't strong enough to override default USWDS as expected, resulting in a grey button.

Before After
image image

@@ -1,3 +1,4 @@
import { mock } from 'node:test';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Using node:test mock here instead of sinon because I think it might be possible to eventually move toward dropping Sinon in the future, this making that future work simpler.

Continuing to use Sinon in spinner-button-element.spec.ts for consistency's sake, since it's already used there.

"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-testing-library": "^6.2.0",
"jsdom": "^22.1.0",
"jsdom": "^25.0.1",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Update to be able to use HTMLElement#ariaDisabled in specs.

jsdom/jsdom#3655

@aduth aduth marked this pull request as draft November 8, 2024 15:30
Rely on SubmitButtonElement to control whether submit event is emitted based on whether it's already being submitted
@aduth aduth marked this pull request as ready for review November 8, 2024 16:05
Copy link
Copy Markdown
Contributor

@kevinsmaster5 kevinsmaster5 left a comment

Choose a reason for hiding this comment

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

LGTM, tested locally.

@aduth aduth merged commit d6ff721 into main Nov 12, 2024
@aduth aduth deleted the aduth-lg-14754-submit-disable branch November 12, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants