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

feat: define missing color property in some components #2754

Merged
merged 2 commits into from
May 14, 2024

Conversation

jawinn
Copy link
Collaborator

@jawinn jawinn commented May 13, 2024

Description

Sets the color property in parts of some components that were relying on inheriting a color from higher up in the DOM. These affected components were noticed in VRTs during the work to create a single entry point for PostCSS. The following has been updated:

  • datepicker: color added to the dash between dates in the "Range" variant. Adds a new property and mod.
  • splitview: default body color added for its contents. Adds a new mod.
  • treeview: section heading set to spectrum-heading-color token per design guidance. Adds a new property.
  • slider: sets the missing color on the tick labels (appearing under each tick). Adds a new property and mod.
  • radio: read-only variant now sets the color (same as label color) instead of inheriting.
  • well: default body color added for its contents. Adds a new mod.
  • progressbar: no changes to styles; only its storybook template was updated. The VRT difference only related to the storybook-only typography above each story.

The Jira issue and VRT also listed the icon component, but that one has been left as is. Its color is intentionally set to inherit by default.

CSS-731

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

  • No visual regressions in the components listed
  • Newly added mods work to change the noted items

Expected VRT differences:

  • Treeview section heading may show a slight VRT difference in color, as this used to default to the spectrum-body-color.
  • Progressbar Storybook headings typography will look a little different (no longer underlined).

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • VRTs have been run and looked at.
  • Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • I have tested these changes in Windows High Contrast mode.
  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

Copy link

changeset-bot bot commented May 13, 2024

🦋 Changeset detected

Latest commit: 7f9def6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@spectrum-css/datepicker Minor
@spectrum-css/splitview Minor
@spectrum-css/treeview Minor
@spectrum-css/slider Minor
@spectrum-css/radio Minor
@spectrum-css/well Minor
@spectrum-css/progressbar Patch
@spectrum-css/fieldgroup Major

Not sure what this means? Click here to learn what changesets are.

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

@jawinn jawinn force-pushed the jawinn/css-731-missing-color-definitions branch from 97e0784 to 10974f2 Compare May 13, 2024 19:49
Copy link
Contributor

github-actions bot commented May 13, 2024

🚀 Deployed on https://pr-2754--spectrum-css.netlify.app

Copy link
Contributor

github-actions bot commented May 13, 2024

File metrics

Summary

Total size: 4.48 MB*
Total change (Δ): ⬆ 3.42 KB (0.07%)

Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.

Package Size Δ
datepicker 14.77 KB ⬆ 0.26 KB
radio 17.95 KB ⬆ 0.13 KB
slider 32.95 KB ⬆ 0.22 KB
splitview 11.41 KB ⬆ 0.15 KB
treeview 17.20 KB ⬆ 0.10 KB
well 1.41 KB ⬆ 0.14 KB

Details

datepicker

File Head Base Δ
index-base.css 14.57 KB 14.31 KB ⬆ 0.26 KB (1.79%)
index-theme.css 0.80 KB 0.80 KB -
index-vars.css 14.77 KB 14.51 KB ⬆ 0.26 KB (1.76%)
index.css 14.77 KB 14.51 KB ⬆ 0.26 KB (1.76%)
metadata.json 8.09 KB 7.93 KB ⬆ 0.17 KB (2.06%)
themes/express.css 0.70 KB 0.70 KB -
themes/spectrum.css 0.69 KB 0.69 KB -

radio

File Head Base Δ
index-base.css 16.88 KB 16.75 KB ⬆ 0.13 KB (0.73%)
index-theme.css 1.65 KB 1.65 KB -
index-vars.css 17.95 KB 17.83 KB ⬆ 0.13 KB (0.68%)
index.css 17.95 KB 17.83 KB ⬆ 0.13 KB (0.68%)
metadata.json 9.26 KB 9.26 KB -
themes/express.css 1.33 KB 1.33 KB -
themes/spectrum.css 1.32 KB 1.32 KB -

slider

File Head Base Δ
index-base.css 30.61 KB 30.40 KB ⬆ 0.22 KB (0.69%)
index-theme.css 2.92 KB 2.92 KB -
index-vars.css 32.95 KB 32.74 KB ⬆ 0.22 KB (0.64%)
index.css 32.95 KB 32.74 KB ⬆ 0.22 KB (0.64%)
metadata.json 14.14 KB 14.06 KB ⬆ 0.08 KB (0.55%)
themes/express.css 1.76 KB 1.76 KB -
themes/spectrum.css 1.73 KB 1.73 KB -

splitview

File Head Base Δ
index-base.css 11.41 KB 11.26 KB ⬆ 0.15 KB (1.32%)
index-vars.css 11.41 KB 11.26 KB ⬆ 0.15 KB (1.32%)
index.css 11.41 KB 11.26 KB ⬆ 0.15 KB (1.32%)
metadata.json 4.69 KB 4.58 KB ⬆ 0.11 KB (2.30%)

treeview

File Head Base Δ
index-base.css 17.20 KB 17.10 KB ⬆ 0.10 KB (0.57%)
index-vars.css 17.20 KB 17.10 KB ⬆ 0.10 KB (0.57%)
index.css 17.20 KB 17.10 KB ⬆ 0.10 KB (0.57%)
metadata.json 8.42 KB 8.35 KB ⬆ 0.07 KB (0.85%)

well

File Head Base Δ
index-base.css 1.41 KB 1.28 KB ⬆ 0.14 KB (10.46%)
index-vars.css 1.41 KB 1.28 KB ⬆ 0.14 KB (10.46%)
index.css 1.41 KB 1.28 KB ⬆ 0.14 KB (10.46%)
metadata.json 0.69 KB 0.59 KB ⬆ 0.10 KB (16.64%)
* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

Use typography template in Meter storybook. Helps make sure that color
is set on this heading without coming from higher up in the DOM, and
completes code TODO item.
During the work to create a single entry point for PostCSS, it was
discovered that parts of several components did not define a color, and
instead relied on a color being set higher up in the DOM. This update
sets a color on some of these and adds mods where necessary.
@jawinn jawinn force-pushed the jawinn/css-731-missing-color-definitions branch from 10974f2 to 7f9def6 Compare May 13, 2024 20:59
Copy link
Collaborator

@castastrophe castastrophe left a comment

Choose a reason for hiding this comment

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

Outstanding addition! Love to see this update going in.

@castastrophe castastrophe added the run_vrt For use on PRs looking to kick off VRT label May 14, 2024
@castastrophe castastrophe merged commit dbf1406 into main May 14, 2024
22 checks passed
@castastrophe castastrophe deleted the jawinn/css-731-missing-color-definitions branch May 14, 2024 13:51
@github-actions github-actions bot mentioned this pull request May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run_vrt For use on PRs looking to kick off VRT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants