Material-ui: [Rating] Wrong star selected when clicking between icons

[RatherSlowTurtle]

Hovering over the right edge of a star does not highlight the next star, but if you click it will select the next star.

• [X] The issue is present in the latest release.
• [X] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Clicking the right edge of a star selects the next star.

Expected Behavior 🤔

It should select the currently highlighted star.

Steps to Reproduce 🕹

If you zoom in closely on the Rating component and then hover as far to the right of a star as you can (without highlighting the next star), then click, it will select the star to the right, even though it was never highlighted.

[eps1lon]

Had to zoom in at 200% to reproduce it but there's definitely an unstable region between two stars.

[RatherSlowTurtle]

I'm happy to contribute if there's a preferred approach to addressing it.

After some brief playing, I think I've found 2 workarounds in Chrome (but they're possibly masking over the underlying issue):

• Reversing the label z-index (so it goes from highest to lowest) - because it causes the click to be captured by the label on the left instead of right when they're edge-to-edge
• Adding a 0.5px transparent right pseudoelement, as it causes the left label to grow and be captured before the right when they're edge-to-edge

[croraf]

Anyone knows what is this line for: https://github.com/mui-org/material-ui/blob/next/packages/material-ui/src/Rating/Rating.js#L345 ?

[eps1lon]

Anyone knows what is this line for: https://github.com/mui-org/material-ui/blob/next/packages/material-ui/src/Rating/Rating.js#L345 ?

This labels the currently selected rating.

[croraf]

I think I have found what is probably the cause. Check this test:

When hover happens the outer label does not expand beyond boundaries, but its immediate child <span class="MuiRating-icon does expand beyond the boundaries.

The overflow: hidden on the label element might be a quick solution.

[croraf]

After further exploration, even though the previously mentioned issue is kind of bad, it seems that it is not the cause of the original issue (and it does not solve it).

It seems that the root cause has to do something with the handleMouseMove and/or percentage calculation.

[croraf]

After more exploration it seems that the browsers precision (mouse position granularity) causes the issue as per: https://stackoverflow.com/questions/47019163/mouseevent-high-precision-mouse-position-when-the-page-is-zoomed-in

The logic in the code says that on >24px mouse (clientX) move to the right (from the left edge of the Rating) the second star has to be highlighted.

If you move the mouse very close to that position, to get the mouse position of 23.6px for example, then until you get with the mouse to the 24.6px (or similar) you will have the clientX recorded as the 23.6 (looks like clientX operates on 1px CSS precision).

As you can see here I moved mouse 13 times in the right direction to move from 23.399 to 24.3999:

But, as the onClick (and onHover) resolutions don't get reduced when you zoom in, the onClick (and onHover) will understand that you passed 24px and that you entered in the area of the next HTML element, and will trigger events on it.

[RatherSlowTurtle]

Had to zoom in at 200% to reproduce it but there's definitely an unstable region between two stars.

Worth pointing out... It's more apparent when zooming, but I'm not sure the issue is related to zooming. I can reproduce this bug, consistently, without zooming at all.

[RatherSlowTurtle]

After more exploration it seems that the browsers precision (mouse position granularity) causes the issue as per: https://stackoverflow.com/questions/47019163/mouseevent-high-precision-mouse-position-when-the-page-is-zoomed-in

The logic in the code says that on >24px mouse (clientX) move to the right (from the left edge of the Rating) the second star has to be highlighted.

If you move the mouse very close to that position, to get the mouse position of 23.6px for example, then until you get with the mouse to the 24.6px (or similar) you will have the clientX recorded as the 23.6 (looks like clientX operates on 1px CSS precision).

As you can see here I moved mouse 13 times in the right direction to move from 23.399 to 24.3999:

But, as the onClick (and onHover) resolutions don't get reduced when you zoom in, the onClick (and onHover) will understand that you passed 24px and that you entered in the area of the next HTML element, and will trigger events on it.

It does definitely seem like the hover calculation needs some tweaking.

Pink is the currently hovered label:

[oliviertassinari]

The only fix I could find is:

diff --git a/packages/material-ui/src/Rating/Rating.js b/packages/material-ui/src/Rating/Rating.js
index e89278b2b8..9d4a558718 100644
--- a/packages/material-ui/src/Rating/Rating.js
+++ b/packages/material-ui/src/Rating/Rating.js
@@ -245,7 +245,11 @@ const Rating = React.forwardRef(function Rating(props, ref) {
   };

   const handleChange = (event) => {
-    const newValue = parseFloat(event.target.value);
+    let newValue = parseFloat(event.target.value);
+
+    if (hover !== -1) {
+      newValue = hover;
+    }

     setValueState(newValue);

The diff makes the mouse move logic take precedence over the input change event when possible. Something is wrong with the <label> interaction area. The click area is wider than the intrinsic size of the element. I could reproduce on Edge, Safari, and Chrome, not Firefox.

[bearfromtheabyss]

If it's free, you can assign it to me :)

[RatherSlowTurtle]

. Something is wrong with the <label> interaction area. The click area is wider than the intrinsic size of the element. I could reproduce on Edge, Safari, and Chrome, not Firefox.

Yeah, very odd! The z-index seems to affect this, so if the z-index is reversed, the interaction from the element on the left takes precedence and the issues doesn't seem to happen.

Thanks for investigating 👍

[croraf]

Guys I have completely bisected the issue and revealed what is the root cause in my post from last week https://github.com/mui-org/material-ui/issues/22827#issuecomment-702423730. The click (and hover) surface detection are much more precise than the granulation of reported mouse positions in mouse events.

[oliviertassinari]

@croraf Any idea on how to turn this investigation into a fix?

[croraf]

One of the options is that the click even listener of the stars does not trigger the rating value change.
But instead that the root component listens for that, and put the rating value depending on the mouse X offset. Then the "hover" and "click" will always be in sync cause they are calculated by the same method.

---

There will still be the negligible issue that when you enter in the next start by less than 0.5px (or so) that all the events will be done on the previous start. And vice versa. But the events will be synchronized, and it will be practically impossible to notice this.

[IngridFuentes]

I can work on it if it's still free :)

[redbmk]

Hey, sorry @IngridFuentes didn't see that comment until I ended up getting a fix for it. I tried a couple approaches but ended up with pretty much what @croraf suggested in https://github.com/mui-org/material-ui/issues/22827#issuecomment-705788208.

Would love some feedback though, especially in regards to accessibility.

[redbmk]

Hmm, played around with it some more and a11y is definitely broken in my PR. May need to come up with a solution where hover/click works one way but focus/onChange works another way

[oliviertassinari]

What's wrong with this solution https://github.com/mui-org/material-ui/issues/22827#issuecomment-704405240?

[redbmk]

@oliviertassinari ah, yeah sorry somehow I overlooked your comment - that seems to work great