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

va-file-input-multiple: update data for vaMultipleChange to return more detail (BREAKING) va-file-input: fix bug with upload message not resetting #1460

Merged
merged 5 commits into from
Jan 28, 2025

Conversation

powellkerry
Copy link
Contributor

@powellkerry powellkerry commented Jan 21, 2025

Chromatic

https://3597-file-in-mult-chng-evt--65a6e2ed2314f7b8f98609d8.chromatic.com

Description

BREAKING CHANGE

Update the vaMultipleChange event to return more data when the event is triggered.

Old Data Structure:

[{
    "changed": BOOLEAN,
    "file": FILE
}]

New Data Structure:

{
    "action": "FILE_REMOVED",
    "file": FILE,
    "state": [
        {
            "file": FILE,
            "changed": false
        }
    ]
}

Closes 3597

QA Checklist

  • [ x] Component maintains 1:1 parity with design mocks
  • [ x] Text is consistent with what's been provided in the mocks
  • [ x] Component behaves as expected across breakpoints
  • [ x] Accessibility expert has signed off on code changes (if applicable. If not applicable provide reason why)
  • [ x] Designer has signed off on changes (if applicable. If not applicable provide reason why)
  • [ x] Tab order and focus state work as expected
  • [ x] Changes have been tested against screen readers (if applicable. If not applicable provide reason why)
  • [ x] New components are covered by e2e tests; updates to existing components are covered by existing test suite
  • Changes have been tested in vets-website using Verdaccio (if applicable. If not applicable provide reason why)

Screenshots

No visual changes

Acceptance criteria

  • QA checklist has been completed
  • [ x] Screenshots have been attached that cover desktop and mobile screens

Definition of done

  • [ x] Documentation has been updated, if applicable
  • [ x] A link has been provided to the originating GitHub issue (or connected to it via ZenHub)

…re detail (BREAKING) va-file-input: fix bug with upload message not resetting
@powellkerry powellkerry added the major Major change in semantic versioning label Jan 21, 2025
@@ -30,6 +30,12 @@ export class VaFileInput {
private fileInputRef!: HTMLInputElement;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a bug that I discovered where if you remove all the files in the va-file-input-multiple field, the instruction text would not reset and the user would be left with a blank white clickable box.

@powellkerry powellkerry marked this pull request as ready for review January 21, 2025 16:44
@powellkerry powellkerry requested a review from a team as a code owner January 21, 2025 16:44
Copy link
Contributor

@jamigibbs jamigibbs left a comment

Choose a reason for hiding this comment

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

@powellkerry Since this is a major breaking change, how many instances of va-file-input do we think will be affected?

@micahchiang Any thoughts on doing a trial run for the proposed major release workflow with this PR?

@powellkerry
Copy link
Contributor Author

@jamigibbs Micah and I both did global searches and it looks like there are no usages for va-file-input-multiple in code yet. There was a task on the card to notify VFS teams, but since its not used we determined that was not necessary.

@powellkerry powellkerry merged commit f87eb1b into main Jan 28, 2025
8 checks passed
@powellkerry powellkerry deleted the 3597-file-in-mult-chng-evt branch January 28, 2025 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Major change in semantic versioning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update va-file-input-multiple's remove file emitted event
2 participants