Skip to content
This repository is currently being migrated. It's locked while the migration is in progress.

va-file-input/va-file-input-multiple: Add support for backwards compatibility of password change/submission#2027

Merged
RyanMunsch merged 30 commits intomainfrom
5851-file-input-pw-flag
Mar 19, 2026
Merged

va-file-input/va-file-input-multiple: Add support for backwards compatibility of password change/submission#2027
RyanMunsch merged 30 commits intomainfrom
5851-file-input-pw-flag

Conversation

@RyanMunsch
Copy link
Copy Markdown
Contributor

@RyanMunsch RyanMunsch commented Mar 11, 2026

Chromatic

https://5851-file-input-pw-flag--65a6e2ed2314f7b8f98609d8.chromatic.com

Summary

This PR update va-file-input and va-file-input-multiple to support the existing behavior for encrypted file passwords (event emission on change of nested va-text-input for password) and the new pattern that includes a submit button for the inputted password.

Description

This PR re-implements the updates to va-file-input and va-file-input-multiple that introduced the new password submit button pattern from PR #1997, while adding backward compatibility for the existing pattern that uses only a password text input and emits vaPasswordChange.

Backward compatibility is supported through a new disablePasswordSubmitButtonPattern prop (default: false) on both components. When this prop is true, the password section conditionally excludes the submit button from being rendered, emits vaPasswordChange on input of the va-text-input component, and uses updated event-handler logic.

This backward compatibility is needed because the Claims Status Tool team cannot currently adopt the submit-button pattern version of file input in their app without significant business-logic changes. Supporting both patterns allows the forms system to adopt the new submit-button pattern now, while giving the Claims Status Tool team time to prepare for eventual deprecation of the original pattern.

Related tickets and links

Closes department-of-veterans-affairs/vets-design-system-documentation#5851

Screenshots

If there are any visual changes, screenshots should be added here.

Using new default password pattern

Monosnap Components : File input USWDS - Accepts File Password With Submit Button ⋅ Storybook — Original profile 2026-03-11 17-14-41

Using disable password submit button pattern

Monosnap Components : File input USWDS - Accepts File Password ⋅ Storybook — Original profile 2026-03-11 17-14-07

Testing and review

Provide any testing instructions or review steps as needed.

Approvals

See the QA Checklists section below for suggested approvals. Use your best judgment if additional reviews are needed. When in doubt, request a review.

Approval groups

Add approval groups to the PR as needed:

QA checklists

Use the QA checklists below as guides, not rules. Not all checklists will apply to every PR but there could be some overlap.

In all scenarios, changes should be fully tested by the author and verified by the reviewer(s); functionality, responsiveness, etc.

✨ New Component Added
  • The PR has the minor label
  • The component matches the Figma designs.
  • All properties, custom events, and utility functions have e2e and/or unit tests
  • A new Storybook page has been added for the component
  • Tested in all VA breakpoints.
  • Chromatic UI Tests have run and snapshot changes have been accepted by the design reviewer
  • Tested in vets-website using Verdaccio
  • Engineering has approved the PR
  • Design has approved the PR
  • Accessibility has approved the PR
🌱 New Component Variation Added
  • The PR has the minor label
  • The variation matches its Figma design.
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • A new story has been added to the component's existing Storybook page
  • Any Chromatic UI snapshot changes have been accepted by a design reviewer
  • Tested in vets-website using Verdaccio
  • Engineering has approved the PR
  • Design has approved the PR
🐞 Component Fix
  • The PR has the patch label
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • Any markup changes are evaluated for impact on vets-website.
    • Will any vets-website tests fail from the change?
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Engineering has approved the PR
♿️ Component Fix - Accessibility
  • The PR has the patch label
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Engineering has approved the PR
  • Accessibility has approved the PR
🚨 Component Fix - Breaking API Change
  • The PR has the major label
  • vets-website and content-build have been evaluated to determine the impact of the breaking change
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Tested in vets-website using Verdaccio
  • Engineering has approved the PR
🔧 Component Update - Non-Breaking API Change
  • The PR has the minor label
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Engineering has approved the PR
📖 Storybook Update
  • The PR has the ignore-for-release label
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Engineering has approved the PR
🎨 CSS-Library Update
  • The PR has the css-library label
  • vets-website and content-build have been checked to determine the impact of any breaking changes
  • Engineering has approved the PR

@RyanMunsch RyanMunsch added the minor For a minor Semantic Version change label Mar 11, 2026
@RyanMunsch RyanMunsch marked this pull request as ready for review March 11, 2026 20:44
@RyanMunsch RyanMunsch requested a review from a team as a code owner March 11, 2026 20:44
@RyanMunsch RyanMunsch requested review from a team and amyleadem March 11, 2026 21:25
@danbrady
Copy link
Copy Markdown
Contributor

danbrady commented Mar 12, 2026

@RyanMunsch When I enter the first character of the password, the input switched to error mode. Just confirming if that's the intended behavior?

Screen.Recording.2026-03-12.at.1.01.30.PM.mov

