feat(rating): S2 migration by cdransf · Pull Request #3627 · adobe/spectrum-css

cdransf · 2025-03-17T18:23:57Z

Description

CSS-1141

This migrates the rating component to S2. Custom properties have been remapped and added per the design specification.

Additions

The medium component variation is the default and a t-shirt size medium variation has been added.

A tooltip may be displayed to a user indicating whether interacting with the component will clear or edit the rating. A control and documentation example have been added.

A rating may have a partially filled star. The width and fill of this star is controlled by adding .is-partial to the parent span with classes of spectrum-Rating-icon and is-selected and then setting --mod-rating-icon-fill to the necessary fill percentage (e.g. 50%).

New mods

--mod-rating-width
--mod-rating-height
--mod-rating-bottom-to-content-area
--mod-rating-edge-to-content-area
--mod-rating-top-to-content-area
--mod-rating-top-to-content-area
--mod-rating-icon-fill

Removed mods

--mod-rating-icon-spacing-vertical-top
--mod-rating-icon-count
--mod-rating-indicator-border-radius
--mod-rating-indicator-height

New custom properties

--spectrum-rating-width
--spectrum-component-size-difference-down
--spectrum-component-size-minimum-perspective-down
--spectrum-corner-radius-medium-size-medium
--spectrum-corner-radius-medium-size-small
--spectrum-neutral-content-color-default
--spectrum-neutral-content-color-down
--spectrum-neutral-content-color-hover
--spectrum-spacing-75
--spectrum-workflow-icon-size-100
--spectrum-workflow-icon-size-75

Removed custom properties

--spectrum-rating-indicator-width
--spectrum-rating-icon-color-key-focus
--spectrum-rating-icon-spacing-vertical-top
--spectrum-rating-focus-indicator-gap
--spectrum-rating-indicator-height
--spectrum-rating-indicator-border-radius
--spectrum-accent-content-color-key-focus
--spectrum-border-width-200
--spectrum-component-top-to-workflow-icon-100
--spectrum-corner-radius-100
--spectrum-neutral-subdued-content-color-default
--spectrum-neutral-subdued-content-color-down
--spectrum-neutral-subdued-content-color-hover
--spectrum-neutral-subdued-content-color-key-focus
--spectrum-workflow-icon-size-100

Validation steps

Open the Storybook URL for the PR.
Navigate to the rating component.
Confirm that the new sizing stories render in the docs view.
Verify that all new tokens have been adopted and properly implemented.

Small rating size is the new default, medium is available with a new t-shirt size class.
isFocused has been changed to isKeyboardFocused
Sizing story control behaves as expected
No regressions are visible

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. ✨

changeset-bot · 2025-03-17T19:39:58Z

🦋 Changeset detected

Latest commit: 9b25e3a

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

This PR includes changesets to release 1 package

Name	Type
@spectrum-css/rating	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

github-actions · 2025-03-20T15:06:19Z

File metrics

Summary

Total size: 1.38 MB^*
Total change (Δ): 🟢 ⬇ 0.25 KB (-0.02%)

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

Package	Size	Minified	Gzipped	Δ
rating	8.75 KB	8.25 KB	1.62 KB	🔴 ⬆ 0.22 KB

File change details

rating

Filename	Head	Minified	Gzipped	Compared to base
index.css	8.75 KB	8.25 KB	1.62 KB	🔴 ⬆ 0.22 KB
metadata.json	5.40 KB	-	-	🔴 ⬆ 0.42 KB

tokens

Filename	Head	Minified	Gzipped	Compared to base
css/dark-vars.css	46.52 KB	-	-	-
css/global-vars.css	72.90 KB	-	-	-
css/index.css	245.15 KB	-	-	🟢 ⬇ 0.13 KB
css/large-vars.css	40.75 KB	-	-	🟢 ⬇ 0.06 KB
css/light-vars.css	46.51 KB	-	-	-
css/medium-vars.css	40.87 KB	-	-	🟢 ⬇ 0.06 KB
json/tokens.json	388.25 KB	-	-	-

* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

github-actions · 2025-03-20T15:06:43Z

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

castastrophe

Left a few suggestions for optimizing the tests and storybook. Otherwise approved but I do think we should push back on design designating a default size other than medium if medium exists. It's really confusing for developers and it's not the norm in our system right now.

rise-erpelding

Thanks for the documentation additions here, particularly for Tooltip and Partial Star! The rating with tooltip instance in the testing grid also looks great!

I left a few thoughts on showing :hover in the testing grid, plus I still think the focus state needs a little bit of work, and we need to make sure our changeset is freshly updated.

rise-erpelding · 2025-04-16T22:28:30Z

+	&.is-focused {
+		padding-block: calc(var(--mod-rating-top-to-content-area, var(--spectrum-rating-top-to-content-area)) + var(--mod-rating-focus-indicator-gap, var(--spectrum-rating-focus-indicator-gap))) calc(var(--mod-rating-bottom-to-content-area, var(--spectrum-rating-bottom-to-content-area)) + var(--mod-rating-focus-indicator-gap, var(--spectrum-rating-focus-indicator-gap)));
+		padding-inline: calc(var(--mod-rating-edge-to-content-area, var(--spectrum-rating-edge-to-content-area)) + var(--mod-rating-focus-indicator-gap, var(--spectrum-rating-focus-indicator-gap)));
+		box-shadow: 0 0 0 var(--mod-rating-focus-indicator-thickness, var(--spectrum-rating-focus-indicator-thickness)) var(--mod-rating-focus-indicator-color, var(--spectrum-rating-focus-indicator-color));


I'm still seeing the stars hopping around here, here's a different view to try to explain what I think might be going on, initially .spectrum-Rating has padding on it like this, 6px on each side:

Then, when the focus state is applied, it has padding like this, 8px on each side, plus the inner dimensions of .spectrum-Rating have changed from above:

So I think that explains why, if I apply the focus state, I'll see the stars move slightly to the right, then back to the left if the focus state is removed.

I do also see the stars change position slightly if the focus state is applied and I hover over each of the individual stars. I'm not totally sure if that's related to this issue or if it's a separate issue though.

rise-erpelding · 2025-04-16T22:45:44Z

+	&:hover .spectrum-Rating-icon,
+	&.is-hovered .spectrum-Rating-icon {
 		/* Make all stars selected when the component is hovered */
 		color: var(--highcontrast-rating-emphasized-icon-color-default, var(--mod-rating-emphasized-icon-color-default, var(--spectrum-rating-emphasized-icon-color-default)));

+		&.is-hovered,


This is something I definitely want to ask and clarify about with our team as a whole because I ran into this same issue with :active on a different PR review, but I believe that the PostCSS pseudo-classes plugin takes care of this, you can see if you yarn build and look at the dist/index.css for Rating, it adds the companion classes automatically so we don't have to, in this case, the postcss.config.js has it set to prefix states with "is-", so you should see it adding .is-hover anywhere where :hover is used.

(Now, for me, the question is, why do we see this so often in CSS files if PostCSS takes care of it for us? But I think in this case we'll be ok reverting this back to what it was before.)

With that, we can also change the setup in template.js to set that .is-hover class based on the arg, and it should work pretty much the same as what you have already, but will leverage PostCSS. I'll stick a comment down there also!

Oh awesome! Yay postcss! I reverted those classes and update the template and the testing grid looks good/in line with what I'd expect. ✨

Oh yes, so PostCSS adds the is-prefix to states only in the Storybook version of the assets. We could ship it but in the past I think we discussed it and decided not to.

Co-authored-by: rise-erpelding <54716846+rise-erpelding@users.noreply.github.com>

…elog

rise-erpelding

Leaving a quick note on the focus state and the emphasized hovered story!

rise-erpelding · 2025-04-18T15:15:55Z

+		padding-block-start: calc(var(--mod-rating-top-to-content-area, var(--spectrum-rating-top-to-content-area)) + var(--mod-rating-focus-indicator-gap, var(--spectrum-rating-focus-indicator-gap)));
+		padding-block-end: calc(var(--mod-rating-bottom-to-content-area, var(--spectrum-rating-bottom-to-content-area)) + var(--mod-rating-focus-indicator-gap, var(--spectrum-rating-focus-indicator-gap)));
+		padding-inline: var(--mod-rating-edge-to-content-area, var(--spectrum-rating-edge-to-content-area));


This does keep our stars from jumping around, but effectively now the padding-inline doesn't change between focused and not focused, and we're adding extra padding to account for that focus indicator gap only on the top and bottom and missing it on the sides.

Typically the components here don't use padding to create the focus indicator gap and I'm wondering if that's part of the reason why, if you search the repo for focus-indicator-gap a lot of the components will set a focus indicator with the ::after pseudo-element and the gap is a margin on that ::after.

It's probably not the only way though, you could definitely explore an approach using outline and outline-offset, it looks like that's what switch does!

Just pushed up 9b25e3a to address this using outline + outline-offset like we discussed. ✨

This reverts commit 2175ea7.

rise-erpelding

⭐ ⭐ ⭐ ⭐ ⭐

* fix(rating): active state star icon * chore(rating): adopt spacing tokens * chore(rating): remove underline when hovering over active rating stars * chore(rating): update color custom properties * chore(rating): size control * chore(rating): size stories * chore(rating): draft changeset * chore(rating): rating component hover logic * chore(rating): update changelog * chore(rating): add tooltip example, control and tests * chore(rating): refactor isFocused to isKeyboardFocused * fix(rating): default to medium size; remove unnecessary tests and template * fix(rating): update changeset heading depth * fix(rating): remove unused key-focus color styles * chore(rating): separate styles from custom properties; implement spacing tokens * chore(rating): application of rating height tokens * chore(rating): fix focus state + sizing * chore(rating): apply focus indicator gap * chore(rating): apply S2 down state * chore(rating): update changelog * fix(rating): corrects padding issues, flex alignment and removes unused property * chore(rating): add configurable filled star mask with mod to control fill percentage; update docs * fix(rating): remove unnecessary focus states * fix(rating): docs, changelog, testing and style updates * chore(rating): add emphasized + hovered story and test * chore(rating): remove duplicate mod * chore(rating): add removed mod * fix(rating): typo in class name Co-authored-by: rise-erpelding <54716846+rise-erpelding@users.noreply.github.com> * chore(rating): remove incorrect mods and custom properties from changelog * fix(rating): remove emphasized state from hover test * fix(rating): adjust block and inline padding for focus states * fix(rating): remove manual hover classes, update class names in template * fix(rating): change EmphasizedHovered story name to Hovered * Revert "fix(rating): adjust block and inline padding for focus states" This reverts commit 2175ea7. * fix(rating): replace padding with outline and outline-offset --------- Co-authored-by: rise-erpelding <54716846+rise-erpelding@users.noreply.github.com>

cdransf added the skip_vrt Add to a PR to skip running VRT (but still pass the action) label Mar 17, 2025

cdransf self-assigned this Mar 17, 2025

cdransf added the size-5 L ~30-42hrs; lots of effort or complexity, most of a sprint needed to complete. label Mar 17, 2025

adobe deleted a comment from changeset-bot Bot Mar 17, 2025

cdransf force-pushed the cdransf/s2-migration-rating branch from 16bbd31 to 39389fc Compare March 17, 2025 20:14

cdransf mentioned this pull request Mar 17, 2025

fix(rating): address inconsistent hover behavior #3196

Closed

9 tasks

cdransf added the S2 Spectrum 2 label Mar 17, 2025

cdransf force-pushed the cdransf/s2-migration-rating branch 2 times, most recently from 405e351 to 10ba423 Compare March 19, 2025 20:55

cdransf marked this pull request as ready for review March 19, 2025 21:11

cdransf requested review from 5t3ph, aramos-adobe, castastrophe and marissahuysentruyt March 19, 2025 22:43

cdransf force-pushed the cdransf/s2-migration-rating branch 2 times, most recently from 31bfeaa to 4dd1d56 Compare March 20, 2025 15:00

cdransf force-pushed the cdransf/s2-migration-rating branch 4 times, most recently from 11b8fdd to c78df20 Compare March 20, 2025 17:40

adobe deleted a comment from github-actions Bot Mar 20, 2025

cdransf force-pushed the cdransf/s2-migration-rating branch 2 times, most recently from 3dd7f1d to 2d5dd3b Compare March 20, 2025 19:37

castastrophe force-pushed the cdransf/s2-migration-rating branch from 2d5dd3b to 6b03e9e Compare March 21, 2025 14:26

castastrophe suggested changes Mar 21, 2025

View reviewed changes

castastrophe force-pushed the cdransf/s2-migration-rating branch from 6b03e9e to 0bf7c47 Compare March 21, 2025 14:43

cdransf requested a review from castastrophe March 21, 2025 16:35

cdransf force-pushed the cdransf/s2-migration-rating branch from d634218 to e576f59 Compare April 16, 2025 20:07

rise-erpelding requested changes Apr 16, 2025

View reviewed changes

cdransf and others added 3 commits April 16, 2025 16:34

chore(rating): remove duplicate mod

2939d2a

chore(rating): add removed mod

241d7c7

fix(rating): typo in class name

00f6c60

Co-authored-by: rise-erpelding <54716846+rise-erpelding@users.noreply.github.com>

cdransf force-pushed the cdransf/s2-migration-rating branch from b7f74c7 to 00f6c60 Compare April 16, 2025 23:38

cdransf added 2 commits April 16, 2025 16:47

chore(rating): remove incorrect mods and custom properties from chang…

f48bf1e

…elog

fix(rating): remove emphasized state from hover test

91d054f

cdransf force-pushed the cdransf/s2-migration-rating branch from c4fecd3 to 91d054f Compare April 17, 2025 14:23

fix(rating): adjust block and inline padding for focus states

2175ea7

cdransf requested a review from rise-erpelding April 17, 2025 15:01

fix(rating): remove manual hover classes, update class names in template

0a8b52e

cdransf force-pushed the cdransf/s2-migration-rating branch from eb547dd to 0a8b52e Compare April 17, 2025 15:05

rise-erpelding reviewed Apr 18, 2025

View reviewed changes

cdransf added 3 commits April 18, 2025 08:27

fix(rating): change EmphasizedHovered story name to Hovered

47f237d

Revert "fix(rating): adjust block and inline padding for focus states"

ebf50ff

This reverts commit 2175ea7.

fix(rating): replace padding with outline and outline-offset

9b25e3a

cdransf requested a review from rise-erpelding April 18, 2025 15:51

rise-erpelding approved these changes Apr 18, 2025

View reviewed changes

cdransf merged commit 888e63d into spectrum-two Apr 18, 2025
12 checks passed

cdransf deleted the cdransf/s2-migration-rating branch April 18, 2025 15:59

This was referenced May 16, 2025

chore(swatch,menu,swatchgroup): fix reverted metadata assets #3752

Merged

chore: merge latest updates from main #3777

Merged

castastrophe mentioned this pull request Jul 25, 2025

ci: chromatic runs for S2 [CSS-1233] #4070

Merged

3 tasks

castastrophe mentioned this pull request Aug 6, 2025

fix: minor linting updates #4113

Merged

11 tasks

This was referenced Aug 27, 2025

chore(popover): wrapper placement in story #4173

Closed

feat(storybook): chromatic a11y testing #4180

Closed

chore: updating lint-staged file inheritance order #4193

Closed

castastrophe mentioned this pull request Oct 22, 2025

feat(badge): remove modifiers from the API [SWC-1264] #4283

Merged

14 tasks

Conversation

cdransf commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additions

New mods

Removed mods

New custom properties

Removed custom properties

Validation steps

To-do list

Uh oh!

changeset-bot Bot commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions Bot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

File metrics

Summary

rating

tokens

Uh oh!

github-actions Bot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

castastrophe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rise-erpelding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rise-erpelding Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rise-erpelding Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

cdransf Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

castastrophe Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rise-erpelding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rise-erpelding Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

cdransf Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

rise-erpelding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cdransf commented Mar 17, 2025 •

edited

Loading

changeset-bot Bot commented Mar 17, 2025 •

edited

Loading

github-actions Bot commented Mar 20, 2025 •

edited

Loading

github-actions Bot commented Mar 20, 2025 •

edited

Loading