[Rating] Fix readOnly + precision combination #19414

TommyJackson85 · 2020-01-26T18:18:17Z

Completed the suggested changes from issue #19368 as suggested by @oliviertassinari .

Failed tests still need to be passed / fixed.
Haven't tested this manually yet.

I will be busy the next three days so I won't have much time to work on it. Feel free to work on it in the mean time. Or suggest changes.

I have followed (at least) the PR section of the contributing guide.

Closes #19368

…ating, from changing it to read only and setting precision to a float number.

mui-pr-bot · 2020-01-26T18:28:05Z

Details of bundle changes.

Comparing: 5c84559...13da7ec

bundle	Size Change	Size	Gzip Change	Gzip
Rating	▲ +9 B (+0.01% )	70.5 kB	▼ -1 B (-0.00% )	22.7 kB
@material-ui/lab	▲ +9 B (0.00% )	185 kB	--	55.2 kB
@material-ui/core	--	360 kB	--	98.7 kB
@material-ui/core[umd]	--	317 kB	--	91.9 kB
@material-ui/styles	--	51.2 kB	--	15.4 kB
@material-ui/system	--	14.8 kB	--	4.07 kB
Alert	--	83.8 kB	--	26.3 kB
AlertTitle	--	64.2 kB	--	20.2 kB
AppBar	--	63.9 kB	--	20 kB
Autocomplete	--	132 kB	--	41.3 kB
Avatar	--	65.2 kB	--	20.6 kB
AvatarGroup	--	62.3 kB	--	19.6 kB
Backdrop	--	67.9 kB	--	21 kB
Badge	--	65.3 kB	--	20.4 kB
BottomNavigation	--	62.4 kB	--	19.6 kB
BottomNavigationAction	--	75.6 kB	--	23.9 kB
Box	--	70.9 kB	--	21.6 kB
Breadcrumbs	--	67.8 kB	--	21.3 kB
Button	--	79.8 kB	--	24.5 kB
ButtonBase	--	74 kB	--	23.3 kB
ButtonGroup	--	83.3 kB	--	25.5 kB
Card	--	62.9 kB	--	19.7 kB
CardActionArea	--	75.1 kB	--	23.7 kB
CardActions	--	62 kB	--	19.5 kB
CardContent	--	62 kB	--	19.4 kB
CardHeader	--	65.1 kB	--	20.5 kB
CardMedia	--	62.3 kB	--	19.6 kB
Checkbox	--	83 kB	--	26.3 kB
Chip	--	82.6 kB	--	25.4 kB
CircularProgress	--	64.1 kB	--	20.3 kB
ClickAwayListener	--	3.91 kB	--	1.55 kB
Collapse	--	68 kB	--	21.1 kB
colorManipulator	--	3.88 kB	--	1.52 kB
Container	--	63.3 kB	--	19.8 kB
CssBaseline	--	57.5 kB	--	18.1 kB
Dialog	--	83.1 kB	--	25.9 kB
DialogActions	--	62.1 kB	--	19.5 kB
DialogContent	--	62.2 kB	--	19.5 kB
DialogContentText	--	64 kB	--	20.2 kB
DialogTitle	--	64.3 kB	--	20.2 kB
Divider	--	62.6 kB	--	19.7 kB
docs.landing	--	50.5 kB	--	13.2 kB
docs.main	--	595 kB	--	194 kB
Drawer	--	84.8 kB	--	25.8 kB
ExpansionPanel	--	72.3 kB	--	22.7 kB
ExpansionPanelActions	--	62.1 kB	--	19.5 kB
ExpansionPanelDetails	--	61.9 kB	--	19.4 kB
ExpansionPanelSummary	--	78.2 kB	--	24.7 kB
Fab	--	76.9 kB	--	24 kB
Fade	--	23.4 kB	--	7.98 kB
FilledInput	--	73.6 kB	--	22.9 kB
FormControl	--	64.4 kB	--	20.1 kB
FormControlLabel	--	65.5 kB	--	20.6 kB
FormGroup	--	62 kB	--	19.5 kB
FormHelperText	--	63.3 kB	--	19.9 kB
FormLabel	--	63.5 kB	--	19.7 kB
Grid	--	65.1 kB	--	20.4 kB
GridList	--	62.5 kB	--	19.7 kB
GridListTile	--	63.7 kB	--	20 kB
GridListTileBar	--	63.2 kB	--	19.8 kB
Grow	--	24 kB	--	8.19 kB
Hidden	--	66 kB	--	20.7 kB
Icon	--	62.8 kB	--	19.7 kB
IconButton	--	76.2 kB	--	23.8 kB
Input	--	72.5 kB	--	22.7 kB
InputAdornment	--	65.1 kB	--	20.5 kB
InputBase	--	70.6 kB	--	22.2 kB
InputLabel	--	65.3 kB	--	20.2 kB
LinearProgress	--	65.3 kB	--	20.4 kB
Link	--	66.6 kB	--	21.1 kB
List	--	62.4 kB	--	19.5 kB
ListItem	--	77.2 kB	--	24.2 kB
ListItemAvatar	--	62.1 kB	--	19.5 kB
ListItemIcon	--	62.2 kB	--	19.5 kB
ListItemSecondaryAction	--	62 kB	--	19.5 kB
ListItemText	--	65 kB	--	20.4 kB
ListSubheader	--	62.8 kB	--	19.8 kB
Menu	--	88.8 kB	--	27.4 kB
MenuItem	--	78.2 kB	--	24.5 kB
MenuList	--	66 kB	--	20.7 kB
MobileStepper	--	67.8 kB	--	21.3 kB
Modal	--	14.5 kB	--	5.05 kB
NativeSelect	--	76.9 kB	--	24.3 kB
NoSsr	--	2.19 kB	--	1.04 kB
OutlinedInput	--	74.6 kB	--	23.2 kB
Paper	--	62.4 kB	--	19.5 kB
Popover	--	83.1 kB	--	25.8 kB
Popper	--	28.8 kB	--	10.3 kB
Portal	--	2.92 kB	--	1.3 kB
Radio	--	84.1 kB	--	26.5 kB
RadioGroup	--	64.5 kB	--	20 kB
RootRef	--	4.24 kB	--	1.64 kB
Select	--	116 kB	--	34.6 kB
Skeleton	--	62.9 kB	--	19.9 kB
Slide	--	25.5 kB	--	8.71 kB
Slider	--	76.6 kB	--	24.3 kB
Snackbar	--	75.4 kB	--	23.6 kB
SnackbarContent	--	63.5 kB	--	20 kB
SpeedDial	--	86.3 kB	--	27.2 kB
SpeedDialAction	--	118 kB	--	37.5 kB
SpeedDialIcon	--	64.6 kB	--	20.3 kB
Step	--	62.7 kB	--	19.7 kB
StepButton	--	82.4 kB	--	26.1 kB
StepConnector	--	62.7 kB	--	19.8 kB
StepContent	--	69.2 kB	--	21.7 kB
StepIcon	--	64.6 kB	--	20.2 kB
StepLabel	--	68.6 kB	--	21.7 kB
Stepper	--	64.9 kB	--	20.5 kB
styles/createMuiTheme	--	16.5 kB	--	5.81 kB
SvgIcon	--	63 kB	--	19.7 kB
SwipeableDrawer	--	92.3 kB	--	28.9 kB
Switch	--	82.2 kB	--	25.9 kB
Tab	--	76.4 kB	--	24.2 kB
Table	--	62.6 kB	--	19.7 kB
TableBody	--	62.1 kB	--	19.5 kB
TableCell	--	64 kB	--	20.2 kB
TableContainer	--	62 kB	--	19.4 kB
TableFooter	--	62.1 kB	--	19.5 kB
TableHead	--	62.1 kB	--	19.5 kB
TablePagination	--	143 kB	--	41.9 kB
TableRow	--	62.5 kB	--	19.6 kB
TableSortLabel	--	77.5 kB	--	24.4 kB
Tabs	--	85.7 kB	--	27.2 kB
TextareaAutosize	--	5.12 kB	--	2.14 kB
TextField	--	125 kB	--	36.6 kB
ToggleButton	--	76.2 kB	--	24.1 kB
ToggleButtonGroup	--	63.2 kB	--	19.9 kB
Toolbar	--	62.3 kB	--	19.6 kB
Tooltip	--	102 kB	--	32.3 kB
TreeItem	--	74 kB	--	23.4 kB
TreeView	--	66.7 kB	--	21 kB
Typography	--	63.7 kB	--	20 kB
useAutocomplete	--	14.6 kB	--	5.29 kB
useMediaQuery	--	2.51 kB	--	1.06 kB
Zoom	--	23.5 kB	--	8.1 kB

Generated by 🚫 dangerJS against 13da7ec

oliviertassinari

I have tried to push it a bit further by improving the code previews under the demo. Thanks for looking into it.

TommyJackson85 · 2020-01-27T13:30:19Z

I’ll try tonight. You could help me by telling my where to find an example page in the repository where to manually test the Rating component. I’m new to this repository.

…

Sent from my iPhone

On 27 Jan 2020, at 10:47, sureshtidbit ***@***.***> wrote: Please check and marge to the master branch because we all need these changes and solve our read-only rating- precision bug. Thanks — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

TommyJackson85

just trying to merge your changes with mine

TommyJackson85 · 2020-01-27T22:09:59Z

Can you tell me how to merge your changes with mine? I am still getting used to git :) and I wanted to test the rating component in a code sandbox with the combined changes.
Or do you think it is ready to merge with the master branch?

oliviertassinari · 2020-01-27T22:39:53Z

@TommyJackson85 You should be able to sync your local branch with your forked remote branch (rating-bug-when-readonly-and-precision-is-float-number). You can find the information to start the documentation locally in our CONTRIBUTING.md file.

oliviertassinari · 2020-01-28T17:10:51Z

@TommyJackson85 Alternatively, you can check the impact on the preview deploy: https://deploy-preview-19414--material-ui.netlify.com/. How is your local setup going? :)

TommyJackson85 · 2020-01-28T23:12:12Z

@TommyJackson85 Alternatively, you can check the impact on the preview deploy: https://deploy-preview-19414--material-ui.netlify.com/. How is your local setup going? :)

from the ratings page of the link you provided: https://deploy-preview-19414--material-ui.netlify.com/components/rating/#rating
I am testing the Rating component from the code sandbox link. Does this automatically use the sources from my PR / branch or does it take from the master branch? So far from here it is not working. Same duplication problem. I would like to try and figure out how to reconfigure this from the code sandbox. Is it the link from the package.json file in the sandbox?

Will try and fix my local setup with your suggested changes and they try and test the Rating component.

I haven't gotten much done today because ive been working for 12 hours the last two days. I should be able to try it tomorrow night onwards. Any other suggestions are welcome in the mean time, or feel free to go at it yourself if its a priority to get it done.

eps1lon · 2020-01-29T07:39:00Z

Is it the link from the package.json file in the sandbox?

Seems to be down at the moment but usually:

If you scroll down to the "Checks" list in this PR you will find a "codesandbox/ci" check. That check builds that are using the build from this PR.

I haven't gotten much done today because ive been working for 12 hours the last two days. I should be able to try it tomorrow night onwards.

You should also give yourself some rest. Don't ever feel pressured to finish a PR after you've opened it. Just let us know if you need more time and we'll give you more time.

TommyJackson85 · 2020-01-30T00:40:35Z

@eps1lon
Just realised the link in package.json is just a description. here is the sandbox: https://codesandbox.io/s/evp1k

in which, from package.json dependencies i find:
"dependencies": { "@material-ui/lab": "latest", "@material-ui/core": "latest" }

and from demo.js file I find
import Rating from '@material-ui/lab/Rating'; import Typography from '@material-ui/core/Typography'; import Box from '@material-ui/core/Box';

Id like to know, if this sandbox was opened from this link: https://deploy-preview-19414--material-ui.netlify.com/components/rating/#rating
should the code (as shown above) is from the this PR and NOT the master branch.
If its from the master branch I would like to figure out if how to reconfigure the sources to retrieve code from this PR. Just to make manual testing easier :)

I dont know where the building packages are coming from here I guess:
ci/codesandbox — Building packages succeeded.`

oliviertassinari · 2020-01-30T10:00:38Z

@TommyJackson85 To answer your questions. The edit button in the documentation doesn't take the version of the documentation into account, it uses master. This means that if you go back to an older major version of the documentation, the demo might not work. This also means that if you open a demo on a potentially future version, it might not work either.

Now, regarding how you could test the changes of this pull request in codesandbox, @eps1lon has shared the solution in its previous message.

I hope it helps. I had a closer look at the problem, I think that we are good. I'm going ahead. If it needs a follow-up, feels free to take care of it :). Thank you, we appreciate the care!

TommyJackson85 · 2020-01-30T13:36:23Z

@TommyJackson85 To answer your questions. The edit button in the documentation doesn't take the version of the documentation into account, it uses master. This means that if you go back to an older major version of the documentation, the demo might not work. This also means that if you open a demo on a potentially future version, it might not work either.

Now, regarding how you could test the changes of this pull request in codesandbox, @eps1lon has shared the solution in its previous message.

I hope it helps. I had a closer look at the problem, I think that we are good. I'm going ahead. If it needs a follow-up, feels free to take care of it :). Thank you, we appreciate the care!

Thanks for the feedback Olivier.

oliviertassinari · 2020-01-30T13:41:58Z

Don't worry, there is no reason to fix all a problem at once, as long as we make continuous small steps forward, it's great.

created initial changes to rating component to prevent it from duplic…

83f204a

…ating, from changing it to read only and setting precision to a float number.

oliviertassinari added bug 🐛 Something doesn't work component: rating This is the name of the generic UI component, not the React module! labels Jan 26, 2020

oliviertassinari changed the title ~~fix: Issue: #19368: Preventing duplication of Rating component, from being set to Readonly and setting precision to a float number.~~ [Rating] Fix readOnly + precision combination Jan 26, 2020

improve the demos

3710c4b

oliviertassinari approved these changes Jan 26, 2020

View reviewed changes

oliviertassinari added the PR: ready to ship label Jan 26, 2020

This comment has been minimized.

Sign in to view

TommyJackson85 commented Jan 27, 2020

View reviewed changes

Merge branch 'master' into pr/TommyJackson85/19414

13da7ec

oliviertassinari merged commit 7e97de3 into mui:master Jan 30, 2020

EsoterikStare pushed a commit to EsoterikStare/material-ui that referenced this pull request Feb 13, 2020

[Rating] Fix readOnly + precision combination (mui#19414)

08a23c6

This was referenced Feb 25, 2020

[Snyk] Upgrade @material-ui/core from 4.1.1 to 4.9.1 baaahs/sparklemotion#199

Closed

[Snyk] Upgrade @material-ui/core from 4.9.0 to 4.9.1 cybersecmoo/brownie-tub#19

Merged

[Snyk] Upgrade @material-ui/icons from 4.5.1 to 4.9.1 cybersecmoo/brownie-tub#20

Merged

snyk-bot mentioned this pull request Mar 22, 2020

[Snyk] Upgrade @material-ui/core from 4.9.1 to 4.9.4 mdfaizan7/react-bitConnect-client#1

Open

This was referenced Apr 16, 2020

[Snyk] Upgrade @material-ui/core from 4.3.3 to 4.9.7 reactioncommerce/reaction#6208

Merged

[Snyk] Upgrade: @material-ui/core, @material-ui/icons BrainBackup/bb8#100

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Rating] Fix readOnly + precision combination #19414

[Rating] Fix readOnly + precision combination #19414

TommyJackson85 commented Jan 26, 2020 •

edited by oliviertassinari

Loading

mui-pr-bot commented Jan 26, 2020 •

edited

Loading

oliviertassinari left a comment

This comment has been minimized.

TommyJackson85 commented Jan 27, 2020 via email

TommyJackson85 left a comment

TommyJackson85 commented Jan 27, 2020

oliviertassinari commented Jan 27, 2020

oliviertassinari commented Jan 28, 2020

TommyJackson85 commented Jan 28, 2020 •

edited

Loading

eps1lon commented Jan 29, 2020

TommyJackson85 commented Jan 30, 2020 •

edited

Loading

oliviertassinari commented Jan 30, 2020 •

edited

Loading

TommyJackson85 commented Jan 30, 2020 •

edited

Loading

oliviertassinari commented Jan 30, 2020

[Rating] Fix readOnly + precision combination #19414

[Rating] Fix readOnly + precision combination #19414

Conversation

TommyJackson85 commented Jan 26, 2020 • edited by oliviertassinari Loading

mui-pr-bot commented Jan 26, 2020 • edited Loading

oliviertassinari left a comment

Choose a reason for hiding this comment

This comment has been minimized.

TommyJackson85 commented Jan 27, 2020 via email

TommyJackson85 left a comment

Choose a reason for hiding this comment

TommyJackson85 commented Jan 27, 2020

oliviertassinari commented Jan 27, 2020

oliviertassinari commented Jan 28, 2020

TommyJackson85 commented Jan 28, 2020 • edited Loading

eps1lon commented Jan 29, 2020

TommyJackson85 commented Jan 30, 2020 • edited Loading

oliviertassinari commented Jan 30, 2020 • edited Loading

TommyJackson85 commented Jan 30, 2020 • edited Loading

oliviertassinari commented Jan 30, 2020

TommyJackson85 commented Jan 26, 2020 •

edited by oliviertassinari

Loading

mui-pr-bot commented Jan 26, 2020 •

edited

Loading

TommyJackson85 commented Jan 28, 2020 •

edited

Loading

TommyJackson85 commented Jan 30, 2020 •

edited

Loading

oliviertassinari commented Jan 30, 2020 •

edited

Loading

TommyJackson85 commented Jan 30, 2020 •

edited

Loading