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-checkbox: add normalized font size token to label #1513

Merged
merged 3 commits into from
Mar 7, 2025

Conversation

jamigibbs
Copy link
Contributor

@jamigibbs jamigibbs commented Mar 7, 2025

Chromatic

https://checkbox-label-font-size--65a6e2ed2314f7b8f98609d8.chromatic.com

Description

I noticed that the label font size was calculating to 16px but we want it to be the normalized font size (1.06rem/16.96px) as verified in Figma.

Closes - No ticket

Screenshots

before

Screenshot 2025-03-07 at 8 37 44 AM

after

Screenshot 2025-03-07 at 8 56 09 AM

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)

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

@jamigibbs jamigibbs added the patch Patch change in semantic versioning label Mar 7, 2025
--vads-input-suffix-color-text-on-light: #757575;
--vads-input-tile-background-active-on-light: rgba(0, 94, 162, 0.1);
--vads-input-tile-border-active-on-light: #005ea2;
--vads-input-tile-border-on-light: rgba(27, 27, 27, 0.03);
Copy link
Contributor Author

@jamigibbs jamigibbs Mar 7, 2025

Choose a reason for hiding this comment

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

All of these tokens are already being imported globally for the web-component files so just doing a little cleanup.

@import '~@department-of-veterans-affairs/css-library/dist/tokens/css/variables.css';

}

.va-checkbox__container--tile {
border-radius: 0.25rem;
background-color: var(--vads-input-background-color-on-light);
border: 2px solid var(--vads-input-tile-border-override);
border: 2px solid #c9c9c9;
Copy link
Contributor Author

@jamigibbs jamigibbs Mar 7, 2025

Choose a reason for hiding this comment

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

This was assigned a token value originally but that token name & color is not one that exists in the VA color palette. I'm reverting it back to the hex value because it's confusing to have a new token created and assigned just specifically in a single component.

If we want to create a semantically named token for this value #c9c9c9 which is gray-20 in the USWDS system, then we should create a follow-up ticket to do that in the CSS-Library.

But I'm not sure that we really need this to be a VA token. If the original accessibility issue gets fixed upstream, we will remove this style and import again from the USWDS checkbox package. So a new VA token for #c9c9c9 may never be needed outside of this specific use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jamigibbs jamigibbs marked this pull request as ready for review March 7, 2025 15:47
@jamigibbs jamigibbs requested a review from a team as a code owner March 7, 2025 15:47
@jamigibbs jamigibbs requested a review from a team March 7, 2025 15:48
Copy link
Contributor

@it-harrison it-harrison left a comment

Choose a reason for hiding this comment

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

looks good!

Copy link
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.

🚀

@jamigibbs jamigibbs merged commit a2c6214 into main Mar 7, 2025
8 checks passed
@jamigibbs jamigibbs deleted the checkbox-label-font-size branch March 7, 2025 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Patch change in semantic versioning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants