va-file-input: new design variation #1142

nickjg231 · 2024-05-06T20:22:01Z

Chromatic

https://2642-file-input-update--65a6e2ed2314f7b8f98609d8.chromatic.com

Description

Updates the va-file-input V3 component with new designs and features. These include:

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

QA Checklist

Component maintains 1:1 parity with design mocks
Text is consistent with what's been provided in the mocks
Component behaves as expected across breakpoints
Accessibility expert has signed off on code changes (if applicable. If not applicable provide reason why)
Designer has signed off on changes (if applicable. If not applicable provide reason why)
Tab order and focus state work as expected
Changes have been tested against screen readers (if applicable. If not applicable provide reason why)
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

Base component:

File uploaded:

Error message:

Different size header:

Additional Info:

Acceptance criteria

QA checklist has been completed
Screenshots have been attached that cover desktop and mobile screens

Definition of done

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

…Update more styles

…instead of the base label

# Conflicts: # packages/web-components/package.json

jamigibbs

Thanks for your work on this Nick! Can you please add a link to the Figma designs in the PR description?

jamigibbs · 2024-05-08T18:30:17Z

packages/web-components/src/components/va-file-input/va-file-input.tsx

  @Element() el: HTMLElement;
+  @State() file?: File;


I know we had discussed keeping file management/state out of the component and having teams do that in their apps instead. But maybe there's a specific reason it's needed here. Do you mind providing some more context?

Edit: I think this is still necessary here, at least for constructing labels. Now, we are creating the labels for filename and file size instead of the USWDS script. Keeping a reference locally will let us do that a lot easier and more cleanly IMO.

packages/web-components/src/components/va-file-input/va-file-input.tsx

jamigibbs · 2024-05-08T18:36:10Z

packages/web-components/src/components/va-file-input/va-file-input.tsx

@@ -114,6 +127,29 @@ export class VaFileInput {
    this.el.shadowRoot.getElementById('fileInputField').click();
  };

+  private removeFile = () => {
+    this.file = undefined;


Would it be better to assign this as null instead of undefined?

packages/web-components/src/components/va-file-input/va-file-input.tsx

jamigibbs · 2024-05-08T18:38:48Z

packages/web-components/src/components/va-file-input/va-file-input.tsx

            {error && (
              <Fragment>
-                <span class="usa-sr-only">{i18next.t('error')}</span>
-                <span class="usa-error-message">{error}</span>
+                <span class="sr-only">{i18next.t('error')}</span>


I think that we can still use the USWDS module for usa-sr-only. That is not specific to file input.

Hi @nickjg231 Please import the USWDS module for usa-sr-only. The sr-only class will be going away with Formation.

Here is an example:
https://github.com/department-of-veterans-affairs/component-library/blob/main/packages/web-components/src/components/va-checkbox/va-checkbox.scss#L3

jamigibbs · 2024-05-08T18:39:23Z

packages/web-components/src/components/va-file-input/va-file-input.tsx

-                <span class="usa-sr-only">{i18next.t('error')}</span>
-                <span class="usa-error-message">{error}</span>
+                <span class="sr-only">{i18next.t('error')}</span>
+                <span>{error}</span>


usa-error-message also is not specific to file input. Do the VA designs diverge from the USWDS error styles? If not, can we continue to use the module that contains that class?

I am using the USWDS styles for hint and error again

LWWright7 · 2024-05-08T18:40:53Z

@nickjg231 - just a few minor things..... there is a black border line missing around the blue background that appears in the design. The only other thing I'm seeing is the difference in the icon. For the design, I'm not sure where @danbrady got that icon, as I'm not seeing it in our icon library in Figma under Font Awesome or USWDS icons, so it may be a custom .svg icon.

jamigibbs · 2024-05-08T18:47:04Z

Are there examples in Storybook to preview for the upload indicator or additional info slot like shown in the ticket's screenshots? I wasn't sure how to reproduce that to test it.

nickjg231 · 2024-05-08T19:51:31Z

Are there examples in Storybook to preview for the upload indicator or additional info slot like shown in the ticket's screenshots? I wasn't sure how to reproduce that to test it.

In the engineering channel the other day, we came to the consensus that we actually should not use the progress bar in this component. The upload in our component doesn't upload when the user selects a file, rather when the user submits the form.

When the user selects a file on the page, the input acts as a pointer to the user's file system and has a little bit of metadata. If we wanted to show the progress bar here, it wouldn't really be in an uploading state long enough to even see the progress bar. Then the user generally moves on in the form and the actual file upload happens at the end. This may mean they don't even see the progress bar upload. In addition, within the va-file-input, we have no knowledge of the form we're in or its events, like if it has been submitted. We would need to add extra props to manage the state, then work with the backend to actually have some sort of value returned from the server. At this point, it seems like it won't behave in a way that's intuitive to the user and would be confusing to handle. We can have a further discussion in the engineers channel if you'd like!

jamigibbs · 2024-05-08T19:53:13Z

A couple other UX things:

This icon doesn't look correct compared to Figma:

The padding in the header and hint text is not matching Figma

Figma

Storybook

I think this text color for the file size might be too dark?

Figma

Storybook

jamigibbs · 2024-05-08T19:55:36Z

In the engineering channel the other day, we came to the consensus that we actually should not use the progress bar in this component.

@nickjg231 Do we need to update the designs then? Or is it possible then for teams to add the progress component into the va-file-input slot in order to match the designs?

nickjg231 · 2024-05-08T19:59:23Z

A couple other UX things:

This icon doesn't look correct compared to Figma:

The padding in the header and hint text is not matching Figma

Figma

Storybook

I think this text color for the file size might be too dark?

Figma

Storybook

@jamigibbs

I had Lucas look into the icon. He says it's a custom file that Dan used on Figma, and isn't something that exists in our project. The one I chose is the closest icon we have to the one in the mockups, and Lucas seems okay with that.
This padding above the va-card is being updated now
This color difference was a typo, this is fixed now

nickjg231 · 2024-05-08T20:24:36Z

In the engineering channel the other day, we came to the consensus that we actually should not use the progress bar in this component.

@nickjg231 Do we need to update the designs then? Or is it possible then for teams to add the progress component into the va-file-input slot in order to match the designs?

@jamigibbs This may require some more discussion with the team, as I didn't think about allowing users to do this with the slot. The only thing I worry is that teams may use slightly different styles across forms so the progress bar could be slightly inconsistent, but otherwise it would take the burden off of us.

For now, I think this should be merged without references to the progress bar in the code, then we can discuss with designers. @LWWright7 what do you think?

jamigibbs · 2024-05-08T20:25:25Z

I had Lucas look into the icon. He says it's a custom file that Dan used on Figma, and isn't something that exists in our project. The one I chose is the closest icon we have to the one in the mockups, and Lucas seems okay with that.

@LWWright7 Should we create a follow-up ticket to update the icon then? cc @danbrady

micahchiang

@nickjg231 @jamigibbs - I haven't finished my review yet, but there are couple things out there that I think need to be looked at before merging, so I'm requesting changes until those things are resolved. I'll follow up with a longer comment here in a second.

danbrady · 2024-05-08T20:54:59Z

After a file is attached, the "icon" is actually a preview of that file (like a thumbnail). This functionality exists already in the USWDS File Input as well as the current v3 version that Andrew worked on. I'm not sure of the exact logic behind it, but my understanding is if the file can have a preview (because it is an image of some type) it will. However, it shows alternate preview images if the attachment is a different "non-previewable" file type.

micahchiang · 2024-05-08T20:55:34Z

@nickjg231 @jamigibbs - I haven't finished my review yet, but I'd like to chime in on some of the thoughts already out there.

Regarding the loading indicator

In the engineering channel the other day, we came to the consensus that we actually should not use the progress bar in this component. The upload in our component doesn't upload when the user selects a file, rather when the user submits the form.

Maybe this was part of the discussion after I had to leave the mob session on Monday, so I'm not sure if I'm missing some additional context here. The loading indicator is definitely part of the file input designs in Figma. It's also functionality that is current on staging. The following screen cap is from the 10-10EZ Form:

Screencast.from.05-08-2024.01.36.28.PM.webm

Because it already exists like this in staging, regardless of how brief it renders, we'll want to add it here.

Regarding the file icon

I'm fine with @LWWright7's sign off here since he's a designer, but I'd like to make sure things are consistent between what's in here and what exists in Figma. So, if we're going with the icon shown in the screenshots here, then I'd appreciate it if the designs in Figma were updated to avoid confusion. If it turns out for some reason that the custom icon @danbrady used was intentional, then we need to add it to va-icon and update the icon used in this component. I'm fine with whatever.

micahchiang

Requesting changes for the following reasons:

we should use our design tokens where we can. I left comments where they can be used.
we should be supporting loading indicator since it's part of the Figma designs unless @danbrady and @LWWright7 say otherwise.
I think we're meant to be displaying preview thumbnails instead of static icons
we need to decide how to truncate long file names. i.e. if a file name is longer than 25 characters should we display a truncated version of it?

packages/web-components/src/components/va-file-input/va-file-input.scss

micahchiang · 2024-05-08T23:00:18Z

packages/web-components/src/components/va-file-input/va-file-input.tsx

+  private removeFile = () => {
+    this.file = null;
+    this.uploadStatus = 'idle';
+  }
+
+  private changeFile = () => {
+    if (this.fileInputRef) {
+      this.fileInputRef.click();
+    }
+  }
+
+  private handleDrop = (event: DragEvent) => {
+    event.preventDefault();
+    event.stopPropagation();
+
+    const files = event.dataTransfer.files;
+    if (files.length > 0) {
+      this.file = files[0];
+      this.vaChange.emit({ files: [this.file] });
+      this.uploadStatus = 'success';
+    }
+  }
+


I'm not sure if our web component has always been this way so forgive me if I'm missing something, but with file upload, I'd expect to see files stored in a FileList on the input. For some reason it's returning an empty list after upload:

packages/web-components/src/components/va-file-input/va-file-input.tsx

micahchiang · 2024-05-08T23:11:09Z

packages/web-components/src/components/va-file-input/va-file-input.tsx

+   * @returns {string} The file size formatted as a string with the appropriate unit.
+   */
+  private formatFileSize = (filesSize): string => {
+    const units = ['B', 'KB', 'MB', 'GB', 'TB'];


I really hope no one tries to upload a file that is x GBs or TBs large lol

Indeed, that would be quite an edge case! However, it might be wise to consider implementing a limit on file size or handling such scenarios gracefully to prevent potential issues. What are your thoughts?

micahchiang · 2024-05-08T23:19:28Z

packages/web-components/src/components/va-file-input/va-file-input.tsx

+                <va-card class="va-card">
+                  <div class="file-info-section">
+                    <va-icon icon="file_present" size={5} class="file-icon"></va-icon>
+                    <span class="file-label">{file.name}</span>


@danbrady @LWWright7 - We need to decide on how to approach name truncation for files with long names. Would appreciate some design input here, and potentially an update to the figma specs to reflect name truncation.

Currently long names extend past the card:

Ok, I think we can handle long file names by splitting the file name and the uploading label / file size to be on different lines. In other words, the file name will be on top line and then either the "Uploading..." label or the file weight on the next line. If the file name is long, it should wrap, and not truncate it, so it does not break outside the boundaries of the card. (This stays inline with what USWDS does.) Some designs below for reference:

Previous design:

(This didn't factor long file names)

Updated design:

Updated design with long name and in uploading state:

Updated design with long name attached:

For Reference:
USWDS with Long File Name:

NOTE: The default body text has a line-height that's too much for this component. So instead I used line-height-2 (1.15) for the file name and size text, which renders 16.96px text to have 19.504 line-height.

I've updated the design files with this. Hit my up with any questions!

This is great, thanks @danbrady!

…ame and file size as 2 separate lines

…le info

… show as default icon for other file types

… from shifting when switching files

# Conflicts: # packages/web-components/package.json # packages/web-components/src/components/va-file-input/va-file-input.scss

# Conflicts: # packages/web-components/package.json

micahchiang

Changes look good. Thanks for your work on this @nickjg231 and @danbrady

nickjg231 added 7 commits April 29, 2024 16:47

WIP: Convert USWDS va-file-input to new design specifications

a1e50f7

Give buttons proper functionality. Format and display the file size. …

8da79d5

…Update more styles

Add headerSize prop and logic/styles to allow user to use header tag …

831fdb6

…instead of the base label

Add the additional info functionality

6f4fbc3

Add drag and drop functionality

512866b

Fix styling issue with active border

f2ca3ff

Remove upload percentage and multiple files props

2795387

nickjg231 added the minor For a minor Semantic Version change label May 6, 2024

nickjg231 self-assigned this May 6, 2024

nickjg231 added 3 commits May 7, 2024 15:28

Update unit tests

439214a

Bump minor version in package.json

6a73ca6

Merge remote-tracking branch 'origin/main' into 2642-file-input-update

63071ca

# Conflicts: # packages/web-components/package.json

nickjg231 marked this pull request as ready for review May 8, 2024 16:08

nickjg231 requested a review from a team as a code owner May 8, 2024 16:08

jamigibbs reviewed May 8, 2024

View reviewed changes

LWWright7 approved these changes May 8, 2024

View reviewed changes

nickjg231 and others added 2 commits May 8, 2024 16:08

Updates from code review and remove unnecessary code

17cbfd7

Merge branch 'main' into 2642-file-input-update

b173625

Change variable to local variable instead of state variable

0fc02f0

micahchiang requested changes May 8, 2024

View reviewed changes

harshil1793 and others added 13 commits May 10, 2024 11:47

using color tokens and clean up

2cebeb5

Optimized the condition for rendering element

6b8f473

add trucating method

e7a23a3

non-brealing space between numeral and unit in file size; sets file n…

24f70eb

…ame and file size as 2 separate lines

copies vads-u-line-height utility value and adjusts line-height of fi…

e5b4fe9

…le info

Render images and PDF previews in va-file-input. Add file_icon svg to…

fb97892

… show as default icon for other file types

Use svg element instead of va-icon to show new icon. Prevent contents…

8d1c38a

… from shifting when switching files

Merge remote-tracking branch 'origin/main' into 2642-file-input-update

17dbd2e

# Conflicts: # packages/web-components/package.json # packages/web-components/src/components/va-file-input/va-file-input.scss

Revert newlines in svg files

0d68ded

Merge remote-tracking branch 'origin/main' into 2642-file-input-update

5bf2821

# Conflicts: # packages/web-components/package.json

Reduce font size for base font size change

d3759c1

Align buttons on small mobile screen

6fe5961

Merge remote-tracking branch 'origin/main' into 2642-file-input-update

02d797f

# Conflicts: # packages/web-components/package.json

micahchiang approved these changes May 23, 2024

View reviewed changes

nickjg231 merged commit 3c2269a into main May 23, 2024
9 checks passed

nickjg231 deleted the 2642-file-input-update branch May 23, 2024 15:18

ataker approved these changes May 23, 2024

View reviewed changes

jamigibbs changed the title ~~2642 file input update~~ va-file-input: new design variation May 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

va-file-input: new design variation #1142

va-file-input: new design variation #1142

nickjg231 commented May 6, 2024 •

edited

jamigibbs left a comment

jamigibbs May 8, 2024

nickjg231 May 8, 2024 •

edited

jamigibbs May 8, 2024

jamigibbs May 8, 2024

jamigibbs May 8, 2024

jamigibbs May 8, 2024

nickjg231 May 8, 2024

LWWright7 commented May 8, 2024

jamigibbs commented May 8, 2024

nickjg231 commented May 8, 2024

jamigibbs commented May 8, 2024

jamigibbs commented May 8, 2024

nickjg231 commented May 8, 2024

nickjg231 commented May 8, 2024

jamigibbs commented May 8, 2024

micahchiang left a comment

danbrady commented May 8, 2024 •

edited

micahchiang commented May 8, 2024

micahchiang left a comment

micahchiang May 8, 2024

micahchiang May 8, 2024

harshil1793 May 9, 2024

micahchiang May 8, 2024

danbrady May 13, 2024

micahchiang May 15, 2024

micahchiang left a comment

va-file-input: new design variation #1142

va-file-input: new design variation #1142

Conversation

nickjg231 commented May 6, 2024 • edited

Chromatic

Description

QA Checklist

Screenshots

Acceptance criteria

Definition of done

jamigibbs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickjg231 May 8, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LWWright7 commented May 8, 2024

jamigibbs commented May 8, 2024

nickjg231 commented May 8, 2024

jamigibbs commented May 8, 2024

jamigibbs commented May 8, 2024

nickjg231 commented May 8, 2024

nickjg231 commented May 8, 2024

jamigibbs commented May 8, 2024

micahchiang left a comment

Choose a reason for hiding this comment

danbrady commented May 8, 2024 • edited

micahchiang commented May 8, 2024

Regarding the loading indicator

Regarding the file icon

micahchiang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Previous design:

Updated design:

Updated design with long name and in uploading state:

Updated design with long name attached:

Choose a reason for hiding this comment

micahchiang left a comment

Choose a reason for hiding this comment

nickjg231 commented May 6, 2024 •

edited

nickjg231 May 8, 2024 •

edited

danbrady commented May 8, 2024 •

edited