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

[ButtonGroup] Add orientation prop #18762

Conversation

SandraMarcelaHerreraArriaga
Copy link
Contributor

@SandraMarcelaHerreraArriaga SandraMarcelaHerreraArriaga commented Dec 9, 2019

Closes #17884

Capture d’écran 2019-12-12 à 14 22 26
Capture d’écran 2019-12-12 à 14 22 35
Capture d’écran 2019-12-12 à 14 22 41

@oliviertassinari oliviertassinari changed the title add orientation prop [ButtonGroup] Add orientation prop Dec 9, 2019
@oliviertassinari oliviertassinari added component: ButtonGroup The React component. new feature New feature or request labels Dec 9, 2019
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Good start, could you:

  • Fix the build
  • Update the TypeScript definitions
  • Add a demo about this prop

Thanks

packages/material-ui/src/ButtonGroup/ButtonGroup.js Outdated Show resolved Hide resolved
@oliviertassinari oliviertassinari added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Dec 9, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Dec 9, 2019

@material-ui/core: parsed: +0.25% , gzip: +0.12%

Details of bundle changes.

Comparing: a8cd858...267b7f3

bundle Size Change Size Gzip Change Gzip
ButtonGroup ▲ +885 B (+1.09% ) 81.8 kB ▲ +146 B (+0.59% ) 25 kB
@material-ui/core ▲ +885 B (+0.25% ) 357 kB ▲ +120 B (+0.12% ) 97.4 kB
@material-ui/core[umd] ▲ +858 B (+0.27% ) 314 kB ▲ +137 B (+0.15% ) 90.4 kB
@material-ui/lab -- 175 kB -- 52.4 kB
@material-ui/styles -- 50.8 kB -- 15.3 kB
@material-ui/system -- 14.5 kB -- 4.04 kB
AppBar -- 62.5 kB -- 19.5 kB
Autocomplete -- 127 kB -- 39.8 kB
Avatar -- 63.9 kB -- 20.1 kB
AvatarGroup -- 60.9 kB -- 19.1 kB
Backdrop -- 66.4 kB -- 20.5 kB
Badge -- 64 kB -- 19.8 kB
BottomNavigation -- 61.1 kB -- 19.1 kB
BottomNavigationAction -- 74.1 kB -- 23.4 kB
Box -- 69.4 kB -- 21 kB
Breadcrumbs -- 66.7 kB -- 20.8 kB
Button -- 78.4 kB -- 23.9 kB
ButtonBase -- 72.6 kB -- 22.7 kB
Card -- 61.4 kB -- 19.1 kB
CardActionArea -- 73.7 kB -- 23.1 kB
CardActions -- 60.7 kB -- 19 kB
CardContent -- 60.7 kB -- 18.9 kB
CardHeader -- 63.7 kB -- 20 kB
CardMedia -- 61 kB -- 19.1 kB
Checkbox -- 80.6 kB -- 25.4 kB
Chip -- 81.2 kB -- 24.8 kB
CircularProgress -- 62.8 kB -- 19.7 kB
ClickAwayListener -- 3.85 kB -- 1.55 kB
Collapse -- 66.6 kB -- 20.6 kB
colorManipulator -- 3.85 kB -- 1.52 kB
Container -- 61.9 kB -- 19.3 kB
CssBaseline -- 56.2 kB -- 17.6 kB
Dialog -- 81.3 kB -- 25.4 kB
DialogActions -- 60.8 kB -- 19 kB
DialogContent -- 60.9 kB -- 19 kB
DialogContentText -- 62.7 kB -- 19.7 kB
DialogTitle -- 63 kB -- 19.7 kB
Divider -- 61.3 kB -- 19.2 kB
docs.landing -- 52.4 kB -- 11.4 kB
docs.main -- 610 kB -- 194 kB
Drawer -- 83 kB -- 25.2 kB
ExpansionPanel -- 69.9 kB -- 21.8 kB
ExpansionPanelActions -- 60.8 kB -- 19 kB
ExpansionPanelDetails -- 60.6 kB -- 18.9 kB
ExpansionPanelSummary -- 76.8 kB -- 24.2 kB
Fab -- 75.5 kB -- 23.4 kB
Fade -- 22.2 kB -- 7.68 kB
FilledInput -- 72.1 kB -- 22.3 kB
FormControl -- 63.1 kB -- 19.6 kB
FormControlLabel -- 64.2 kB -- 20.1 kB
FormGroup -- 60.7 kB -- 19 kB
FormHelperText -- 61.9 kB -- 19.4 kB
FormLabel -- 62.1 kB -- 19.2 kB
Grid -- 63.8 kB -- 19.9 kB
GridList -- 61.2 kB -- 19.2 kB
GridListTile -- 62.4 kB -- 19.5 kB
GridListTileBar -- 61.9 kB -- 19.3 kB
Grow -- 22.8 kB -- 7.8 kB
Hidden -- 64.6 kB -- 20.2 kB
Icon -- 61.5 kB -- 19.2 kB
IconButton -- 74.8 kB -- 23.3 kB
Input -- 71.1 kB -- 22.1 kB
InputAdornment -- 63.8 kB -- 20 kB
InputBase -- 69.2 kB -- 21.6 kB
InputLabel -- 64 kB -- 19.9 kB
LinearProgress -- 64 kB -- 19.9 kB
Link -- 65.3 kB -- 20.5 kB
List -- 61 kB -- 19 kB
ListItem -- 75.8 kB -- 23.6 kB
ListItemAvatar -- 60.8 kB -- 19 kB
ListItemIcon -- 60.9 kB -- 19 kB
ListItemSecondaryAction -- 60.7 kB -- 19 kB
ListItemText -- 63.6 kB -- 19.9 kB
ListSubheader -- 61.4 kB -- 19.3 kB
Menu -- 86.9 kB -- 26.7 kB
MenuItem -- 76.8 kB -- 23.9 kB
MenuList -- 64.6 kB -- 20.2 kB
MobileStepper -- 66.4 kB -- 20.7 kB
Modal -- 14.3 kB -- 5 kB
NativeSelect -- 75.4 kB -- 23.7 kB
NoSsr -- 2.19 kB -- 1.03 kB
OutlinedInput -- 72.6 kB -- 22.5 kB
Paper -- 60.9 kB -- 18.9 kB
Popover -- 81.3 kB -- 25.1 kB
Popper -- 28.7 kB -- 10.3 kB
Portal -- 2.9 kB -- 1.3 kB
Radio -- 81.5 kB -- 25.6 kB
RadioGroup -- 61.9 kB -- 19.3 kB
Rating -- 68.7 kB -- 22 kB
RootRef -- 4.21 kB -- 1.64 kB
Select -- 113 kB -- 33.5 kB
Skeleton -- 61.2 kB -- 19.3 kB
Slide -- 24.3 kB -- 8.3 kB
Slider -- 74.3 kB -- 23.3 kB
Snackbar -- 75.8 kB -- 23.7 kB
SnackbarContent -- 64.3 kB -- 20.2 kB
SpeedDial -- 84.7 kB -- 26.7 kB
SpeedDialAction -- 116 kB -- 36.5 kB
SpeedDialIcon -- 63.2 kB -- 19.8 kB
Step -- 61.3 kB -- 19.2 kB
StepButton -- 80.9 kB -- 25.5 kB
StepConnector -- 61.4 kB -- 19.3 kB
StepContent -- 67.7 kB -- 21.1 kB
StepIcon -- 63.3 kB -- 19.7 kB
StepLabel -- 67.3 kB -- 21.1 kB
Stepper -- 63.4 kB -- 19.9 kB
styles/createMuiTheme -- 15.4 kB -- 5.44 kB
SvgIcon -- 61.7 kB -- 19.2 kB
SwipeableDrawer -- 90.4 kB -- 27.9 kB
Switch -- 79.8 kB -- 25 kB
Tab -- 75 kB -- 23.7 kB
Table -- 61.2 kB -- 19.2 kB
TableBody -- 60.8 kB -- 19 kB
TableCell -- 62.7 kB -- 19.7 kB
TableContainer -- 60.6 kB -- 18.9 kB
TableFooter -- 60.8 kB -- 19 kB
TableHead -- 60.8 kB -- 19 kB
TablePagination -- 140 kB -- 40.8 kB
TableRow -- 61.2 kB -- 19.1 kB
TableSortLabel -- 76 kB -- 23.8 kB
Tabs -- 84.1 kB -- 26.5 kB
TextareaAutosize -- 5.06 kB -- 2.11 kB
TextField -- 122 kB -- 35.6 kB
ToggleButton -- 74.8 kB -- 23.6 kB
ToggleButtonGroup -- 61.9 kB -- 19.4 kB
Toolbar -- 61 kB -- 19.1 kB
Tooltip -- 99.5 kB -- 31.4 kB
TreeItem -- 72.3 kB -- 22.8 kB
TreeView -- 65 kB -- 20.4 kB
Typography -- 62.3 kB -- 19.4 kB
useAutocomplete -- 12.4 kB -- 4.62 kB
useMediaQuery -- 2.5 kB -- 1.06 kB
Zoom -- 22.3 kB -- 7.68 kB

Generated by 🚫 dangerJS against 267b7f3

@SandraMarcelaHerreraArriaga
Copy link
Contributor Author

I catch errors at ButtonGroup.md I already add the prop orientation but the test continue failing, what other things could I be missing?

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Running yarn prettier should help with the static fail.

packages/material-ui/src/ButtonGroup/ButtonGroup.js Outdated Show resolved Hide resolved
packages/material-ui/src/ButtonGroup/ButtonGroup.d.ts Outdated Show resolved Hide resolved
@mbrookes
Copy link
Member

mbrookes commented Dec 9, 2019

@SandraMarcelaHerreraArriaga Try: yarn docs:api

@oliviertassinari
Copy link
Member

@SandraMarcelaHerreraArriaga I believe the last missing part is a new demo, we are almost there :).

@SandraMarcelaHerreraArriaga
Copy link
Contributor Author

@SandraMarcelaHerreraArriaga I believe the last missing part is a new demo, we are almost there :).

Thank you!! I started to check how others contributors make a demo :)

@SandraMarcelaHerreraArriaga
Copy link
Contributor Author

SandraMarcelaHerreraArriaga commented Dec 11, 2019

@SandraMarcelaHerreraArriaga I believe the last missing part is a new demo, we are almost there :).

Hello, I have been added in the current demo but I'm not sure why now two tests continue failing...
I read again the contributing and try some commands to fix some tests as the file mention.
I modify the /material-ui/docs/src/pages/components/buttons/GroupedButtons.tsx. How another way can I check if my demo is correct? Thanks in advance :)

