Skip to content

Silently ignore invalid params for FormSteps #10005

Merged
aduth merged 2 commits intomainfrom
aduth-form-steps-invalid-step-param
Feb 1, 2024
Merged

Silently ignore invalid params for FormSteps #10005
aduth merged 2 commits intomainfrom
aduth-form-steps-invalid-step-param

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jan 30, 2024

🛠 Summary of changes

Updates handling for how FormSteps considers step names from URL parameters, to ensure that URL changes which result in an invalid step (e.g. the skipnav navigation) would not have an effect on what's shown to the user, while avoiding bypassing skipnav URL change altogether.

This is an alternative to #7769. The primary goal of these changes is to avoid potential conflicts with default behavior of U.S. Web Design System by keeping the component implementations closer in sync. This also has a secondary effect of reducing the size of the common application JavaScript file that is loaded on every screen of the application.

There's a related USWDS issue at uswds/uswds#5133 which may end up being closed as "working as intended", based on recent findings shared at uswds/uswds#5133 (comment).

📜 Testing Plan

Repeat Testing Plan from #7769

Specifically verify:

  • Back and forward buttons behave as expected when traversing "Steps" in document capture, particularly within in-person proofing
  • Using the "Skip to main content" link after tabbing from the URL address bar will now update the URL, but it will not affect what's shown on the screen
  1. Go to http://localhost:3000
  2. Sign in
  3. Go to http://localhost:3000/verify
  4. Complete identity verification up to document capture step
  5. Use an error file as front and back image to trigger a failure for document capture
  6. Click Submit
  7. On failure, click "Try in person"
  8. On "Verify your identity in person", click "Continue"
  9. Press the back button
  10. Observe that you are returned to "Verify your identity in person"
  11. Focus the browser URL address bar
  12. Press Tab
  13. Observe that the "Skip to main content" link is shown
  14. Press Enter
  15. Observe:
    a. Focus shifts to main content
    b. The URL hash fragment changes to #main-content
    c. You are still on the "Verify your identity in person" screen (this is the bug that LG-8682 aimed to fix)

aduth added 2 commits January 30, 2024 15:58
changelog: Internal, Document Capture, Refactor handling for invalid URL steps
@aduth aduth requested review from a team, JackRyan1989 and eileen-nava and removed request for a team January 30, 2024 21:06
@JackRyan1989
Copy link
Contributor

@aduth - so the motivation here is to bring the current skip to main content implementation closer to USWDS' because they you're not convinced there's a bug here, or because USWDS is planning on releasing changes in the near future?

I noticed that on the #prepare and #location pages in document capture, that backward navigation doesn't immediately direct you to the previous page, but rather steps backwards through the previous urls for the same page because of the hashes at the end of the url. I guess this is expected behavior from the browsers point of view, but I wonder if a user wouldn't be confused?

@aduth
Copy link
Contributor Author

aduth commented Jan 31, 2024

I noticed that on the #prepare and #location pages in document capture, that backward navigation doesn't immediately direct you to the previous page, but rather steps backwards through the previous urls for the same page because of the hashes at the end of the url.

I'm not entirely sure I follow the exact behavior you're describing, but could you clarify if what you're seeing is any different from what you see on main?

@JackRyan1989
Copy link
Contributor

Yeah so it does work differently on main. Using the browser to navigate backwards requires extra clicks since the hash is updated by the skip to main button.
Behavior on Main branch:

  1. Navigate to http://localhost:3000/verify/document_capture#review
  2. Select "Try in Person"
  3. URL is now "http://localhost:3000/verify/document_capture#prepare"
  4. Select URL bar
  5. Tab to Skip to Main Content & press enter
  6. URL is still "http://localhost:3000/verify/document_capture#prepare"
  7. Navigate backwards using browser
  8. URL is now http://localhost:3000/verify/document_capture

Behavior on this branch:

  1. Navigate to http://localhost:3000/verify/document_capture#review
  2. Select "Try in Person"
  3. URL is now "http://localhost:3000/verify/document_capture#prepare"
  4. Select URL bar
  5. Tab to Skip to Main Content & press enter
  6. URL is now "http://localhost:3000/verify/document_capture#main-content"
  7. Navigate backwards using browser
  8. URL is now "http://localhost:3000/verify/document_capture#prepare"
  9. Navigate backwards using browser
  10. URL is now http://localhost:3000/verify/document_capture

@aduth
Copy link
Contributor Author

aduth commented Jan 31, 2024

Ah, I see what you mean now. I don't know how frequently this would happen in the real world (nor even the original bug in the first place), but if I were pressed, I'd probably argue that the revised behavior is a slight improvement to what should be expected, since in-page links are expected to create a new history entry (same as if you go to a page like https://login.gov/about-us/, click "Our mission" in the sidebar, then click "Back" in your browser).

It would also work this way for any other USWDS site that uses skip navigate (for example, go to https://designsystem.digital.gov/, tab to "Skip to main content", press Enter, press browser back button), and I would generally expect this to be the more common implementation of skip links such that anyone using them would come to expect it.

@aduth
Copy link
Contributor Author

aduth commented Jan 31, 2024

@aduth - so the motivation here is to bring the current skip to main content implementation closer to USWDS' because they you're not convinced there's a bug here, or because USWDS is planning on releasing changes in the near future?

Yes, I think the current default USWDS behavior is correct, and our workaround shouldn't bypass skip link behavior, and instead make our custom URL logic more interoperable (i.e. at FormSteps, as proposed here). I'm not aware of any specific changes coming to USWDS, but I think this would help future-proof in case any were to come down the road.

Copy link
Contributor

@JackRyan1989 JackRyan1989 left a comment

Choose a reason for hiding this comment

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

LGTM!

@eileen-nava
Copy link
Contributor

eileen-nava commented Jan 31, 2024

since in-page links are expected to create a new history entry (same as if you go to a page like https://login.gov/about-us/, click "Our mission" in the sidebar, then click "Back" in your browser).

It would also work this way for any other USWDS site that uses skip navigate (for example, go to https://designsystem.digital.gov/, tab to "Skip to main content", press Enter, press browser back button), and I would generally expect this to be the more common implementation of skip links such that anyone using them would come to expect it.

This was a really helpful explanation - thanks for clarifying! I will review the changes now.

Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

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

Approved. 👍🏻

@aduth aduth merged commit eb7aa03 into main Feb 1, 2024
@aduth aduth deleted the aduth-form-steps-invalid-step-param branch February 1, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants