Skip to content

chore(meter): add vrt coverage - #6576

Open
Rajdeepc wants to merge 2 commits into
mainfrom
rajdeepchandra/chore-add-meter-vrt-coverage
Open

chore(meter): add vrt coverage#6576
Rajdeepc wants to merge 2 commits into
mainfrom
rajdeepchandra/chore-add-meter-vrt-coverage

Conversation

@Rajdeepc

@Rajdeepc Rajdeepc commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Description

Adds dedicated Storybook VRT coverage for swc-meter, following the foundation established in the button/action-button VRT work:

  • 2nd-gen/packages/swc/components/meter/test/vrt/meter.vrt.tsPermutations and ForcedColors stories covering every size, every variant, both label positions, value states (0/25/50/75/100%, plus an over-100% case with a clamped bar and an overridden value-label), full anatomy (label-only, label + description, accessible-label-only fallback, custom value text), behaviors (custom range, format options), text wrapping, CJK language rendering, and static colors on their contrast backgrounds.
  • 2nd-gen/packages/swc/components/meter/test/vrt/meter-custom-properties.vrt.ts — one reference/override row per each of the 6 documented --swc-linear-progress-* custom properties, with coverage verified against the generated custom-elements manifest.

No meter-global-styles.vrt.ts was added: unlike button, there is no generated global stylesheet for meter/linear-progress to cover. Meter is also non-focusable and read-only, so unlike button/action-button/accordion there are no :hover/:focus-visible/:active rules to force in a VRT snapshot.

Motivation and context

Meter currently has no dedicated visual regression coverage. Chromatic VRT catches rendering regressions (color, layout, forced-colors/high-contrast) that unit tests can't, and this closes that gap for meter the same way #6463 and #6569 did for button and action-button.

Related issue(s)

  • SWC-2411

Screenshots (if appropriate)

N/A — new Chromatic snapshots will be generated on this PR's first VRT run for review.

Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes.
  • I have included a well-written changeset if my change needs to be published.
  • I have included updated documentation if my change required it.

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

  • Meter VRT stories render as expected

    1. Run Storybook (yarn storybook) in 2nd-gen/packages/swc
    2. Go to Meter → Meter VRT → Permutations, Forced colors, and Custom properties
    3. Confirm each row renders the labeled size/variant/state/anatomy/behavior/language/static-color case correctly, the forced-colors story shows ButtonFace/ButtonText track and fill overrides, and each custom-property override cell visibly differs from its reference cell
  • Chromatic snapshots are accurate

    1. Run the project VRT command scoped to meter
    2. Review the generated snapshots for each option/state/variant combination
    3. Confirm no unintended diffs against existing shipped meter visuals

Adds dedicated Storybook VRT stories for swc-meter, following the
button/action-button VRT foundation: size, variant, label-position,
value-state (including over-100% and anatomy/label variations),
behaviors, wrapping, CJK, and static-color permutations plus a
forced-colors snapshot in meter.vrt.ts, and one reference/override row
per public --swc-linear-progress-* custom property in
meter-custom-properties.vrt.ts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Rajdeepc
Rajdeepc requested a review from a team as a code owner August 4, 2026 12:21
@changeset-bot

changeset-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ba1f188

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

📚 Branch Preview Links

🔍 Gen1 Visual Regression Test Results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

Deployed to Azure Blob Storage: pr-6576

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

@Rajdeepc Rajdeepc self-assigned this Aug 4, 2026
@Rajdeepc Rajdeepc added Status:Ready for review PR ready for review or re-review. run_vrt Triggers the Chromatic VRT run for 2nd-gen labels Aug 4, 2026

@cdransf cdransf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great! ✨

[
renderMeter({
label:
'A label long enough to wrap onto multiple lines within the available inline space',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be nice if we could show the wrapping. There's plenty of text here, but the Meter's width is wide enough to accommodate all of the text without actually wrapping when the snapshot is taken.

Image

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.

Fixed in ba1f188 — constrained the meter to max-inline-size: 200px for this case so the label actually wraps onto multiple lines in the snapshot now.

Comment on lines +264 to +269
// `forced-colors` replaces the whole page palette, so it can't be scoped to
// a subtree the way theme()'s light/dark split is, and needs its own
// snapshot rather than folding into Permutations. Confirms the
// `@media (forced-colors: active)` override in linear-progress-base.css
// (ButtonFace track, ButtonText fill) applies regardless of variant or
// static-color.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think this comment is adding much value in its current state. How would you feel about removing it?

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.

Removed in ba1f188.

Comment on lines +254 to +255
// Rendered once in light/ltr and once in dark/rtl (that combination covers
// both axes), all still in a single story so it costs one snapshot.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't know that this comment is adding much value in its current state. How would you feel about removing it?

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.

Removed in ba1f188.

@pfulton pfulton left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I left a few comments here, @Rajdeepc. I'll leave it to your judgement as to whether you keep or get rid of the AI-generated comments that I left feedback on. For the wrapping label example, if you're able to get that one to actually wrap in the snapshot, that would be valuable.

@miwha-adobe

Copy link
Copy Markdown
Contributor

Can we adjust the color from magenta, to something easier to view? The contrast fails:https://webaim.org/resources/contrastchecker/

Screenshot 2026-08-05 at 1 33 09 PM Screenshot 2026-08-05 at 1 33 53 PM

@miwha-adobe
miwha-adobe self-requested a review August 5, 2026 19:35

@miwha-adobe miwha-adobe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please see comment about color contrast above

- Constrain the Wrapping case's width so the label actually wraps in
  the snapshot instead of fitting on one line (pfulton).
- Drop two low-value comments above Permutations/ForcedColors (pfulton).
- Swap the --swc-linear-progress-text-color override from magenta to
  darkmagenta: magenta text fails WCAG contrast on the default light
  background and was flagged by axe (miwha-adobe).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Rajdeepc

Rajdeepc commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@miwha-adobe Thanks for catching that! Fixed in ba1f188 — swapped `--swc-linear-progress-text-color`'s override from `magenta` to `darkmagenta` (~7.5:1 contrast on the default light background vs. magenta's ~2.3:1), so it now passes WCAG AA while still rendering obviously different from the default. Left `fill-color`/`track-color` as `magenta` since those aren't text colors and aren't subject to the same contrast check.

@Rajdeepc
Rajdeepc requested review from miwha-adobe and pfulton August 7, 2026 11:47

const MOD_PROPERTY_CASES: readonly MeterPropertyCase[] = [
{ property: '--swc-linear-progress-fill-color', value: 'magenta' },
{ property: '--swc-linear-progress-track-color', value: 'magenta' },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if we go, deep-plum instead for all three of these? I checked out deep-magenta, and it still fails for the fill (though the text is great and has the 7:5:1 like you said against white).

Also I don't know that the comment is necessarily needed and or can be condensed. I feel like claude tends to over explain in it's comments.

Image Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run_vrt Triggers the Chromatic VRT run for 2nd-gen Status:Ready for review PR ready for review or re-review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants