(#1839) Card font size change

karlikpj commented 1 month ago

Closes #1839

Pull Request Details

Add description

Closes #

Author PR Checklist

Items that the author of the PR is responsible for checking before submitted the PR.

General:

[x] I have reviewed the acceptance criteria defined in the ticket and ensured the work has been completed.
[x] The commit message passes all quality commit message standards.
[x] Unit tests have been updated or created to reflect any javascript changes.
[x] Storybook scenarios have been updated or created to reflect any html/css/js changes.

Accessibility:

[x] WCAG 2.1 Level AA requirements have been met.

Development:

[x] Any new or updated javascript code has 100% unit test coverage.
[x] New or updated breakpoints have regression images.
[x] Breaking changes have been thoroughly documented in the PR.

Product Reviewer PR Checklist

Items the product team is responsible for reviewing.

General:

[x] There are no unexpected or unapproved regression image changes. 
[x] Functionality of interactive elements meet the acceptance criteria.
[x] The product is visually and functionally the same across the different browsers.

Accessibility:

[x] AxeDev Tools: there are no new or outstanding accessibility issues introduced in this PR.
[x] Lighthouse: scores have not noticeably decreased during this PR.
[x] Wave: there are no new errors or contrast errors introduced in this PR.

Design Reviewer PR Checklist

Items the design team is responsible for reviewing.  

General:

[ ] New or updated features introduced in this PR are developed mobile-first.
[ ] Breakpoint changes and regression images match those breakpoints.
[ ] This PR has been tested in all supported browsers at all breakpoints.

Developer Reviewer PR Checklist

Items the development team is responsible for reviewing.

General:

[x] New code passes code quality standards set by industry standards.
[x] The expected Storybook stories have been added or updated for the new or updated feature.
[x] The expected unit tests have been added or updated for the new or updated feature.

Accessibility:

[x] VoiceOver: Described content matches with what was expected.
[x] Keyboard navigation: new or updated features and content are navigable via the keyboard.

github-actions[bot] commented 1 month ago

Viewing Information

bryanpizzillo commented 1 month ago

I would like us to discuss this a bit more and bring some consistency to these token usages. I.E. We should have a $theme-card-title and $theme-card-body or something tokens and set them accordingly. This change in this PR (as of this writing) has now made these items out of sync with Guide Cards. However, we have a ticket in the platform saying, "The body text shall have the same font size as all cards from the Design System," which is for a wide guide card. This type of requirement would do better by referencing the theme token to use. (Figma too?)

@andyvanavery31 and I talked about this. The Guide Card component and the Card component should be kept separate. However, we should still have component theme tokens for the font sizes for this card.

Note my naming in the initial comment sucks. Funny enough, there are some tokens already in USWDS for cards. Probably a lot more than we need. https://designsystem.digital.gov/documentation/settings/#card.

bryanpizzillo commented 1 month ago

One more followup, WRT the token naming. So this has an odd issue - we "replaced" usa-card with nci-card. While we don't pass through usa-card at this point, we could in the future. If we reuse or name our tokens for nci-card to be $theme-card we will break or conflict with usa-card. So for this instance we should use $theme-nci-card-* for nci-card settings. There should be a nice comment above these settings pointing out that we are naming this $theme-nci-card- to avoid conflicts with usa-card in the future.

@karlikpj suggested $theme-nci-card-title-typeset and $theme-nci-card-description-typeset for the token names, which look good.

andyvanavery31 commented 1 month ago

After the change to use $thme-nci-card-title-typeset and $theme-nci-card-description-typeset, this still passes Product Review.

NCIOCPL / ncids