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.

Sign up for GitHub

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

Jump to bottom

[docs] Add CheckboxesGroup TypeScript demo #15228

Merged
merged 4 commits into from Apr 8, 2019
Merged

[docs] Add CheckboxesGroup TypeScript demo #15228

merged 4 commits into from Apr 8, 2019

Conversation

donigianrp
Copy link
Contributor

@donigianrp donigianrp commented Apr 6, 2019 •
edited

@donigianrp
Copy link
Contributor Author

TS does not like component={'legend' as 'div'} as a solution although has no problem with component={'fieldset' as 'div'}. I followed that solution from a previous issue message chain. I'm not sure why CheckboxesGroup.js is throwing a failure.

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation typescript labels Apr 6, 2019
@eps1lon eps1lon mentioned this pull request Apr 7, 2019
Closed
eps1lon
eps1lon reviewed Apr 7, 2019
View reviewed changes
docs/src/pages/demos/selection-controls/CheckboxesGroup.tsx Outdated
return (
<div className={classes.root}>
<FormControl component={'fieldset' as 'div'} className={classes.formControl}>
<FormLabel component={'legend' as 'div'}>Assign responsibility</FormLabel>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What 'fieldset' as 'div' means is to lie to typescript that we actually use the default component. For FormControl this is div for FormLabel it would be label.

I guess legend is rejected? If so we need to cast it the the default component.

eps1lon
eps1lon reviewed Apr 7, 2019
View reviewed changes
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to run yarn:typescript:formatted to include the formatting changes in the JS demos. The changes are perfectly fine. I'm fairly certain it would be something similar to

-component="div"
+component={'div'}

@mui-pr-bot
Copy link

mui-pr-bot commented Apr 7, 2019 •
edited

No bundle size changes comparing e1b5fac...d406e5a

Generated by 🚫 dangerJS against d406e5a

@donigianrp
Copy link
Contributor Author

yarn docs:typescript:formatted changes CheckboxesGroup.js so that we are passing in styles as a prop but then removes prop-types. Lint wants me to keep prop-types in.

eps1lon
eps1lon requested changes Apr 7, 2019
View reviewed changes
docs/src/pages/demos/selection-controls/CheckboxesGroup.js Outdated
import FormLabel from '@material-ui/core/FormLabel';
import FormControl from '@material-ui/core/FormControl';
import FormGroup from '@material-ui/core/FormGroup';
import FormControlLabel from '@material-ui/core/FormControlLabel';
import FormHelperText from '@material-ui/core/FormHelperText';
import Checkbox from '@material-ui/core/Checkbox';

const useStyles = makeStyles(theme => ({
const styles = theme => ({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like you were migrating an outdated version of CheckboxesGroup. CheckboxesGroup.tsx should use the same styling solution.

@donigianrp
replace withStyles with makeStyles in CheckboxesGroup.tsx to correspo…
d406e5a 
…nd with the js file, restore CheckboxesGroup.js to it's original file
@eps1lon eps1lon changed the title [docs] Add Typescript to CheckboxesGroup demo [docs] Add CheckboxesGroup TypeScript demo Apr 8, 2019
@eps1lon eps1lon merged commit 130cc2e into mui:next Apr 8, 2019
@donigianrp donigianrp deleted the ts-demo-checkboxes-group branch April 8, 2019 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eps1lon eps1lon eps1lon approved these changes

Assignees
No one assigned
Labels
docs Improvements or additions to the documentation typescript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@donigianrp @mui-pr-bot @eps1lon @oliviertassinari