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

Repository for design.va.gov website
https://design.va.gov
40 stars 61 forks source link

Update Sketch VA Pattern library to match developed components #675

Closed GnatalieH closed 2 years ago

GnatalieH commented 2 years ago

Steps to complete

  1. Refer to results of audit in #674.
  2. Update Sketch symbols to match developed components. Or, in cases where components should be altered, make tickets for fixes.

Progress Checklist

Form controls

caw310 commented 2 years ago

Here is the prioritization from Matt for updating the Sketch library.

P1: Every component and variation in the Design System (DS) is in the Sketch Library (SL) and no components are in the SL that are not in the DS P2: There are symbols for every state of each component P3: The way the SL is organized matches the DS

humancompanion-usds commented 2 years ago

We've done 12 and have 25 to go. So roughly 1/3rd of the way there after 2 sprints. A few of these components (header, footer, card) are not in design.va.gov yet but are on the way. A few of these just recently got promoted form experimental and thus don't exist in Sketch at all.

seanasewell commented 2 years ago

Completed the following...

The ones remaining need symbols.

GnatalieH commented 2 years ago

@seanasewell what is the left and right padding we're recommending around the text label in the Tag component? I was just wondering after looking at the example in Sketch, which shows the text layer stretched the width of the background with centered text. Storybook shows 7px left/right padding, but I think we should make it 8px to align with our spacing units. The line height on the developed component is also unusual: 22.5px, and the font size is 15px. Thoughts, @humancompanion-usds? Should we make Tag use more standardized specs?

Sean, I also noticed the developed Tag component has a 2px corner radius on all sides.

seanasewell commented 2 years ago

@GnatalieH The tag component was already made in Sketch. I assumed it was correct since it was already a part of the library. So I just moved it over to it's own page. Sorry for the oversight! But I agree with your recommendations. I'll make all of the obvious updates now besides the font (until we can narrow down specific specs for that).

Components I have created that need review:

Components I have started reviewing however we should probably discuss:

caw310 commented 2 years ago

As discussed in stand up, Matt will review the updates that have been made. We will close out as many as possible by the end of the sprint then close this ticket and write up new tickets for the remaining ones which are either more complex or lower priority to be done in future sprints.

caw310 commented 2 years ago

@humancompanion-usds please review

humancompanion-usds commented 2 years ago

@seanasewell - Looks good. Feedback: To my understanding, it is not currently possible to disable a checkbox (at least, that is what Storybook is telling me). There is a request from the a11y folks that we not make it possible. Thus we should not show a disabled state for checkboxes and radio buttons.

GnatalieH commented 2 years ago

CC @caw310 and @humancompanion-usds

seanasewell commented 2 years ago

@seanasewell - Looks good. Feedback: To my understanding, it is not currently possible to disable a checkbox (at least, that is what Storybook is telling me). There is a request from the a11y folks that we not make it possible. Thus we should not show a disabled state for checkboxes and radio buttons.

  • Archive disabled states for checkbox and radio buttons.
  • Margins:

    • Checkbox symbols have 14px below them. In fact they should have 12px above them and be aligned to the bottom edge of the artboard because that's how they are coded. Essentially, our labels have 12px of margin-top. This is so when you stack them they are appropriately spaced.
    • Radio buttons. Currently have 14px below (unselected variation) and should have 10px above.
    • Number input: 24px in Sketch but should have 30px margin-top (!? not sure why it's so much but, that's how it is coded)

Sorry, I didn't actually design these. Since they already had states, I figured they were already good to go and just moved them over to the page that designers reference. Oversight on my end! I'll take a look at all of these today to ensure they are parallel to what is in Storybook.

humancompanion-usds commented 2 years ago

I took care of a number of things yesterday evening and starred the release:

I will file an issue for the Conditional fields. Once Natalie finishes up filing issues we can close this.

seanasewell commented 2 years ago

I made updates to Checkbox, Radio Button, and Number Input. Ready for review @humancompanion-usds @GnatalieH

GnatalieH commented 2 years ago

I made a ticket for the Link - deep content component (it's listed as Copy deep link in this ticket). We can close this ticket when @humancompanion-usds has approved the changes made by @seanasewell. CC @caw310

humancompanion-usds commented 2 years ago

@seanasewell - Checkbox and radio button looked good. I fixed one little alignment issue with the error text and label to left align them. One little nit to fix up on Number Input: It looks like the border in the error state drops down below the input by 8px so the overall component is a bit taller in that error state. Otherwise these are good to go.

caw310 commented 2 years ago

Closing as we have addressed most issues or have written tickets for the remaining items that require more time and coordination.