@oliviertassinari
Copy link
Member

Thanks for the demos, what do you thinking of breaking it up into smaller demos? Outside visual regression tests, I don't think that we need to render all the size or orientation variants

@SandraMarcelaHerreraArriaga
Copy link
Contributor Author

Thanks for the demos, what do you thinking of breaking it up into smaller demos? Outside visual regression tests, I don't think that we need to render all the size or orientation variants

Ohh I see, got it. let me try :) and fix all the size or orientation variants that I made before

@oliviertassinari oliviertassinari removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Dec 12, 2019
@@ -301,7 +301,7 @@ function Demo(props) {
!demoOptions.hideHeader &&
demoOptions.defaultCodeOpen !== false &&
jsx !== demoData.raw &&
jsx.split(/\n/).length <= 15;
jsx.split(/\n/).length <= 16;
Copy link
Member

Choose a reason for hiding this comment

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

@mbrookes one step at the time 👼.

@oliviertassinari oliviertassinari merged commit 6fb06f9 into mui:master Dec 12, 2019
@oliviertassinari
Copy link
Member

@SandraMarcelaHerreraArriaga Thanks

@aress31
Copy link

aress31 commented Dec 12, 2019

May I also suggest to add some responsiveness by automatically triggering the orientation prop to vertical whenever the screen width becomes inferior to the ButtonGroup width.

@oliviertassinari
Copy link
Member

@aress31 Thanks for the suggestion. I don't think that we should bake this feature in. I think that an external wrapper, possibly in userland would be better to start with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ButtonGroup The React component. new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ButtonGroup] Support vertical option
5 participants