fix(rating): address inconsistent hover behavior

[cdransf]

Description

Provides more granular control over the hover behavior of child stars within the rating component to prevent hovering in space adjacent to the component from highlighting all stars.

CSS-735

How and where has this been tested?

Verified in local Storybook story for rating component.

Validation steps

1. Fetch the branch and run it locally or open the Storybook URL for the PR.
2. Navigate to the rating component.
3. Verify that hovering over rating stars is now more controlled and that hovering between stars doesn't result or all/no stars being displayed as active.
4. Verify that changing the selected star works as expected with the hover behavior.

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.

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

Screenshots

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]

🦋 Changeset detected

Latest commit: 1ed0669

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]

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

[github-actions]

File metrics

Summary

Total size: 4.27 MB^*
Total change (Δ): 🔴 ⬆ 0.61 KB (0.01%)

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

Package	Size	Δ
rating	9.99 KB	🔴 ⬆ 0.20 KB

Details

rating

Filename	Head	Compared to base
index-base.css	9.21 KB	🔴 ⬆ 0.20 KB (2.20%)
index-theme.css	1.39 KB	-
index-vars.css	9.99 KB	🔴 ⬆ 0.20 KB (2.02%)
index.css	9.99 KB	🔴 ⬆ 0.20 KB (2.02%)
themes/express.css	1.02 KB	-
themes/spectrum.css	1.01 KB	-

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

[jawinn]

When testing, I'm seeing a couple issues:

1. The focus ring should not be appearing on click. The existing is-focused is actually just for keyboard focus.

   Really.is-focused should be named .is-keyboardFocused or even better the selector could be &:has(:focus-visible), &.is-keyboardFocused { ... }, though I'm not sure we want to do that sort of renaming now? If we're already doing a breaking change, we could, though I'm hesitant to increase the scope of this work.

   I'd definitely reword the docs sentence that says "When the rating receives focus" to "When the rating receives keyboard focus".

2. Clicking on any of the ratings on the Docs page doesn't update something with the current value; the underline will show in the wrong place.

3. The is-hovered class gets leftover on elements when Rating is no longer being hovered

[jawinn]

The requirement for this class and how it should be used needs to be included in usage notes within the documentation. Within the PR description would be helpful as well.

As I am understanding it, previously CSS would set all stars filled on hover of the whole component, and then all stars following the hovered star unfilled using the subsequent-sibling combinator (~). And that will not work when you hover on the gap between items. And with this change you are looking at having a class that the implementation will apply to the hovered star as well as all previous stars? And those classes should stay until hover or focus leaves the component?

I think that could work. Perhaps this class should be something like .is-hoverSelection? It seems odd to also have is-hovered on elements that aren't actually hovered. It might also be worth adding the hover pseudo to the selector :hover, &.is-hoverSelection.

Another option would be using padding instead of gaps, but I don't know if we want to go that route or not.

[cdransf]

Got it! I'll add this to the PR description and will look for the usage notes.

That is an accurate description of the issue — I first approached it using CSS but there's an issue with one of our postcss plugins when trying to style one of the stars based on the hover state of the next: saulhardman/postcss-hover-media-feature#35.

I've updated the selector to use :hover, .is-hoverSelection. The latter is now removed from all child icons on mouseout of the parent component.

[cdransf]

I've updated is-focused to be is-keyboardFocused and changed the state in the stories and templates too.

Clicking on any of the ratings on the Docs page doesn't update something with the current value; the underline will show in the wrong place.

I need to look at this a bit more — it doesn't look like the instance in the docs is making it into the @change handler that updates this (as it works in the stories for the component):

@change=${function(e) {
  const rating = e.target.closest(`.${rootClass}`);
  if (!rating) return;

  const input = rating.closest(`.${rootClass}-input`);
  if (!input) return;
  if (!isReadOnly && !isDisabled) {
    updateArgs({ value: parseInt(input.value, 10) });
  }
}}

[pfulton]

Marking this as blocked for now until we get S2 Foundations merged into main. Once we do, we'll come back to this, get it rebased against the new main stuff, and then look to get it merged in.