department-of-veterans-affairs / vets-design-system-documentation

Repository for design.va.gov website
https://design.va.gov
36 stars 55 forks source link

Figma Component Rebuild: Form field atomic elements #2126

Closed babsdenney closed 8 months ago

babsdenney commented 8 months ago

Description

We have to rebuild our components in Figma since they do not transition well from Sketch. This is the list of components we will need to rebuild. The goal with rebuilding the components is to make them as reactive as possible using auto layout and properties.

Components to create in this ticket:

Look at the instance in storybook and also in Sketch to make sure all states are able to be shown in the Figma component.

Details

Compare the different options for importing the component to the VADS component library. Below are some different sources that might help you rebuild the component in the VADS component library. You will need developer help to import using story.to.design Plugin

Ideally, it would be nice to think of all the form fields as atom elements that can be used for multiple form fields. Think of the label, error fields, and hint texts as something that will be used in multiple places. How can we set up these different form fields to be resilient to future updates and changes?

Tasks

Acceptance Criteria

The design is the same as what is in the Component Symbol Library in Sketch

danbrady commented 8 months ago

@babsdenney I updated the components for this ticket based on our discussion.

babsdenney commented 8 months ago

@danbrady I might be missing it but I don't see any changes in the ticket.

babsdenney commented 8 months ago

@danbrady My bad, I see it now. Thank you for editing the ticket.

danbrady commented 8 months ago

@babsdenney @LWWright7 Please review:

Text Input: https://www.figma.com/file/afurtw4iqQe6y4gXfNfkkk/VADS-Component-Library?type=design&node-id=199%3A1223&mode=design&t=0LcTbuI4mKp91wpH-1

Textarea: https://www.figma.com/file/afurtw4iqQe6y4gXfNfkkk/VADS-Component-Library?type=design&node-id=199%3A1222&mode=design&t=0LcTbuI4mKp91wpH-1

Select: https://www.figma.com/file/afurtw4iqQe6y4gXfNfkkk/VADS-Component-Library?type=design&node-id=293%3A4969&mode=design&t=0LcTbuI4mKp91wpH-1

Also, please try the components in the "Example Library" as a subscriber.

babsdenney commented 8 months ago

@danbrady This is looking really great! It's so much easier to do all the variations in Figma compared to Sketch. Thank you for working on all these different components.

For the text input:

  1. Could you make the label a max-width like the hint text?
  2. For the "Show format hint" there is also an error state that is shown when the character count goes over the limit.
  3. It might also be better to label that "Character count hint text" so Designers know it's specific to Character count options only.

Everything else looks super awesome! This is amazing though, it handles changes so much better than Sketch. It's very hard to get all these different properties to work well in Sketch, you have done such a great job. 100% improvement from Sketch.

babsdenney commented 8 months ago

@danbrady For the textarea has the same label max-width issue as text-input. Other than that, looks amazing. I appreciate the addition of the expandable textarea icon in the bottom right corner. Nice touch!

caw310 commented 8 months ago

This may carry over to get in edits.

babsdenney commented 8 months ago

@danbrady For select -

  1. One thing I noticed is when you toggle the "Required" property on and off the line height changes very subtly. Can you see if we can make the required option keep the same space between the input and the label?
  2. In storybook it looks like the max-height is 480px but it shrinks to the screen size of 320px on smaller screens. It's interesting that the text input and the select don't behave the same way in that text-input has standard sizes and select doesn't.

Other than that, this is amazing! Having an open state with editable options is such a level up. Thank you for doing all this.

danbrady commented 8 months ago

Thanks for looking and the feedback, @babsdenney! I'll incorporate your feedback and ask for a re-review.

danbrady commented 8 months ago

Thanks again for looking at this, @babsdenney! My comments below:

For the text input:

  1. Could you make the label a max-width like the hint text?

This is a bit challenging because the toggleable "required" tag affects how the wrapping of the label works. Let's put our heads together on how to tackle this, because this could affect a few components.

  1. For the "Show format hint" there is also an error state that is shown when the character count goes over the limit.

That format hint "subcomponent" should have it's own State property for when there's a character count error. It won't change if the parent text input is in error mode because the error might not be related to the character count. Does that work? Here's what the State property should look like:

image

  1. It might also be better to label that "Character count hint text" so Designers know it's specific to Character count options only.

Ah, good point! Will update.

@danbrady For the textarea has the same label max-width issue as text-input. Other than that, looks amazing. I appreciate the addition of the expandable textarea icon in the bottom right corner. Nice touch!

Yeah! Figma has a way to simulate absolute positioning. That's how that icon always gets positioned in the bottom right, even if the textarea size changes. Regarding the max-width issue, it's the same as (1) in the text input comments above.

For select -

  1. One thing I noticed is when you toggle the "Required" property on and off the line height changes very subtly. Can you see if we can make the required option keep the same space between the input and the label?

Great catch! Will fix.

  1. In storybook it looks like the max-height is 480px but it shrinks to the screen size of 320px on smaller screens. It's interesting that the text input and the select don't behave the same way in that text-input has standard sizes and select doesn't.

Yeah, I noticed that too. Maybe it's because native HTML selects will resize to the biggest option and that was preferred (up to 480px width)? Perhaps we'll have sizing options eventually, but for I matched the web component. I did update it though so you should be able to resize this component, but it has a max-width of 480px.

I republished the library. Please re-test. Thanks!

babsdenney commented 8 months ago

@danbrady Thanks for fixing all this! I didn't know if that max-width to minimum width for the select would be possible. I don't think you can do that in Sketch. That looks awesome!

I should have checked the text input and the text area, they also have that issue with the required making the input shift down a bit. Could you apply the same fix from select to the text input and the text area?

Also, this is super nitpicky so my apologies. I was wondering why there's a 2px space around the select and text area box? I didn't see that in code and just wanted to make sure that was necessary in figma. image

danbrady commented 8 months ago

I should have checked the text input and the text area, they also have that issue with the required making the input shift down a bit. Could you apply the same fix from select to the text input and the text area?

Will do!

Also, this is super nitpicky so my apologies. I was wondering why there's a 2px space around the select and text area box? I didn't see that in code and just wanted to make sure that was necessary in figma. image

Not nitpicky at all and another great catch! That is an issue with how the focus state in Figma is set up. First, let's go on a side quest...

In HTML we can simply use and outline-offset to size and position the focus ring. Unfortunately, we can't do that in Figma. A workaround is to use two borders: one, a 2px white border that matches the background color and adds the needed offset/buffer space around the input. Then two, the second border, gets place "under" the first border. It is a 4px yellow one. Now because the 2px white border is on top, it gives the illusion that a 2px yellow/focus ring is offset.

Ok, back to main quest... The reason why there is that whitespace inside the component is because that first, white border was set with an "inside" stroke and not "outside". Silly mistake on my part. I updated the components and hope I covered all the issues. Please re-test the three components. Thanks!

babsdenney commented 8 months ago

@danbrady Thank you for explaining all this. Sketch had a similar issue trying to get focus to work and you had to kind of rig it to do a shadow and stroke options. It was sort of hacky. This looks so great! Thank you for looking at all this in such detail and solving the text-wrapping issue. A huge huge improvement from Sketch and so well built!

danbrady commented 8 months ago

Thanks @babsdenney!

Tagging @humancompanion-usds for PO review. FYI: I dropped in different variations into the Example Library pages to validate the variations, but feel free to bring in new instances to test.

humancompanion-usds commented 8 months ago

Barb's QA is very thorough so I just glanced at these. This spacing seems a little tight to me:

Private Zenhub Image

But I know that the spacing units jump from 8 to 12px so perhaps I'll just live with that.

Another minor thing: In the examples I'd like to retain at least the name of the component at the top of the artboard. I see it on Radio buttons but not the others. This is great progress!

humancompanion-usds commented 8 months ago

Just a note on one of the comments: The USWDS select menu is set to 100% of whatever container it is within. That's typically .usa-form which is set to a max-width of 20rem (320px). Our forms tend to be 8 columns wide, so wider than 320px but they still max out at a reasonable size.

While we could introduce sizing, USWDS would probably only want smaller sizes because of .usa-form and because it's odd if the select menu in the closed state is narrower than the open state (in fact, I wonder if the browser will let you do that). At any rate, we can ask DanW and co. if the just haven't gotten to selects for sizing or if they don't want to support select sizes.

danbrady commented 8 months ago

Thanks for looking, @humancompanion-usds.

You're right about the spacing between the additional info and the input. (It does look extra tight with the focus ring on the input.) FWIW, Sketch has 16px of spacing but the v3 component currently uses 8px (which is what we followed):

Margin between additional info and text input

Regarding the page names, we were following what is published on the doc site and not what is in Sketch. So "Select" over "Select Box" and "Textarea" over "Text area". Let us know if that's incorrect.

(BTW, a lot of pages don't have titles applied yet. Those are pages we haven't touched yet. If there's a component on it, its just the imported versions that needs to be touched for resizing, tokens, text wrapping, etc. They haven't been tested either. In other words, if the page in the nav doesn't have a blue icon or green icon, then consider it not ready :)

danbrady commented 8 months ago

Just a note on one of the comments: The USWDS select menu is set to 100% of whatever container it is within. That's typically .usa-form which is set to a max-width of 20rem (320px). Our forms tend to be 8 columns wide, so wider than 320px but they still max out at a reasonable size.

While we could introduce sizing, USWDS would probably only want smaller sizes because of .usa-form and because it's odd if the select menu in the closed state is narrower than the open state (in fact, I wonder if the browser will let you do that). At any rate, we can ask DanW and co. if the just haven't gotten to selects for sizing or if they don't want to support select sizes.

Good to know. Figma does support max-width on components. So where possible we did introduce max-widths to components that match what is developed in the v3 component itself. For example, the select will max out at 480px because that's how it is currently developed. We can, of course, remove that but it would allow a designer to potentially making it bigger than what is currently supported.

I think you can size select with styles to any size regardless of the option lengths. However, you can't size the default browser/operating system popup menu of options. I validated this with a quick demo.

Closed:

Select sized to 200px

Open:

Select with options open bigger than 200px
humancompanion-usds commented 8 months ago

Regarding the page names, we were following what is published on the doc site and not what is in Sketch. So "Select" over "Select Box" and "Textarea" over "Text area". Let us know if that's incorrect.

That's the correct thing to do.

(BTW, a lot of pages don't have titles applied yet.... if the page in the nav doesn't have a blue icon or green icon, then consider it not ready :)

The pages I reviewed had blue icons but no page title: Select, Textarea, Text Input, and Additional Info. They should have a title.

the select will max out at 480px because that's how it is currently developed.

Great! As long as we are reflecting what is coded we are good.