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

[Rating] [email protected] bug when readOnly and precision is float number #19368

Closed
2 tasks done
michael-land opened this issue Jan 23, 2020 · 3 comments · Fixed by #19414
Closed
2 tasks done

[Rating] [email protected] bug when readOnly and precision is float number #19368

michael-land opened this issue Jan 23, 2020 · 3 comments · Fixed by #19414
Labels
bug 🐛 Something doesn't work component: rating This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@michael-land
Copy link
Contributor

michael-land commented Jan 23, 2020

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

Current Behavior 😯

Multiple rating components displayed when readOnly and precision is floating point number

https://codesandbox.io/s/interesting-voice-pfguk

Expected Behavior 🤔

Display single rating component

Steps to Reproduce 🕹

https://codesandbox.io/s/interesting-voice-pfguk

Context 🔦

Your Environment 🌎

Tech Version
Material-UI v4.9.0
React latest
Browser chrome
TypeScript latest
etc.
@mbrookes mbrookes added bug 🐛 Something doesn't work component: rating This is the name of the generic UI component, not the React module! labels Jan 23, 2020
@jonyw4

This comment has been minimized.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 25, 2020

Oops, it's a regression from #19103. The best fix I can think of is:

diff --git a/packages/material-ui-lab/src/Rating/Rating.js b/packages/material-ui-lab/src/Rating/Rating.js
index 2089fd521..f8a94ebce 100644
--- a/packages/material-ui-lab/src/Rating/Rating.js
+++ b/packages/material-ui-lab/src/Rating/Rating.js
@@ -352,7 +352,7 @@ const Rating = React.forwardRef(function Rating(props, ref) {
     );

     if (readOnly) {
-      return <React.Fragment key={state.value}>{container}</React.Fragment>;
+      return <span key={state.value} {...labelProps}>{container}</span>;
     }

     return (

We would also need a visual test to avoid this regression in the future, something like this:

diff --git a/docs/src/pages/components/rating/HalfRating.tsx b/docs/src/pages/components/rating/HalfRating.tsx
index 3beeb8f18..c3c8f30d9 100644
--- a/docs/src/pages/components/rating/HalfRating.tsx
+++ b/docs/src/pages/components/rating/HalfRating.tsx
@@ -2,5 +2,10 @@ import React from 'react';
 import Rating from '@material-ui/lab/Rating';

 export default function HalfRating() {
-  return <Rating name="half-rating" defaultValue={2.5} precision={0.5} />;
+  return (
+    <div>
+      <Rating name="half-rating" defaultValue={2.5} precision={0.5} />
+      <Rating name="half-rating-read-only" defaultValue={2.5} precision={0.5} readOnly />
+    </div>
+  );
 }

Does anyone want to look into it?

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Jan 25, 2020
@TommyJackson85
Copy link
Contributor

@oliviertassinari I will look into it. I might ask for guidance if I am stuck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: rating This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants