Skip to content
This repository is currently being migrated. It's locked while the migration is in progress.

Font resize component fixes: Phase 2#1800

Open
amyleadem wants to merge 22 commits into
mainfrom
4881-font-resizing-fixes
Open

Font resize component fixes: Phase 2#1800
amyleadem wants to merge 22 commits into
mainfrom
4881-font-resizing-fixes

Conversation

@amyleadem
Copy link
Copy Markdown
Contributor

@amyleadem amyleadem commented Sep 29, 2025

Chromatic

https://4881-font-resizing-fixes--65a6e2ed2314f7b8f98609d8.chromatic.com

Description

Fixed font resizing issues in the following components:

  • va-additional-info (issue found in a text input variant)
  • va-date
  • va-memorable-date
  • va-search-input
  • va-telephone-input
  • va-text-input

For reference, these issues are documented in the component stories spreadsheet (Google docs 🔒)

Related tickets and links

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

Screenshots

va-additional-info (as found in va-text-input)

image

va-date

image image

va-memorable-date

image image

va-search-input

image image

va-telephone-input

va-text-input

image image

Testing and review

Approvals

See the QA Checklists section below for suggested approvals. Use your best judgment if additional reviews are needed. When in doubt, request a review.

Approval groups

Add approval groups to the PR as needed:

QA checklists

Use the QA checklists below as guides, not rules. Not all checklists will apply to every PR but there could be some overlap.

In all scenarios, changes should be fully tested by the author and verified by the reviewer(s); functionality, responsiveness, etc.

✨ New Component Added
  • The PR has the minor label
  • The component matches the Figma designs.
  • All properties, custom events, and utility functions have e2e and/or unit tests
  • A new Storybook page has been added for the component
  • Tested in all VA breakpoints.
  • Chromatic UI Tests have run and snapshot changes have been accepted by the design reviewer
  • Tested in vets-website using Verdaccio
  • Engineering has approved the PR
  • Design has approved the PR
  • Accessibility has approved the PR
🌱 New Component Variation Added
  • The PR has the minor label
  • The variation matches its Figma design.
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • A new story has been added to the component's existing Storybook page
  • Any Chromatic UI snapshot changes have been accepted by a design reviewer
  • Tested in vets-website using Verdaccio
  • Engineering has approved the PR
  • Design has approved the PR
🐞 Component Fix
  • The PR has the patch label
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • Any markup changes are evaluated for impact on vets-website.
    • Will any vets-website tests fail from the change?
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Engineering has approved the PR
♿️ Component Fix - Accessibility
  • The PR has the patch label
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Engineering has approved the PR
  • Accessibility has approved the PR
🚨 Component Fix - Breaking API Change
  • The PR has the major label
  • vets-website and content-build have been evaluated to determine the impact of the breaking change
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Tested in vets-website using Verdaccio
  • Engineering has approved the PR
🔧 Component Update - Non-Breaking API Change
  • The PR has the minor label
  • Any new properties, custom events, or utility functions have e2e and/or unit tests
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Engineering has approved the PR
📖 Storybook Update
  • The PR has the ignore-for-release label
  • Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
  • Engineering has approved the PR
🎨 CSS-Library Update
  • The PR has the css-library label
  • vets-website and content-build have been checked to determine the impact of any breaking changes
  • Engineering has approved the PR

@amyleadem amyleadem added the patch Patch change in semantic versioning label Sep 29, 2025
.select-month {
width: 8.125rem;
margin-right: 0.9375rem;
width: 16ch;
Copy link
Copy Markdown
Contributor Author

@amyleadem amyleadem Sep 30, 2025

Choose a reason for hiding this comment

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

Note

Replaced rem with ch here because the width is determined primarily by the number of characters expected/allowed. I did not use the current input width tokens because they rely on ex units, which is intended primarily for vertical height of the letters, rather than width.


.select-month {
width: 8.125rem;
margin-right: 0.9375rem;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note

Replaced margin-right here with a column-gap on .date-container. This simplified implementation and standardized it to more closely match the spacing in memorable date.

.usa-memorable-date {
margin-top: 16px;
// USWDS v3.7.1 set flex-wrap: wrap; This caused the year to always wrap when the month select option was active.
flex-wrap: nowrap;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note

We want to allow wrap here to accommodate zoom and text resizing.

@amyleadem amyleadem marked this pull request as ready for review September 30, 2025 19:43
@amyleadem amyleadem requested a review from a team as a code owner September 30, 2025 19:43
Comment on lines +491 to +517
<div>
<input
class="usa-input"
id="search-field"
name="search"
type="search"
ref={el => (this.inputRef = el as HTMLInputElement)}
aria-autocomplete={ariaAutoComplete}
aria-controls={ariaControls}
aria-expanded={ariaExpanded}
aria-haspopup={ariaHasPopup}
aria-label={label}
autocomplete="off"
onFocus={handleInputFocus}
onInput={handleInput}
onKeyDown={handleInputKeyDown}
role={role}
value={value}
/>
</button>
<button type="button" onClick={handleClearButtonClick} class={clearButtonClasses} aria-label="Clear the search contents">
<va-icon
class="usa-search__clear-icon"
icon="close"
size={3}
/>
</button>
</div>
Copy link
Copy Markdown
Contributor Author

@amyleadem amyleadem Sep 30, 2025

Choose a reason for hiding this comment

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

Note

Adding this wrapper div lets us position the clear icon relative to the nested div, rather than having to position it relative to the calculated widths of the search button. This new structure should make the styles more resilient, but a markup change has the potential to cause breaks, so curious to hear feedback.

Copy link
Copy Markdown
Contributor

@jamigibbs jamigibbs left a comment

Choose a reason for hiding this comment

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

I think dev looks good but I do have a question/concern about how the USWDS label style is being imported in va-date. I think we could do the same thing with the component module import pattern like the other USWDS-isified components. We'd just need to change the stylesheet to scss and add the CSS class. You are much more in the weeds here so I might not have the full context what what is needing to be done.

Comment thread packages/web-components/src/components/va-date/va-date.scss Outdated
@amyleadem amyleadem requested a review from jeana-adhoc October 1, 2025 23:38
Copy link
Copy Markdown
Contributor

@jamigibbs jamigibbs left a comment

Choose a reason for hiding this comment

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

It feels like a small miracle that there are no UI test changes to accept but dev looks good to me! Thank you! 🚀

@amyleadem amyleadem changed the base branch from main to 4878-font-resize-phase-1 October 2, 2025 23:08
@amyleadem amyleadem changed the base branch from 4878-font-resize-phase-1 to main October 2, 2025 23:08
@amyleadem amyleadem changed the base branch from main to 4878-font-resize-phase-1 October 2, 2025 23:11
@amyleadem amyleadem changed the base branch from 4878-font-resize-phase-1 to main October 2, 2025 23:15
@amyleadem
Copy link
Copy Markdown
Contributor Author

amyleadem commented Oct 2, 2025

I feel like it should be reporting changes in the UI tests :/
Va-date at least has a clear visual change in the default font size and display:
image

@jamigibbs Is it normal for Chromatic to miss them? Any tips to re-run the tests so they catch the differences?

@jamigibbs
Copy link
Copy Markdown
Contributor

jamigibbs commented Oct 3, 2025

@jamigibbs Is it normal for Chromatic to miss them? Any tips to re-run the tests so they catch the differences?

@amyleadem What has worked in the past is updating the onlyChanged setting in the workflow to false. That should force it to create a full snapshot run and pick up the changes. Before merging, just revert it back to true.

@amyleadem
Copy link
Copy Markdown
Contributor Author

Thanks @jamigibbs, that worked! Re-running Chromatic caught 30 changes (😮), but thankfully all of them are expected. If you could take a look and let me know if you have any concerns, that would be great.

Copy link
Copy Markdown
Contributor

@jeana-adhoc jeana-adhoc left a comment

Choose a reason for hiding this comment

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

I don't know how/why this has sat since October, but I just reviewed this, and it looks good.

I did not merge main down into this, but we ought to and hope nothing... breaks.

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.

Font resizing fixes: Component library form elements (Phase 2)

3 participants