From 393f3a07b571e32f96b14bc02ce5f70dc94be8fc Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Thu, 17 Aug 2023 12:29:28 +0530 Subject: [PATCH 01/19] add regression test --- .../fixtures/ButtonGroup/DifferentChildren.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 test/regressions/fixtures/ButtonGroup/DifferentChildren.js diff --git a/test/regressions/fixtures/ButtonGroup/DifferentChildren.js b/test/regressions/fixtures/ButtonGroup/DifferentChildren.js new file mode 100644 index 00000000000000..726726b9813ece --- /dev/null +++ b/test/regressions/fixtures/ButtonGroup/DifferentChildren.js @@ -0,0 +1,33 @@ +import * as React from 'react'; +import Button from '@mui/material/Button'; +import ButtonGroup from '@mui/material/ButtonGroup'; +import ArrowDropDownIcon from '@mui/icons-material/ArrowDropDown'; +import { Stack, Tooltip } from '@mui/material'; + +export default function DifferentChildren() { + return ( + + {/* It has one button with href which is rendered as anchor tag */} + + + + + + {/* With tooltip */} + + + + + + + + + + + + + + ); +} From 9dcb3270c74fabd12beb56383bb4fef26ae586a9 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Thu, 17 Aug 2023 12:30:14 +0530 Subject: [PATCH 02/19] [ButtonGroup] Determine first and last buttons --- packages/mui-material/src/Button/Button.js | 20 ++++++++++++- .../src/ButtonGroup/ButtonGroup.js | 30 +++++++++++++++++-- .../ButtonGroup/ButtonGroupButtonContext.ts | 19 ++++++++++++ .../src/ButtonGroup/buttonGroupClasses.ts | 6 ++++ 4 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index dd26bdd925f4f2..8ed6b27e81ed5f 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -11,6 +11,7 @@ import ButtonBase from '../ButtonBase'; import capitalize from '../utils/capitalize'; import buttonClasses, { getButtonUtilityClass } from './buttonClasses'; import ButtonGroupContext from '../ButtonGroup/ButtonGroupContext'; +import ButtonGroupButtonContext from '../ButtonGroup/ButtonGroupButtonContext'; const useUtilityClasses = (ownerState) => { const { color, disableElevation, fullWidth, size, variant, classes } = ownerState; @@ -295,9 +296,21 @@ const ButtonEndIcon = styled('span', { ...commonIconStyles(ownerState), })); +const getClassNameBasedOnPosition = (buttonGroupButtonContext) => { + if (buttonGroupButtonContext.firstButton) { + return buttonGroupButtonContext.firstButtonClass; + } + + if (buttonGroupButtonContext.lastButton) { + return buttonGroupButtonContext.lastButtonClass; + } + return ''; +}; + const Button = React.forwardRef(function Button(inProps, ref) { // props priority: `inProps` > `contextProps` > `themeDefaultProps` const contextProps = React.useContext(ButtonGroupContext); + const buttonGroupButtonContext = React.useContext(ButtonGroupButtonContext); const resolvedProps = resolveProps(contextProps, inProps); const props = useThemeProps({ props: resolvedProps, name: 'MuiButton' }); const { @@ -348,7 +361,12 @@ const Button = React.forwardRef(function Button(inProps, ref) { return ( { const { ownerState } = props; @@ -27,6 +28,8 @@ const overridesResolver = (props, styles) => { [`& .${buttonGroupClasses.grouped}`]: styles[`grouped${capitalize(ownerState.variant)}${capitalize(ownerState.color)}`], }, + { [`& .${buttonGroupClasses.firstButton}`]: styles.firstButton }, + { [`& .${buttonGroupClasses.lastButton}`]: styles.lastButton }, styles.root, styles[ownerState.variant], ownerState.disableElevation === true && styles.disableElevation, @@ -55,6 +58,8 @@ const useUtilityClasses = (ownerState) => { `grouped${capitalize(variant)}${capitalize(color)}`, disabled && 'disabled', ], + firstButton: ['firstButton'], + lastButton: ['lastButton'], }; return composeClasses(slots, getButtonGroupUtilityClass, classes); @@ -81,7 +86,7 @@ const ButtonGroupRoot = styled('div', { }), [`& .${buttonGroupClasses.grouped}`]: { minWidth: 40, - '&:not(:first-of-type)': { + [`&.${buttonGroupClasses.lastButton}`]: { ...(ownerState.orientation === 'horizontal' && { borderTopLeftRadius: 0, borderBottomLeftRadius: 0, @@ -99,7 +104,7 @@ const ButtonGroupRoot = styled('div', { marginTop: -1, }), }, - '&:not(:last-of-type)': { + [`&.${buttonGroupClasses.firstButton}`]: { ...(ownerState.orientation === 'horizontal' && { borderTopRightRadius: 0, borderBottomRightRadius: 0, @@ -243,6 +248,13 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { ], ); + const buttonGroupPositionContext = (index, childrenParam) => ({ + firstButton: index === 0, + lastButton: index === childrenParam.length - 1, + firstButtonClass: classes.firstButton, + lastButtonClass: classes.lastButton, + }); + return ( - {children} + + {React.Children.map(children, (child, index) => { + if (!React.isValidElement(child)) { + return child; + } + + return ( + + {child} + + ); + })} + ); }); diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts new file mode 100644 index 00000000000000..a7b77e62de5b55 --- /dev/null +++ b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts @@ -0,0 +1,19 @@ +import * as React from 'react'; + +interface IButtonGroupButtonContext { + firstButton?: boolean; + lastButton?: boolean; + firstButtonClass?: string; + lastButtonClass?: string; +} + +/** + * @ignore - internal component. + */ +const ButtonGroupButtonContext = React.createContext({}); + +if (process.env.NODE_ENV !== 'production') { + ButtonGroupButtonContext.displayName = 'ButtonGroupButtonContext'; +} + +export default ButtonGroupButtonContext; diff --git a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts index 55dec11fdfe720..05d36119b703f3 100644 --- a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts +++ b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts @@ -16,6 +16,8 @@ export interface ButtonGroupClasses { disabled: string; /** Styles applied to the root element if `fullWidth={true}`. */ fullWidth: string; + /** Styled applied to the button child if it is the first button. */ + firstButton: string; /** Styles applied to the root element if `orientation="vertical"`. */ vertical: string; /** Styles applied to the children. */ @@ -54,6 +56,8 @@ export interface ButtonGroupClasses { groupedContainedPrimary: string; /** Styles applied to the children if `variant="contained"` and `color="secondary"`. */ groupedContainedSecondary: string; + /** Styled applied to the button child if it is the last button. */ + lastButton: string; } export type ButtonGroupClassKey = keyof ButtonGroupClasses; @@ -70,6 +74,7 @@ const buttonGroupClasses: ButtonGroupClasses = generateUtilityClasses('MuiButton 'disableElevation', 'disabled', 'fullWidth', + 'firstButton', 'vertical', 'grouped', 'groupedHorizontal', @@ -89,6 +94,7 @@ const buttonGroupClasses: ButtonGroupClasses = generateUtilityClasses('MuiButton 'groupedContainedVertical', 'groupedContainedPrimary', 'groupedContainedSecondary', + 'lastButton', ]); export default buttonGroupClasses; From 01103cc8b5c207891fc266f12725a2b5204f1ae6 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Thu, 17 Aug 2023 18:05:33 +0530 Subject: [PATCH 03/19] add middle button class --- packages/mui-material/src/Button/Button.js | 2 +- .../src/ButtonGroup/ButtonGroup.js | 179 +++++++++--------- .../ButtonGroup/ButtonGroupButtonContext.ts | 1 + .../src/ButtonGroup/buttonGroupClasses.ts | 7 +- .../fixtures/ButtonGroup/DifferentChildren.js | 50 ++--- 5 files changed, 124 insertions(+), 115 deletions(-) diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index 8ed6b27e81ed5f..31c069d6467131 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -304,7 +304,7 @@ const getClassNameBasedOnPosition = (buttonGroupButtonContext) => { if (buttonGroupButtonContext.lastButton) { return buttonGroupButtonContext.lastButtonClass; } - return ''; + return buttonGroupButtonContext.middleButtonClass; }; const Button = React.forwardRef(function Button(inProps, ref) { diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.js index 3d59a00a76e5c8..9b26d43dc6910a 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.js @@ -30,6 +30,7 @@ const overridesResolver = (props, styles) => { }, { [`& .${buttonGroupClasses.firstButton}`]: styles.firstButton }, { [`& .${buttonGroupClasses.lastButton}`]: styles.lastButton }, + { [`& .${buttonGroupClasses.middleButton}`]: styles.middleButton }, styles.root, styles[ownerState.variant], ownerState.disableElevation === true && styles.disableElevation, @@ -60,6 +61,7 @@ const useUtilityClasses = (ownerState) => { ], firstButton: ['firstButton'], lastButton: ['lastButton'], + middleButton: ['middleButton'], }; return composeClasses(slots, getButtonGroupUtilityClass, classes); @@ -86,106 +88,106 @@ const ButtonGroupRoot = styled('div', { }), [`& .${buttonGroupClasses.grouped}`]: { minWidth: 40, - [`&.${buttonGroupClasses.lastButton}`]: { - ...(ownerState.orientation === 'horizontal' && { - borderTopLeftRadius: 0, - borderBottomLeftRadius: 0, - }), - ...(ownerState.orientation === 'vertical' && { - borderTopRightRadius: 0, - borderTopLeftRadius: 0, + '&:hover': { + ...(ownerState.variant === 'contained' && { + boxShadow: 'none', }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'horizontal' && { - marginLeft: -1, - }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'vertical' && { - marginTop: -1, - }), }, - [`&.${buttonGroupClasses.firstButton}`]: { - ...(ownerState.orientation === 'horizontal' && { - borderTopRightRadius: 0, - borderBottomRightRadius: 0, + ...(ownerState.variant === 'contained' && { + boxShadow: 'none', + }), + }, + [`& .${buttonGroupClasses.firstButton},.${buttonGroupClasses.middleButton}`]: { + ...(ownerState.orientation === 'horizontal' && { + borderTopRightRadius: 0, + borderBottomRightRadius: 0, + }), + ...(ownerState.orientation === 'vertical' && { + borderBottomRightRadius: 0, + borderBottomLeftRadius: 0, + }), + ...(ownerState.variant === 'text' && + ownerState.orientation === 'horizontal' && { + borderRight: theme.vars + ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` + : `1px solid ${ + theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' + }`, + [`&.${buttonGroupClasses.disabled}`]: { + borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, }), - ...(ownerState.orientation === 'vertical' && { - borderBottomRightRadius: 0, - borderBottomLeftRadius: 0, + ...(ownerState.variant === 'text' && + ownerState.orientation === 'vertical' && { + borderBottom: theme.vars + ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` + : `1px solid ${ + theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' + }`, + [`&.${buttonGroupClasses.disabled}`]: { + borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, }), - ...(ownerState.variant === 'text' && - ownerState.orientation === 'horizontal' && { - borderRight: theme.vars - ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` - : `1px solid ${ - theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' - }`, - [`&.${buttonGroupClasses.disabled}`]: { - borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, - }), - ...(ownerState.variant === 'text' && - ownerState.orientation === 'vertical' && { - borderBottom: theme.vars - ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` - : `1px solid ${ - theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' - }`, - [`&.${buttonGroupClasses.disabled}`]: { - borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, - }), - ...(ownerState.variant === 'text' && - ownerState.color !== 'inherit' && { - borderColor: theme.vars - ? `rgba(${theme.vars.palette[ownerState.color].mainChannel} / 0.5)` - : alpha(theme.palette[ownerState.color].main, 0.5), - }), + ...(ownerState.variant === 'text' && + ownerState.color !== 'inherit' && { + borderColor: theme.vars + ? `rgba(${theme.vars.palette[ownerState.color].mainChannel} / 0.5)` + : alpha(theme.palette[ownerState.color].main, 0.5), + }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'horizontal' && { + borderRightColor: 'transparent', + }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'vertical' && { + borderBottomColor: 'transparent', + }), + ...(ownerState.variant === 'contained' && + ownerState.orientation === 'horizontal' && { + borderRight: `1px solid ${(theme.vars || theme).palette.grey[400]}`, + [`&.${buttonGroupClasses.disabled}`]: { + borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, + }), + ...(ownerState.variant === 'contained' && + ownerState.orientation === 'vertical' && { + borderBottom: `1px solid ${(theme.vars || theme).palette.grey[400]}`, + [`&.${buttonGroupClasses.disabled}`]: { + borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, + }), + ...(ownerState.variant === 'contained' && + ownerState.color !== 'inherit' && { + borderColor: (theme.vars || theme).palette[ownerState.color].dark, + }), + '&:hover': { ...(ownerState.variant === 'outlined' && ownerState.orientation === 'horizontal' && { - borderRightColor: 'transparent', + borderRightColor: 'currentColor', }), ...(ownerState.variant === 'outlined' && ownerState.orientation === 'vertical' && { - borderBottomColor: 'transparent', - }), - ...(ownerState.variant === 'contained' && - ownerState.orientation === 'horizontal' && { - borderRight: `1px solid ${(theme.vars || theme).palette.grey[400]}`, - [`&.${buttonGroupClasses.disabled}`]: { - borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, - }), - ...(ownerState.variant === 'contained' && - ownerState.orientation === 'vertical' && { - borderBottom: `1px solid ${(theme.vars || theme).palette.grey[400]}`, - [`&.${buttonGroupClasses.disabled}`]: { - borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, - }), - ...(ownerState.variant === 'contained' && - ownerState.color !== 'inherit' && { - borderColor: (theme.vars || theme).palette[ownerState.color].dark, + borderBottomColor: 'currentColor', }), - '&:hover': { - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'horizontal' && { - borderRightColor: 'currentColor', - }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'vertical' && { - borderBottomColor: 'currentColor', - }), - }, }, - '&:hover': { - ...(ownerState.variant === 'contained' && { - boxShadow: 'none', - }), - }, - ...(ownerState.variant === 'contained' && { - boxShadow: 'none', + }, + [`& .${buttonGroupClasses.lastButton},.${buttonGroupClasses.middleButton}`]: { + ...(ownerState.orientation === 'horizontal' && { + borderTopLeftRadius: 0, + borderBottomLeftRadius: 0, + }), + ...(ownerState.orientation === 'vertical' && { + borderTopRightRadius: 0, + borderTopLeftRadius: 0, }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'horizontal' && { + marginLeft: -1, + }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'vertical' && { + marginTop: -1, + }), }, })); @@ -253,6 +255,7 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { lastButton: index === childrenParam.length - 1, firstButtonClass: classes.firstButton, lastButtonClass: classes.lastButton, + middleButtonClass: classes.middleButton, }); return ( diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts index a7b77e62de5b55..0a4ff727743894 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts +++ b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts @@ -5,6 +5,7 @@ interface IButtonGroupButtonContext { lastButton?: boolean; firstButtonClass?: string; lastButtonClass?: string; + middleButtonClass?: string; } /** diff --git a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts index 05d36119b703f3..428a17faa7c498 100644 --- a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts +++ b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts @@ -16,7 +16,7 @@ export interface ButtonGroupClasses { disabled: string; /** Styles applied to the root element if `fullWidth={true}`. */ fullWidth: string; - /** Styled applied to the button child if it is the first button. */ + /** Styles applied to the button child if it is the first button. */ firstButton: string; /** Styles applied to the root element if `orientation="vertical"`. */ vertical: string; @@ -56,8 +56,10 @@ export interface ButtonGroupClasses { groupedContainedPrimary: string; /** Styles applied to the children if `variant="contained"` and `color="secondary"`. */ groupedContainedSecondary: string; - /** Styled applied to the button child if it is the last button. */ + /** Styles applied to the button child if it is the last button. */ lastButton: string; + /** Style applied to the button child if it is in the the middle buttons. */ + middleButton: string; } export type ButtonGroupClassKey = keyof ButtonGroupClasses; @@ -95,6 +97,7 @@ const buttonGroupClasses: ButtonGroupClasses = generateUtilityClasses('MuiButton 'groupedContainedPrimary', 'groupedContainedSecondary', 'lastButton', + 'middleButton', ]); export default buttonGroupClasses; diff --git a/test/regressions/fixtures/ButtonGroup/DifferentChildren.js b/test/regressions/fixtures/ButtonGroup/DifferentChildren.js index 726726b9813ece..618a30ae4d2fa5 100644 --- a/test/regressions/fixtures/ButtonGroup/DifferentChildren.js +++ b/test/regressions/fixtures/ButtonGroup/DifferentChildren.js @@ -1,33 +1,35 @@ import * as React from 'react'; import Button from '@mui/material/Button'; import ButtonGroup from '@mui/material/ButtonGroup'; -import ArrowDropDownIcon from '@mui/icons-material/ArrowDropDown'; -import { Stack, Tooltip } from '@mui/material'; +import Stack from '@mui/material/Stack'; +import Tooltip from '@mui/material/Tooltip'; export default function DifferentChildren() { return ( - - {/* It has one button with href which is rendered as anchor tag */} - - - - + + {/* It has one button with href which is rendered as anchor tag */} + + + + + - {/* With tooltip */} - - - - - - - - - - - - - + {/* With tooltip */} + + + + + + + + + + + + + + + + ); } From 62af815344724b5097d3a643538d575375dce56b Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Thu, 17 Aug 2023 18:10:03 +0530 Subject: [PATCH 04/19] update docs --- docs/pages/material-ui/api/button-group.json | 5 ++++- .../api-docs/button-group/button-group.json | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/docs/pages/material-ui/api/button-group.json b/docs/pages/material-ui/api/button-group.json index ef1eb072c05e1b..2b202fcc4e5e28 100644 --- a/docs/pages/material-ui/api/button-group.json +++ b/docs/pages/material-ui/api/button-group.json @@ -55,6 +55,7 @@ "disableElevation", "disabled", "fullWidth", + "firstButton", "vertical", "grouped", "groupedHorizontal", @@ -73,7 +74,9 @@ "groupedContainedHorizontal", "groupedContainedVertical", "groupedContainedPrimary", - "groupedContainedSecondary" + "groupedContainedSecondary", + "lastButton", + "middleButton" ], "globalClasses": { "disabled": "Mui-disabled" }, "name": "MuiButtonGroup" diff --git a/docs/translations/api-docs/button-group/button-group.json b/docs/translations/api-docs/button-group/button-group.json index 4f813acc0ba716..157e405f52ab6c 100644 --- a/docs/translations/api-docs/button-group/button-group.json +++ b/docs/translations/api-docs/button-group/button-group.json @@ -61,6 +61,11 @@ "nodeName": "the root element", "conditions": "fullWidth={true}" }, + "firstButton": { + "description": "Styles applied to {{nodeName}} if {{conditions}}.", + "nodeName": "the button child", + "conditions": "it is the first button" + }, "vertical": { "description": "Styles applied to {{nodeName}} if {{conditions}}.", "nodeName": "the root element", @@ -151,6 +156,14 @@ "description": "Styles applied to {{nodeName}} if {{conditions}}.", "nodeName": "the children", "conditions": "variant=\"contained\" and color=\"secondary\"" + }, + "lastButton": { + "description": "Styles applied to {{nodeName}} if {{conditions}}.", + "nodeName": "the button child", + "conditions": "it is the last button" + }, + "middleButton": { + "description": "Style applied to the button child if it is in the the middle buttons." } } } From f4bb2b9dbe46058489c81aa705f678ea98ac6796 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Thu, 17 Aug 2023 18:52:35 +0530 Subject: [PATCH 05/19] fix style description --- docs/translations/api-docs/button-group/button-group.json | 2 +- packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/translations/api-docs/button-group/button-group.json b/docs/translations/api-docs/button-group/button-group.json index 157e405f52ab6c..ecb07857fdc248 100644 --- a/docs/translations/api-docs/button-group/button-group.json +++ b/docs/translations/api-docs/button-group/button-group.json @@ -163,7 +163,7 @@ "conditions": "it is the last button" }, "middleButton": { - "description": "Style applied to the button child if it is in the the middle buttons." + "description": "Style applied to the button child if it is in the middle buttons." } } } diff --git a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts index 428a17faa7c498..081d6cd1521870 100644 --- a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts +++ b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts @@ -58,7 +58,7 @@ export interface ButtonGroupClasses { groupedContainedSecondary: string; /** Styles applied to the button child if it is the last button. */ lastButton: string; - /** Style applied to the button child if it is in the the middle buttons. */ + /** Style applied to the button child if it is in the middle buttons. */ middleButton: string; } From a7f1084dab4b4b83397874579c98a6a3034ef7f1 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Fri, 18 Aug 2023 19:53:51 +0530 Subject: [PATCH 06/19] use data- attributes instead of class names --- docs/pages/material-ui/api/button-group.json | 5 +---- .../api-docs/button-group/button-group.json | 13 ------------- packages/mui-material/src/Button/Button.js | 19 ++++++++----------- .../src/ButtonGroup/ButtonGroup.js | 13 ++----------- .../ButtonGroup/ButtonGroupButtonContext.ts | 3 --- .../src/ButtonGroup/buttonGroupClasses.ts | 9 --------- 6 files changed, 11 insertions(+), 51 deletions(-) diff --git a/docs/pages/material-ui/api/button-group.json b/docs/pages/material-ui/api/button-group.json index 2b202fcc4e5e28..ef1eb072c05e1b 100644 --- a/docs/pages/material-ui/api/button-group.json +++ b/docs/pages/material-ui/api/button-group.json @@ -55,7 +55,6 @@ "disableElevation", "disabled", "fullWidth", - "firstButton", "vertical", "grouped", "groupedHorizontal", @@ -74,9 +73,7 @@ "groupedContainedHorizontal", "groupedContainedVertical", "groupedContainedPrimary", - "groupedContainedSecondary", - "lastButton", - "middleButton" + "groupedContainedSecondary" ], "globalClasses": { "disabled": "Mui-disabled" }, "name": "MuiButtonGroup" diff --git a/docs/translations/api-docs/button-group/button-group.json b/docs/translations/api-docs/button-group/button-group.json index ecb07857fdc248..4f813acc0ba716 100644 --- a/docs/translations/api-docs/button-group/button-group.json +++ b/docs/translations/api-docs/button-group/button-group.json @@ -61,11 +61,6 @@ "nodeName": "the root element", "conditions": "fullWidth={true}" }, - "firstButton": { - "description": "Styles applied to {{nodeName}} if {{conditions}}.", - "nodeName": "the button child", - "conditions": "it is the first button" - }, "vertical": { "description": "Styles applied to {{nodeName}} if {{conditions}}.", "nodeName": "the root element", @@ -156,14 +151,6 @@ "description": "Styles applied to {{nodeName}} if {{conditions}}.", "nodeName": "the children", "conditions": "variant=\"contained\" and color=\"secondary\"" - }, - "lastButton": { - "description": "Styles applied to {{nodeName}} if {{conditions}}.", - "nodeName": "the button child", - "conditions": "it is the last button" - }, - "middleButton": { - "description": "Style applied to the button child if it is in the middle buttons." } } } diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index 31c069d6467131..eb713d00a262f5 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -296,15 +296,14 @@ const ButtonEndIcon = styled('span', { ...commonIconStyles(ownerState), })); -const getClassNameBasedOnPosition = (buttonGroupButtonContext) => { +const addPositionDataAttr = (buttonGroupButtonContext) => { if (buttonGroupButtonContext.firstButton) { - return buttonGroupButtonContext.firstButtonClass; + return { 'data-first-child': '' }; } - if (buttonGroupButtonContext.lastButton) { - return buttonGroupButtonContext.lastButtonClass; + return { 'data-last-child': '' }; } - return buttonGroupButtonContext.middleButtonClass; + return { 'data-middle-child': '' }; }; const Button = React.forwardRef(function Button(inProps, ref) { @@ -358,21 +357,19 @@ const Button = React.forwardRef(function Button(inProps, ref) { ); + const dataAttrs = addPositionDataAttr(buttonGroupButtonContext); + return ( diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.js index 9b26d43dc6910a..886f09ad7ab57e 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.js @@ -28,9 +28,6 @@ const overridesResolver = (props, styles) => { [`& .${buttonGroupClasses.grouped}`]: styles[`grouped${capitalize(ownerState.variant)}${capitalize(ownerState.color)}`], }, - { [`& .${buttonGroupClasses.firstButton}`]: styles.firstButton }, - { [`& .${buttonGroupClasses.lastButton}`]: styles.lastButton }, - { [`& .${buttonGroupClasses.middleButton}`]: styles.middleButton }, styles.root, styles[ownerState.variant], ownerState.disableElevation === true && styles.disableElevation, @@ -59,9 +56,6 @@ const useUtilityClasses = (ownerState) => { `grouped${capitalize(variant)}${capitalize(color)}`, disabled && 'disabled', ], - firstButton: ['firstButton'], - lastButton: ['lastButton'], - middleButton: ['middleButton'], }; return composeClasses(slots, getButtonGroupUtilityClass, classes); @@ -97,7 +91,7 @@ const ButtonGroupRoot = styled('div', { boxShadow: 'none', }), }, - [`& .${buttonGroupClasses.firstButton},.${buttonGroupClasses.middleButton}`]: { + '& [data-first-child],[data-middle-child]': { ...(ownerState.orientation === 'horizontal' && { borderTopRightRadius: 0, borderBottomRightRadius: 0, @@ -171,7 +165,7 @@ const ButtonGroupRoot = styled('div', { }), }, }, - [`& .${buttonGroupClasses.lastButton},.${buttonGroupClasses.middleButton}`]: { + '& [data-last-child],[data-middle-child]': { ...(ownerState.orientation === 'horizontal' && { borderTopLeftRadius: 0, borderBottomLeftRadius: 0, @@ -253,9 +247,6 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { const buttonGroupPositionContext = (index, childrenParam) => ({ firstButton: index === 0, lastButton: index === childrenParam.length - 1, - firstButtonClass: classes.firstButton, - lastButtonClass: classes.lastButton, - middleButtonClass: classes.middleButton, }); return ( diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts index 0a4ff727743894..1eaa8b0c363741 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts +++ b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts @@ -3,9 +3,6 @@ import * as React from 'react'; interface IButtonGroupButtonContext { firstButton?: boolean; lastButton?: boolean; - firstButtonClass?: string; - lastButtonClass?: string; - middleButtonClass?: string; } /** diff --git a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts index 081d6cd1521870..55dec11fdfe720 100644 --- a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts +++ b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts @@ -16,8 +16,6 @@ export interface ButtonGroupClasses { disabled: string; /** Styles applied to the root element if `fullWidth={true}`. */ fullWidth: string; - /** Styles applied to the button child if it is the first button. */ - firstButton: string; /** Styles applied to the root element if `orientation="vertical"`. */ vertical: string; /** Styles applied to the children. */ @@ -56,10 +54,6 @@ export interface ButtonGroupClasses { groupedContainedPrimary: string; /** Styles applied to the children if `variant="contained"` and `color="secondary"`. */ groupedContainedSecondary: string; - /** Styles applied to the button child if it is the last button. */ - lastButton: string; - /** Style applied to the button child if it is in the middle buttons. */ - middleButton: string; } export type ButtonGroupClassKey = keyof ButtonGroupClasses; @@ -76,7 +70,6 @@ const buttonGroupClasses: ButtonGroupClasses = generateUtilityClasses('MuiButton 'disableElevation', 'disabled', 'fullWidth', - 'firstButton', 'vertical', 'grouped', 'groupedHorizontal', @@ -96,8 +89,6 @@ const buttonGroupClasses: ButtonGroupClasses = generateUtilityClasses('MuiButton 'groupedContainedVertical', 'groupedContainedPrimary', 'groupedContainedSecondary', - 'lastButton', - 'middleButton', ]); export default buttonGroupClasses; From 34f37ff5f45a23162edf3f40ed3f8ca084fc9d38 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Sat, 19 Aug 2023 12:05:42 +0530 Subject: [PATCH 07/19] handle case for only a single button and if not within a button group context --- packages/mui-material/src/Button/Button.js | 12 +++++++++--- .../mui-material/src/ButtonGroup/ButtonGroup.js | 14 ++++++++++---- .../src/ButtonGroup/ButtonGroupButtonContext.ts | 5 ++++- .../fixtures/ButtonGroup/DifferentChildren.js | 7 ++++++- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index eb713d00a262f5..0fb8940d8a52bb 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -303,6 +303,9 @@ const addPositionDataAttr = (buttonGroupButtonContext) => { if (buttonGroupButtonContext.lastButton) { return { 'data-last-child': '' }; } + if (buttonGroupButtonContext.onlyChild) { + return {}; + } return { 'data-middle-child': '' }; }; @@ -357,7 +360,11 @@ const Button = React.forwardRef(function Button(inProps, ref) { ); - const dataAttrs = addPositionDataAttr(buttonGroupButtonContext); + let extendedOtherProp = other; + if (buttonGroupButtonContext) { + const dataAttrs = addPositionDataAttr(buttonGroupButtonContext); + extendedOtherProp = { ...dataAttrs, ...other }; + } return ( {startIcon} diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.js index 886f09ad7ab57e..1bd45989ddf743 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.js @@ -244,10 +244,16 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { ], ); - const buttonGroupPositionContext = (index, childrenParam) => ({ - firstButton: index === 0, - lastButton: index === childrenParam.length - 1, - }); + const buttonGroupPositionContext = (index, childrenParam) => { + const childrenCount = React.Children.count(childrenParam); + const onlyChild = childrenCount === 1; + + return { + firstButton: !onlyChild && index === 0, + lastButton: !onlyChild && index === childrenCount - 1, + onlyChild, + }; + }; return ( ({}); +const ButtonGroupButtonContext = React.createContext( + undefined, +); if (process.env.NODE_ENV !== 'production') { ButtonGroupButtonContext.displayName = 'ButtonGroupButtonContext'; diff --git a/test/regressions/fixtures/ButtonGroup/DifferentChildren.js b/test/regressions/fixtures/ButtonGroup/DifferentChildren.js index 618a30ae4d2fa5..dea599fcfac406 100644 --- a/test/regressions/fixtures/ButtonGroup/DifferentChildren.js +++ b/test/regressions/fixtures/ButtonGroup/DifferentChildren.js @@ -8,7 +8,7 @@ export default function DifferentChildren() { return ( {/* It has one button with href which is rendered as anchor tag */} - + @@ -30,6 +30,11 @@ export default function DifferentChildren() { + + {/* One button */} + + + ); } From 8a1c41671d03ae4bfd295066bce77c6198ef79d3 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Mon, 28 Aug 2023 12:48:03 +0530 Subject: [PATCH 08/19] reorganize Button group styles --- .../src/ButtonGroup/ButtonGroup.js | 176 +++++++++--------- 1 file changed, 88 insertions(+), 88 deletions(-) diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.js index 1bd45989ddf743..ee5a0de34e2732 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.js @@ -82,106 +82,106 @@ const ButtonGroupRoot = styled('div', { }), [`& .${buttonGroupClasses.grouped}`]: { minWidth: 40, - '&:hover': { - ...(ownerState.variant === 'contained' && { - boxShadow: 'none', - }), - }, - ...(ownerState.variant === 'contained' && { - boxShadow: 'none', - }), - }, - '& [data-first-child],[data-middle-child]': { - ...(ownerState.orientation === 'horizontal' && { - borderTopRightRadius: 0, - borderBottomRightRadius: 0, - }), - ...(ownerState.orientation === 'vertical' && { - borderBottomRightRadius: 0, - borderBottomLeftRadius: 0, - }), - ...(ownerState.variant === 'text' && - ownerState.orientation === 'horizontal' && { - borderRight: theme.vars - ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` - : `1px solid ${ - theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' - }`, - [`&.${buttonGroupClasses.disabled}`]: { - borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, + '&[data-last-child],&[data-middle-child]': { + ...(ownerState.orientation === 'horizontal' && { + borderTopLeftRadius: 0, + borderBottomLeftRadius: 0, }), - ...(ownerState.variant === 'text' && - ownerState.orientation === 'vertical' && { - borderBottom: theme.vars - ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` - : `1px solid ${ - theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' - }`, - [`&.${buttonGroupClasses.disabled}`]: { - borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, + ...(ownerState.orientation === 'vertical' && { + borderTopRightRadius: 0, + borderTopLeftRadius: 0, }), - ...(ownerState.variant === 'text' && - ownerState.color !== 'inherit' && { - borderColor: theme.vars - ? `rgba(${theme.vars.palette[ownerState.color].mainChannel} / 0.5)` - : alpha(theme.palette[ownerState.color].main, 0.5), - }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'horizontal' && { - borderRightColor: 'transparent', - }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'vertical' && { - borderBottomColor: 'transparent', - }), - ...(ownerState.variant === 'contained' && - ownerState.orientation === 'horizontal' && { - borderRight: `1px solid ${(theme.vars || theme).palette.grey[400]}`, - [`&.${buttonGroupClasses.disabled}`]: { - borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, - }), - ...(ownerState.variant === 'contained' && - ownerState.orientation === 'vertical' && { - borderBottom: `1px solid ${(theme.vars || theme).palette.grey[400]}`, - [`&.${buttonGroupClasses.disabled}`]: { - borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'horizontal' && { + marginLeft: -1, + }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'vertical' && { + marginTop: -1, + }), + }, + '&[data-first-child],&[data-middle-child]': { + ...(ownerState.orientation === 'horizontal' && { + borderTopRightRadius: 0, + borderBottomRightRadius: 0, }), - ...(ownerState.variant === 'contained' && - ownerState.color !== 'inherit' && { - borderColor: (theme.vars || theme).palette[ownerState.color].dark, + ...(ownerState.orientation === 'vertical' && { + borderBottomRightRadius: 0, + borderBottomLeftRadius: 0, }), - '&:hover': { + ...(ownerState.variant === 'text' && + ownerState.orientation === 'horizontal' && { + borderRight: theme.vars + ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` + : `1px solid ${ + theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' + }`, + [`&.${buttonGroupClasses.disabled}`]: { + borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, + }), + ...(ownerState.variant === 'text' && + ownerState.orientation === 'vertical' && { + borderBottom: theme.vars + ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` + : `1px solid ${ + theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' + }`, + [`&.${buttonGroupClasses.disabled}`]: { + borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, + }), + ...(ownerState.variant === 'text' && + ownerState.color !== 'inherit' && { + borderColor: theme.vars + ? `rgba(${theme.vars.palette[ownerState.color].mainChannel} / 0.5)` + : alpha(theme.palette[ownerState.color].main, 0.5), + }), ...(ownerState.variant === 'outlined' && ownerState.orientation === 'horizontal' && { - borderRightColor: 'currentColor', + borderRightColor: 'transparent', }), ...(ownerState.variant === 'outlined' && ownerState.orientation === 'vertical' && { - borderBottomColor: 'currentColor', + borderBottomColor: 'transparent', + }), + ...(ownerState.variant === 'contained' && + ownerState.orientation === 'horizontal' && { + borderRight: `1px solid ${(theme.vars || theme).palette.grey[400]}`, + [`&.${buttonGroupClasses.disabled}`]: { + borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, + }), + ...(ownerState.variant === 'contained' && + ownerState.orientation === 'vertical' && { + borderBottom: `1px solid ${(theme.vars || theme).palette.grey[400]}`, + [`&.${buttonGroupClasses.disabled}`]: { + borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, + }), + ...(ownerState.variant === 'contained' && + ownerState.color !== 'inherit' && { + borderColor: (theme.vars || theme).palette[ownerState.color].dark, }), + '&:hover': { + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'horizontal' && { + borderRightColor: 'currentColor', + }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'vertical' && { + borderBottomColor: 'currentColor', + }), + }, }, - }, - '& [data-last-child],[data-middle-child]': { - ...(ownerState.orientation === 'horizontal' && { - borderTopLeftRadius: 0, - borderBottomLeftRadius: 0, - }), - ...(ownerState.orientation === 'vertical' && { - borderTopRightRadius: 0, - borderTopLeftRadius: 0, - }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'horizontal' && { - marginLeft: -1, - }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'vertical' && { - marginTop: -1, + '&:hover': { + ...(ownerState.variant === 'contained' && { + boxShadow: 'none', }), + }, + ...(ownerState.variant === 'contained' && { + boxShadow: 'none', + }), }, })); From 5c7de191c479a9acd709c4a1db9294ce644f265c Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Tue, 29 Aug 2023 12:06:11 +0530 Subject: [PATCH 09/19] rename interface --- .../mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts index ba8bb4ea09d6ed..784182a41f5802 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts +++ b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts @@ -1,6 +1,6 @@ import * as React from 'react'; -interface IButtonGroupButtonContext { +interface ButtonGroupButtonContextType { firstButton?: boolean; lastButton?: boolean; onlyChild?: boolean; @@ -9,7 +9,7 @@ interface IButtonGroupButtonContext { /** * @ignore - internal component. */ -const ButtonGroupButtonContext = React.createContext( +const ButtonGroupButtonContext = React.createContext( undefined, ); From 5e38727b07eb9888546593466783daf88873eb53 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Tue, 29 Aug 2023 12:07:08 +0530 Subject: [PATCH 10/19] add data attributes directly --- packages/mui-material/src/Button/Button.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index 0fb8940d8a52bb..1935b2e8cb5d77 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -360,10 +360,9 @@ const Button = React.forwardRef(function Button(inProps, ref) { ); - let extendedOtherProp = other; + let dataAttrs = {}; if (buttonGroupButtonContext) { - const dataAttrs = addPositionDataAttr(buttonGroupButtonContext); - extendedOtherProp = { ...dataAttrs, ...other }; + dataAttrs = addPositionDataAttr(buttonGroupButtonContext); } return ( @@ -376,7 +375,8 @@ const Button = React.forwardRef(function Button(inProps, ref) { focusVisibleClassName={clsx(classes.focusVisible, focusVisibleClassName)} ref={ref} type={type} - {...extendedOtherProp} + {...dataAttrs} + {...other} classes={classes} > {startIcon} From 26650cf1174348dd3a7911be738c6506535f71b8 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Tue, 29 Aug 2023 12:10:26 +0530 Subject: [PATCH 11/19] remove redundant onlyChild --- packages/mui-material/src/Button/Button.js | 6 +++--- .../mui-material/src/ButtonGroup/ButtonGroup.js | 14 ++++---------- .../src/ButtonGroup/ButtonGroupButtonContext.ts | 1 - 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index 1935b2e8cb5d77..a7eb58ee41ccb2 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -297,15 +297,15 @@ const ButtonEndIcon = styled('span', { })); const addPositionDataAttr = (buttonGroupButtonContext) => { + if (buttonGroupButtonContext.firstButton && buttonGroupButtonContext.lastButton) { + return {}; + } if (buttonGroupButtonContext.firstButton) { return { 'data-first-child': '' }; } if (buttonGroupButtonContext.lastButton) { return { 'data-last-child': '' }; } - if (buttonGroupButtonContext.onlyChild) { - return {}; - } return { 'data-middle-child': '' }; }; diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.js index ee5a0de34e2732..de4c7f0c5a1d1f 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.js @@ -244,16 +244,10 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { ], ); - const buttonGroupPositionContext = (index, childrenParam) => { - const childrenCount = React.Children.count(childrenParam); - const onlyChild = childrenCount === 1; - - return { - firstButton: !onlyChild && index === 0, - lastButton: !onlyChild && index === childrenCount - 1, - onlyChild, - }; - }; + const buttonGroupPositionContext = (index, childrenParam) => ({ + firstButton: index === 0, + lastButton: index === React.Children.count(childrenParam) - 1, + }); return ( Date: Tue, 29 Aug 2023 12:11:42 +0530 Subject: [PATCH 12/19] rename one to single --- test/regressions/fixtures/ButtonGroup/DifferentChildren.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/regressions/fixtures/ButtonGroup/DifferentChildren.js b/test/regressions/fixtures/ButtonGroup/DifferentChildren.js index dea599fcfac406..44f107dcc0badd 100644 --- a/test/regressions/fixtures/ButtonGroup/DifferentChildren.js +++ b/test/regressions/fixtures/ButtonGroup/DifferentChildren.js @@ -31,9 +31,9 @@ export default function DifferentChildren() { - {/* One button */} + {/* Single button */} - + ); From dc7f138890fcade6580967288077c831e1b6aa4e Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Tue, 29 Aug 2023 14:42:26 +0530 Subject: [PATCH 13/19] rename variables --- packages/mui-material/src/Button/Button.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index a7eb58ee41ccb2..289847022aa757 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -296,7 +296,7 @@ const ButtonEndIcon = styled('span', { ...commonIconStyles(ownerState), })); -const addPositionDataAttr = (buttonGroupButtonContext) => { +const addPositionDataAttributes = (buttonGroupButtonContext) => { if (buttonGroupButtonContext.firstButton && buttonGroupButtonContext.lastButton) { return {}; } @@ -360,9 +360,9 @@ const Button = React.forwardRef(function Button(inProps, ref) { ); - let dataAttrs = {}; + let dataAttributes = {}; if (buttonGroupButtonContext) { - dataAttrs = addPositionDataAttr(buttonGroupButtonContext); + dataAttributes = addPositionDataAttributes(buttonGroupButtonContext); } return ( @@ -375,7 +375,7 @@ const Button = React.forwardRef(function Button(inProps, ref) { focusVisibleClassName={clsx(classes.focusVisible, focusVisibleClassName)} ref={ref} type={type} - {...dataAttrs} + {...dataAttributes} {...other} classes={classes} > From 77f651d5d44179e134ce5ddafe5526763f6e00c9 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Thu, 31 Aug 2023 13:17:22 +0530 Subject: [PATCH 14/19] use "is" prefix in first and last button variable names --- packages/mui-material/src/Button/Button.js | 6 +++--- packages/mui-material/src/ButtonGroup/ButtonGroup.js | 4 ++-- .../src/ButtonGroup/ButtonGroupButtonContext.ts | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index 289847022aa757..5c1915b27e34c5 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -297,13 +297,13 @@ const ButtonEndIcon = styled('span', { })); const addPositionDataAttributes = (buttonGroupButtonContext) => { - if (buttonGroupButtonContext.firstButton && buttonGroupButtonContext.lastButton) { + if (buttonGroupButtonContext.isFirstButton && buttonGroupButtonContext.isLastButton) { return {}; } - if (buttonGroupButtonContext.firstButton) { + if (buttonGroupButtonContext.isFirstButton) { return { 'data-first-child': '' }; } - if (buttonGroupButtonContext.lastButton) { + if (buttonGroupButtonContext.isLastButton) { return { 'data-last-child': '' }; } return { 'data-middle-child': '' }; diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.js index de4c7f0c5a1d1f..acb36d5a84ecd7 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.js @@ -245,8 +245,8 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { ); const buttonGroupPositionContext = (index, childrenParam) => ({ - firstButton: index === 0, - lastButton: index === React.Children.count(childrenParam) - 1, + isFirstButton: index === 0, + isLastButton: index === React.Children.count(childrenParam) - 1, }); return ( diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts index 6c24f43434b586..5d3b96c2da914b 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts +++ b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts @@ -1,8 +1,8 @@ import * as React from 'react'; interface ButtonGroupButtonContextType { - firstButton?: boolean; - lastButton?: boolean; + isFirstButton?: boolean; + isLastButton?: boolean; } /** From 1939e1bcc396f7224a36f5d40aec411a0cb74eb9 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Wed, 6 Sep 2023 12:09:21 +0530 Subject: [PATCH 15/19] use classnames instead of data attributes --- docs/pages/material-ui/api/button-group.json | 5 +- .../api-docs/button-group/button-group.json | 12 ++ packages/mui-material/src/Button/Button.js | 26 +-- .../src/ButtonGroup/ButtonGroup.js | 194 ++++++++++-------- .../ButtonGroup/ButtonGroupButtonContext.ts | 5 +- .../src/ButtonGroup/buttonGroupClasses.ts | 9 + 6 files changed, 146 insertions(+), 105 deletions(-) diff --git a/docs/pages/material-ui/api/button-group.json b/docs/pages/material-ui/api/button-group.json index ef1eb072c05e1b..a89d57e49e857b 100644 --- a/docs/pages/material-ui/api/button-group.json +++ b/docs/pages/material-ui/api/button-group.json @@ -54,6 +54,7 @@ "text", "disableElevation", "disabled", + "firstButton", "fullWidth", "vertical", "grouped", @@ -73,7 +74,9 @@ "groupedContainedHorizontal", "groupedContainedVertical", "groupedContainedPrimary", - "groupedContainedSecondary" + "groupedContainedSecondary", + "lastButton", + "middleButton" ], "globalClasses": { "disabled": "Mui-disabled" }, "name": "MuiButtonGroup" diff --git a/docs/translations/api-docs/button-group/button-group.json b/docs/translations/api-docs/button-group/button-group.json index 4f813acc0ba716..d2479a7bb3dee2 100644 --- a/docs/translations/api-docs/button-group/button-group.json +++ b/docs/translations/api-docs/button-group/button-group.json @@ -56,6 +56,10 @@ "nodeName": "the child elements", "conditions": "disabled={true}" }, + "firstButton": { + "description": "Styles applied to {{nodeName}}.", + "nodeName": "the first button in the button group" + }, "fullWidth": { "description": "Styles applied to {{nodeName}} if {{conditions}}.", "nodeName": "the root element", @@ -151,6 +155,14 @@ "description": "Styles applied to {{nodeName}} if {{conditions}}.", "nodeName": "the children", "conditions": "variant=\"contained\" and color=\"secondary\"" + }, + "lastButton": { + "description": "Styles applied to {{nodeName}}.", + "nodeName": "the last button in the button group" + }, + "middleButton": { + "description": "Styles applied to {{nodeName}}.", + "nodeName": "buttons in the middle of the button group" } } } diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index 5c1915b27e34c5..b287f9e603d545 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -296,17 +296,20 @@ const ButtonEndIcon = styled('span', { ...commonIconStyles(ownerState), })); -const addPositionDataAttributes = (buttonGroupButtonContext) => { - if (buttonGroupButtonContext.isFirstButton && buttonGroupButtonContext.isLastButton) { - return {}; +const addClassNameBasedOnPosition = (buttonGroupButtonContext) => { + if ( + buttonGroupButtonContext.firstButtonClassName && + buttonGroupButtonContext.lastButtonClassName + ) { + return ''; } - if (buttonGroupButtonContext.isFirstButton) { - return { 'data-first-child': '' }; + if (buttonGroupButtonContext.firstButtonClassName) { + return buttonGroupButtonContext.firstButtonClassName; } - if (buttonGroupButtonContext.isLastButton) { - return { 'data-last-child': '' }; + if (buttonGroupButtonContext.lastButtonClassName) { + return buttonGroupButtonContext.lastButtonClassName; } - return { 'data-middle-child': '' }; + return buttonGroupButtonContext.middleButtonClassName; }; const Button = React.forwardRef(function Button(inProps, ref) { @@ -360,22 +363,21 @@ const Button = React.forwardRef(function Button(inProps, ref) { ); - let dataAttributes = {}; + let positionClassName = ''; if (buttonGroupButtonContext) { - dataAttributes = addPositionDataAttributes(buttonGroupButtonContext); + positionClassName = addClassNameBasedOnPosition(buttonGroupButtonContext); } return ( diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.js index acb36d5a84ecd7..3046786bd3cb22 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.js @@ -28,6 +28,15 @@ const overridesResolver = (props, styles) => { [`& .${buttonGroupClasses.grouped}`]: styles[`grouped${capitalize(ownerState.variant)}${capitalize(ownerState.color)}`], }, + { + [`& .${buttonGroupClasses.firstButton}`]: styles.firstButton, + }, + { + [`& .${buttonGroupClasses.lastButton}`]: styles.lastButton, + }, + { + [`& .${buttonGroupClasses.middleButton}`]: styles.middleButton, + }, styles.root, styles[ownerState.variant], ownerState.disableElevation === true && styles.disableElevation, @@ -56,6 +65,9 @@ const useUtilityClasses = (ownerState) => { `grouped${capitalize(variant)}${capitalize(color)}`, disabled && 'disabled', ], + firstButton: ['firstButton'], + lastButton: ['lastButton'], + middleButton: ['middleButton'], }; return composeClasses(slots, getButtonGroupUtilityClass, classes); @@ -82,106 +94,106 @@ const ButtonGroupRoot = styled('div', { }), [`& .${buttonGroupClasses.grouped}`]: { minWidth: 40, - '&[data-last-child],&[data-middle-child]': { - ...(ownerState.orientation === 'horizontal' && { - borderTopLeftRadius: 0, - borderBottomLeftRadius: 0, - }), - ...(ownerState.orientation === 'vertical' && { - borderTopRightRadius: 0, - borderTopLeftRadius: 0, + '&:hover': { + ...(ownerState.variant === 'contained' && { + boxShadow: 'none', }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'horizontal' && { - marginLeft: -1, - }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'vertical' && { - marginTop: -1, - }), }, - '&[data-first-child],&[data-middle-child]': { - ...(ownerState.orientation === 'horizontal' && { - borderTopRightRadius: 0, - borderBottomRightRadius: 0, + ...(ownerState.variant === 'contained' && { + boxShadow: 'none', + }), + }, + [`& .${buttonGroupClasses.firstButton},& .${buttonGroupClasses.middleButton}`]: { + ...(ownerState.orientation === 'horizontal' && { + borderTopRightRadius: 0, + borderBottomRightRadius: 0, + }), + ...(ownerState.orientation === 'vertical' && { + borderBottomRightRadius: 0, + borderBottomLeftRadius: 0, + }), + ...(ownerState.variant === 'text' && + ownerState.orientation === 'horizontal' && { + borderRight: theme.vars + ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` + : `1px solid ${ + theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' + }`, + [`&.${buttonGroupClasses.disabled}`]: { + borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, }), - ...(ownerState.orientation === 'vertical' && { - borderBottomRightRadius: 0, - borderBottomLeftRadius: 0, + ...(ownerState.variant === 'text' && + ownerState.orientation === 'vertical' && { + borderBottom: theme.vars + ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` + : `1px solid ${ + theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' + }`, + [`&.${buttonGroupClasses.disabled}`]: { + borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, }), - ...(ownerState.variant === 'text' && - ownerState.orientation === 'horizontal' && { - borderRight: theme.vars - ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` - : `1px solid ${ - theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' - }`, - [`&.${buttonGroupClasses.disabled}`]: { - borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, - }), - ...(ownerState.variant === 'text' && - ownerState.orientation === 'vertical' && { - borderBottom: theme.vars - ? `1px solid rgba(${theme.vars.palette.common.onBackgroundChannel} / 0.23)` - : `1px solid ${ - theme.palette.mode === 'light' ? 'rgba(0, 0, 0, 0.23)' : 'rgba(255, 255, 255, 0.23)' - }`, - [`&.${buttonGroupClasses.disabled}`]: { - borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, - }), - ...(ownerState.variant === 'text' && - ownerState.color !== 'inherit' && { - borderColor: theme.vars - ? `rgba(${theme.vars.palette[ownerState.color].mainChannel} / 0.5)` - : alpha(theme.palette[ownerState.color].main, 0.5), - }), + ...(ownerState.variant === 'text' && + ownerState.color !== 'inherit' && { + borderColor: theme.vars + ? `rgba(${theme.vars.palette[ownerState.color].mainChannel} / 0.5)` + : alpha(theme.palette[ownerState.color].main, 0.5), + }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'horizontal' && { + borderRightColor: 'transparent', + }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'vertical' && { + borderBottomColor: 'transparent', + }), + ...(ownerState.variant === 'contained' && + ownerState.orientation === 'horizontal' && { + borderRight: `1px solid ${(theme.vars || theme).palette.grey[400]}`, + [`&.${buttonGroupClasses.disabled}`]: { + borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, + }), + ...(ownerState.variant === 'contained' && + ownerState.orientation === 'vertical' && { + borderBottom: `1px solid ${(theme.vars || theme).palette.grey[400]}`, + [`&.${buttonGroupClasses.disabled}`]: { + borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, + }, + }), + ...(ownerState.variant === 'contained' && + ownerState.color !== 'inherit' && { + borderColor: (theme.vars || theme).palette[ownerState.color].dark, + }), + '&:hover': { ...(ownerState.variant === 'outlined' && ownerState.orientation === 'horizontal' && { - borderRightColor: 'transparent', + borderRightColor: 'currentColor', }), ...(ownerState.variant === 'outlined' && ownerState.orientation === 'vertical' && { - borderBottomColor: 'transparent', - }), - ...(ownerState.variant === 'contained' && - ownerState.orientation === 'horizontal' && { - borderRight: `1px solid ${(theme.vars || theme).palette.grey[400]}`, - [`&.${buttonGroupClasses.disabled}`]: { - borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, - }), - ...(ownerState.variant === 'contained' && - ownerState.orientation === 'vertical' && { - borderBottom: `1px solid ${(theme.vars || theme).palette.grey[400]}`, - [`&.${buttonGroupClasses.disabled}`]: { - borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, - }, + borderBottomColor: 'currentColor', }), - ...(ownerState.variant === 'contained' && - ownerState.color !== 'inherit' && { - borderColor: (theme.vars || theme).palette[ownerState.color].dark, - }), - '&:hover': { - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'horizontal' && { - borderRightColor: 'currentColor', - }), - ...(ownerState.variant === 'outlined' && - ownerState.orientation === 'vertical' && { - borderBottomColor: 'currentColor', - }), - }, - }, - '&:hover': { - ...(ownerState.variant === 'contained' && { - boxShadow: 'none', - }), }, - ...(ownerState.variant === 'contained' && { - boxShadow: 'none', + }, + [`& .${buttonGroupClasses.lastButton},& .${buttonGroupClasses.middleButton}`]: { + ...(ownerState.orientation === 'horizontal' && { + borderTopLeftRadius: 0, + borderBottomLeftRadius: 0, }), + ...(ownerState.orientation === 'vertical' && { + borderTopRightRadius: 0, + borderTopLeftRadius: 0, + }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'horizontal' && { + marginLeft: -1, + }), + ...(ownerState.variant === 'outlined' && + ownerState.orientation === 'vertical' && { + marginTop: -1, + }), }, })); @@ -245,8 +257,10 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { ); const buttonGroupPositionContext = (index, childrenParam) => ({ - isFirstButton: index === 0, - isLastButton: index === React.Children.count(childrenParam) - 1, + firstButtonClassName: index === 0 ? classes.firstButton : '', + lastButtonClassName: + index === React.Children.count(childrenParam) - 1 ? classes.lastButton : '', + middleButtonClassName: classes.middleButton, }); return ( diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts index 5d3b96c2da914b..dc5809a7c16cb5 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts +++ b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts @@ -1,8 +1,9 @@ import * as React from 'react'; interface ButtonGroupButtonContextType { - isFirstButton?: boolean; - isLastButton?: boolean; + firstButtonClassName?: string; + lastButtonClassName?: string; + middleButtonClassName?: string; } /** diff --git a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts index 55dec11fdfe720..433083e3ae20f9 100644 --- a/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts +++ b/packages/mui-material/src/ButtonGroup/buttonGroupClasses.ts @@ -14,6 +14,8 @@ export interface ButtonGroupClasses { disableElevation: string; /** State class applied to the child elements if `disabled={true}`. */ disabled: string; + /** Styles applied to the first button in the button group. */ + firstButton: string; /** Styles applied to the root element if `fullWidth={true}`. */ fullWidth: string; /** Styles applied to the root element if `orientation="vertical"`. */ @@ -54,6 +56,10 @@ export interface ButtonGroupClasses { groupedContainedPrimary: string; /** Styles applied to the children if `variant="contained"` and `color="secondary"`. */ groupedContainedSecondary: string; + /** Styles applied to the last button in the button group. */ + lastButton: string; + /** Styles applied to buttons in the middle of the button group. */ + middleButton: string; } export type ButtonGroupClassKey = keyof ButtonGroupClasses; @@ -69,6 +75,7 @@ const buttonGroupClasses: ButtonGroupClasses = generateUtilityClasses('MuiButton 'text', 'disableElevation', 'disabled', + 'firstButton', 'fullWidth', 'vertical', 'grouped', @@ -89,6 +96,8 @@ const buttonGroupClasses: ButtonGroupClasses = generateUtilityClasses('MuiButton 'groupedContainedVertical', 'groupedContainedPrimary', 'groupedContainedSecondary', + 'lastButton', + 'middleButton', ]); export default buttonGroupClasses; From 30b5ba8eb8b4f48bada3c62913579189dff665db Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Wed, 6 Sep 2023 12:40:23 +0530 Subject: [PATCH 16/19] add unit tests --- .../src/ButtonGroup/ButtonGroup.test.js | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.test.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.test.js index 283625bd5ef426..e79439f7b13cba 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.test.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.test.js @@ -209,4 +209,46 @@ describe('', () => { expect(screen.getByRole('button')).to.have.class(buttonClasses.outlinedSecondary); }); }); + + describe('position classes', () => { + it('correctly applies position classes to buttons', () => { + render( + + + + + , + ); + + const firstButton = screen.getAllByRole('button')[0]; + const middleButton = screen.getAllByRole('button')[1]; + const lastButton = screen.getAllByRole('button')[2]; + + expect(firstButton).to.have.class(classes.firstButton); + expect(firstButton).not.to.have.class(classes.middleButton); + expect(firstButton).not.to.have.class(classes.lastButton); + + expect(middleButton).to.have.class(classes.middleButton); + expect(middleButton).not.to.have.class(classes.firstButton); + expect(middleButton).not.to.have.class(classes.lastButton); + + expect(lastButton).to.have.class(classes.lastButton); + expect(lastButton).not.to.have.class(classes.middleButton); + expect(lastButton).not.to.have.class(classes.firstButton); + }); + + it('does not apply any position classes to a single button', () => { + render( + + + , + ); + + const button = screen.getByRole('button'); + + expect(button).not.to.have.class(classes.firstButton); + expect(button).not.to.have.class(classes.middleButton); + expect(button).not.to.have.class(classes.lastButton); + }); + }); }); From 69b97f6d9f2a370c5691f0d33c9c26b70271c9e6 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Wed, 6 Sep 2023 18:46:28 +0530 Subject: [PATCH 17/19] move button position classes logic to button group component --- packages/mui-material/src/Button/Button.js | 23 ++--------------- .../src/ButtonGroup/ButtonGroup.js | 25 +++++++++++++------ .../ButtonGroup/ButtonGroupButtonContext.ts | 12 +++------ 3 files changed, 24 insertions(+), 36 deletions(-) diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js index b287f9e603d545..ad9fb46f49afc8 100644 --- a/packages/mui-material/src/Button/Button.js +++ b/packages/mui-material/src/Button/Button.js @@ -296,26 +296,10 @@ const ButtonEndIcon = styled('span', { ...commonIconStyles(ownerState), })); -const addClassNameBasedOnPosition = (buttonGroupButtonContext) => { - if ( - buttonGroupButtonContext.firstButtonClassName && - buttonGroupButtonContext.lastButtonClassName - ) { - return ''; - } - if (buttonGroupButtonContext.firstButtonClassName) { - return buttonGroupButtonContext.firstButtonClassName; - } - if (buttonGroupButtonContext.lastButtonClassName) { - return buttonGroupButtonContext.lastButtonClassName; - } - return buttonGroupButtonContext.middleButtonClassName; -}; - const Button = React.forwardRef(function Button(inProps, ref) { // props priority: `inProps` > `contextProps` > `themeDefaultProps` const contextProps = React.useContext(ButtonGroupContext); - const buttonGroupButtonContext = React.useContext(ButtonGroupButtonContext); + const buttonGroupButtonContextPositionClassName = React.useContext(ButtonGroupButtonContext); const resolvedProps = resolveProps(contextProps, inProps); const props = useThemeProps({ props: resolvedProps, name: 'MuiButton' }); const { @@ -363,10 +347,7 @@ const Button = React.forwardRef(function Button(inProps, ref) { ); - let positionClassName = ''; - if (buttonGroupButtonContext) { - positionClassName = addClassNameBasedOnPosition(buttonGroupButtonContext); - } + const positionClassName = buttonGroupButtonContextPositionClassName || ''; return ( ({ - firstButtonClassName: index === 0 ? classes.firstButton : '', - lastButtonClassName: - index === React.Children.count(childrenParam) - 1 ? classes.lastButton : '', - middleButtonClassName: classes.middleButton, - }); + const addButtonGroupButtonContextPositionClassName = (index, childrenParam) => { + const isFirstButton = index === 0; + const isLastButton = index === React.Children.count(childrenParam) - 1; + + if (isFirstButton && isLastButton) { + return ''; + } + if (isFirstButton) { + return classes.firstButton; + } + if (isLastButton) { + return classes.lastButton; + } + return classes.middleButton; + }; return ( + {child} ); diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts index dc5809a7c16cb5..18f4c4c7e13407 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts +++ b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts @@ -1,17 +1,13 @@ import * as React from 'react'; -interface ButtonGroupButtonContextType { - firstButtonClassName?: string; - lastButtonClassName?: string; - middleButtonClassName?: string; -} +type ButtonGroupButtonContextClassNameValue = string; /** * @ignore - internal component. */ -const ButtonGroupButtonContext = React.createContext( - undefined, -); +const ButtonGroupButtonContext = React.createContext< + ButtonGroupButtonContextClassNameValue | undefined +>(undefined); if (process.env.NODE_ENV !== 'production') { ButtonGroupButtonContext.displayName = 'ButtonGroupButtonContext'; From 9f3dd29f2b69445a1d438d42d916821af1be2821 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Wed, 6 Sep 2023 19:18:49 +0530 Subject: [PATCH 18/19] rename method name and type --- packages/mui-material/src/ButtonGroup/ButtonGroup.js | 4 ++-- .../src/ButtonGroup/ButtonGroupButtonContext.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.js index 4e63d0accda3a2..bed6b946a25d5e 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.js @@ -256,7 +256,7 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { ], ); - const addButtonGroupButtonContextPositionClassName = (index, childrenParam) => { + const getButtonPositionClassName = (index, childrenParam) => { const isFirstButton = index === 0; const isLastButton = index === React.Children.count(childrenParam) - 1; @@ -289,7 +289,7 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { return ( {child} diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts index 18f4c4c7e13407..8a93fe171954ef 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts +++ b/packages/mui-material/src/ButtonGroup/ButtonGroupButtonContext.ts @@ -1,13 +1,13 @@ import * as React from 'react'; -type ButtonGroupButtonContextClassNameValue = string; +type ButtonPositionClassName = string; /** * @ignore - internal component. */ -const ButtonGroupButtonContext = React.createContext< - ButtonGroupButtonContextClassNameValue | undefined ->(undefined); +const ButtonGroupButtonContext = React.createContext( + undefined, +); if (process.env.NODE_ENV !== 'production') { ButtonGroupButtonContext.displayName = 'ButtonGroupButtonContext'; From e76692f6a148a6315721a5a6274252921205581c Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Wed, 6 Sep 2023 19:23:01 +0530 Subject: [PATCH 19/19] prettier --- packages/mui-material/src/ButtonGroup/ButtonGroup.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/mui-material/src/ButtonGroup/ButtonGroup.js b/packages/mui-material/src/ButtonGroup/ButtonGroup.js index bed6b946a25d5e..c9d548175044aa 100644 --- a/packages/mui-material/src/ButtonGroup/ButtonGroup.js +++ b/packages/mui-material/src/ButtonGroup/ButtonGroup.js @@ -288,9 +288,7 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(inProps, ref) { } return ( - + {child} );