Also, it looks like the top margin on the label gets lost when the component is in error state. Can we maintain it?

Screenshot 2026-03-12 at 1 04 13 PM

@danbrady
Copy link
Copy Markdown
Contributor

@RyanMunsch I'm not sure if this is an issue or validation would still catch at some point, but if I add a file, add a few characters to the password, and then change the file, the error state goes away even though I didn't meet the requirements (4 chars).

Screen.Recording.2026-03-12.at.1.17.29.PM.mov

@RyanMunsch
Copy link
Copy Markdown
Contributor Author

Thanks for taking a look, @danbrady.

When I enter the first character of the password, the input switched to error mode. Just confirming if that's the intended behavior?

Yes, this is expected for the minimum password requirement story and is in line with expected behavior for emitting on change (the functionality we wanted to get rid of with the submit button pattern). You can see that is the case on the production version here

Also, it looks like the top margin on the label gets lost when the component is in error state. Can we maintain it?

This exists on prod, but I just made a CSS update here to resolve that.

I'm not sure if this is an issue or validation would still catch at some point, but if I add a file, add a few characters to the password, and then change the file, the error state goes away even though I didn't meet the requirements (4 chars).

Good catch! This was a combination of a missing piece of validation in the story template component and logic in the component itself. I made an update to each, notably passing the value of this.passwordError to va-text-input.value. I did some testing on Storybook and ran unit tests and didn't see any regressions from this update.

Copy link
Copy Markdown
Contributor

@danbrady danbrady left a comment

Choose a reason for hiding this comment

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

LGTM!

Removes duplicate controls and allows component to reflect toggled values for `use-password-submit-button-pattern` prop
@jeana-adhoc jeana-adhoc self-requested a review March 17, 2026 16:21
Copy link
Copy Markdown
Contributor

@jeana-adhoc jeana-adhoc left a comment

Choose a reason for hiding this comment

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

I found a few things...

  1. For the stories that have a minimum password requirement, focus is returning to the component instead of going into the file password field when the file is successfully uploaded.

  2. Can we add the same alert in the 'default' (no submit button) password variant that is also in the submit button variant. We should have an equitable experience here.

  3. I did not catch this in the first round of review, so we can leave this as a known issue, but, we should add a unique accessible name to the multi-file upload password fields, and we should make the label consistent between the submit button/no submit button variations.

Image

I would make the visible label File password, and the aria-label="File password for $fileName" This ensures that if someone just uploads files and then goes back to add passwords they know which file the field is associated with.

@RyanMunsch RyanMunsch requested a review from jeana-adhoc March 17, 2026 20:58
@RyanMunsch RyanMunsch requested a review from jamigibbs March 19, 2026 13:36
* to the password input field for encrypted files. When false, only the
* password input field will be rendered for encrypted files.
*/
@Prop() usePasswordSubmitButtonPattern?: boolean = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion (not blocking): I generally prefer to always default a component prop to false (opt-in pattern) which would mean in this case changing the name of the prop to something like disablePasswordSubmitPattern and defaulting to false. But I think in this specific case it can be argued either way (there's some cognitive load for downstream consumers to reason about turning a "disable" prop to "true") so this is just a non-blocking suggestion.

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.

Yeah, I generally have the same preference. I was on the fence about changing it to accommodate a default false value, but ultimately decided to keep the original name without having a strong reason to other than Jeana had already written the ADR to describe the variants. Additionally, I would usually assume that setting a prop that renders a different variant of the component to true would indicate that the rendered output of that variant is not the "default" version, which is the opposite of the direction we want to go in with these changes (hopefully that makes sense here as it does in my head 😆).

I'll update the prop name/default now.

@RyanMunsch RyanMunsch requested a review from jamigibbs March 19, 2026 16:24
Comment thread packages/web-components/src/components/va-file-input-multiple/README.md Outdated
@RyanMunsch
Copy link
Copy Markdown
Contributor Author

Good catch on those missed updates, @jamigibbs. Thank you!

Copy link
Copy Markdown
Contributor

@jeana-adhoc jeana-adhoc left a comment

Choose a reason for hiding this comment

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

Thank you for all your work on this @RyanMunsch and the back and forth with me...

I've tested this on

  • Edge & Chrome with NVDA & JAWS
  • Safari & Chrome with VO on Mac
  • Safari with VO on IOS
  • Talkback on Android

In all cases the fields operated as exepected.

  • Focus went to the password field on file submission
  • We added the warning alert to both the default and the submit button version
  • File password label and file name were announced
  • In cases where there were a validation error on not a long enough password, we updated the error message to read Enter a password with at least 4 characters
  • We added an ADR for this work

Marking this as approved!

@RyanMunsch RyanMunsch merged commit f27fdc2 into main Mar 19, 2026
8 checks passed
@RyanMunsch RyanMunsch deleted the 5851-file-input-pw-flag branch March 19, 2026 19:53
@powellkerry powellkerry mentioned this pull request Mar 30, 2026
44 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor For a minor Semantic Version change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update va-file-input with password enhancement

5 participants