From 6b9771d93088b90c6c5a3cdb4d3c47804090889d Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Fri, 6 Jan 2023 15:35:56 -0800 Subject: [PATCH 1/9] chore: Simplify Checkbox style selectors using component checked/disabled state --- ...-fac35afd-0d24-4b2b-ac4d-3224f4437e63.json | 7 + .../src/components/Checkbox/Checkbox.types.ts | 2 +- .../src/components/Checkbox/useCheckbox.tsx | 5 +- .../components/Checkbox/useCheckboxStyles.ts | 149 +++++++++--------- 4 files changed, 82 insertions(+), 81 deletions(-) create mode 100644 change/@fluentui-react-checkbox-fac35afd-0d24-4b2b-ac4d-3224f4437e63.json diff --git a/change/@fluentui-react-checkbox-fac35afd-0d24-4b2b-ac4d-3224f4437e63.json b/change/@fluentui-react-checkbox-fac35afd-0d24-4b2b-ac4d-3224f4437e63.json new file mode 100644 index 0000000000000..d774e09e11412 --- /dev/null +++ b/change/@fluentui-react-checkbox-fac35afd-0d24-4b2b-ac4d-3224f4437e63.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "chore: Simplify Checkbox style selectors using component checked/disabled state", + "packageName": "@fluentui/react-checkbox", + "email": "behowell@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-checkbox/src/components/Checkbox/Checkbox.types.ts b/packages/react-components/react-checkbox/src/components/Checkbox/Checkbox.types.ts index 02a6f857b014c..acb4001a98624 100644 --- a/packages/react-components/react-checkbox/src/components/Checkbox/Checkbox.types.ts +++ b/packages/react-components/react-checkbox/src/components/Checkbox/Checkbox.types.ts @@ -95,4 +95,4 @@ export interface CheckboxOnChangeData { * State used in rendering Checkbox */ export type CheckboxState = ComponentState & - Required>; + Required>; diff --git a/packages/react-components/react-checkbox/src/components/Checkbox/useCheckbox.tsx b/packages/react-components/react-checkbox/src/components/Checkbox/useCheckbox.tsx index 70fed20abde2c..5a0b3a488ddcd 100644 --- a/packages/react-components/react-checkbox/src/components/Checkbox/useCheckbox.tsx +++ b/packages/react-components/react-checkbox/src/components/Checkbox/useCheckbox.tsx @@ -29,7 +29,7 @@ import { useFocusWithin } from '@fluentui/react-tabster'; * @param ref - reference to `` element of Checkbox */ export const useCheckbox_unstable = (props: CheckboxProps, ref: React.Ref): CheckboxState => { - const { disabled, required, shape = 'square', size = 'medium', labelPosition = 'after', onChange } = props; + const { disabled = false, required, shape = 'square', size = 'medium', labelPosition = 'after', onChange } = props; const [checked, setChecked] = useControllableState({ defaultState: props.defaultChecked, @@ -53,13 +53,14 @@ export const useCheckbox_unstable = (props: CheckboxProps, ref: React.Ref : ; } - } else { + } else if (checked) { checkmarkIcon = size === 'large' ? : ; } const state: CheckboxState = { shape, checked, + disabled, size, labelPosition, components: { diff --git a/packages/react-components/react-checkbox/src/components/Checkbox/useCheckboxStyles.ts b/packages/react-components/react-checkbox/src/components/Checkbox/useCheckboxStyles.ts index d0d22c2a609a3..a05048f6f3ed7 100644 --- a/packages/react-components/react-checkbox/src/components/Checkbox/useCheckboxStyles.ts +++ b/packages/react-components/react-checkbox/src/components/Checkbox/useCheckboxStyles.ts @@ -18,130 +18,110 @@ const indicatorSizeLarge = '20px'; const useRootBaseClassName = makeResetStyles({ position: 'relative', display: 'inline-flex', + cursor: 'pointer', + color: tokens.colorNeutralForeground3, ...createFocusOutlineStyle({ style: {}, selector: 'focus-within' }), }); -const useInputBaseClassName = makeResetStyles({ - boxSizing: 'border-box', - cursor: 'pointer', - height: '100%', - margin: 0, - opacity: 0, - position: 'absolute', - top: 0, - // Calculate the width of the hidden input by taking into account the size of the indicator + the padding around it. - // This is done so that clicking on that "empty space" still toggles the checkbox. - width: `calc(${indicatorSizeMedium} + 2 * ${tokens.spacingHorizontalS})`, - - // When unchecked, hide the the checkmark icon (child of the indicator slot) - [`:not(:checked):not(:indeterminate) ~ .${checkboxClassNames.indicator} > *`]: { - opacity: 0, - }, - - // Colors for the unchecked state - ':enabled:not(:checked):not(:indeterminate)': { - [`& ~ .${checkboxClassNames.label}`]: { - color: tokens.colorNeutralForeground3, - }, - [`& ~ .${checkboxClassNames.indicator}`]: { - borderColor: tokens.colorNeutralStrokeAccessible, - }, - +const useRootStyles = makeStyles({ + unchecked: { ':hover': { - [`& ~ .${checkboxClassNames.label}`]: { - color: tokens.colorNeutralForeground2, - }, - [`& ~ .${checkboxClassNames.indicator}`]: { - borderColor: tokens.colorNeutralStrokeAccessibleHover, + color: tokens.colorNeutralForeground2, + [`> .${checkboxClassNames.indicator}`]: { + ...shorthands.borderColor(tokens.colorNeutralStrokeAccessibleHover), }, }, - ':active:hover': { - [`& ~ .${checkboxClassNames.label}`]: { - color: tokens.colorNeutralForeground1, - }, - [`& ~ .${checkboxClassNames.indicator}`]: { - borderColor: tokens.colorNeutralStrokeAccessiblePressed, + ':active': { + color: tokens.colorNeutralForeground1, + [`> .${checkboxClassNames.indicator}`]: { + ...shorthands.borderColor(tokens.colorNeutralStrokeAccessiblePressed), }, }, }, - // Colors for the checked state - ':enabled:checked:not(:indeterminate)': { - [`& ~ .${checkboxClassNames.label}`]: { - color: tokens.colorNeutralForeground1, - }, - [`& ~ .${checkboxClassNames.indicator}`]: { + checked: { + color: tokens.colorNeutralForeground1, + + [`> .${checkboxClassNames.indicator}`]: { backgroundColor: tokens.colorCompoundBrandBackground, color: tokens.colorNeutralForegroundInverted, - borderColor: tokens.colorCompoundBrandBackground, + ...shorthands.borderColor(tokens.colorCompoundBrandBackground), }, ':hover': { - [`& ~ .${checkboxClassNames.indicator}`]: { + [`> .${checkboxClassNames.indicator}`]: { backgroundColor: tokens.colorCompoundBrandBackgroundHover, - borderColor: tokens.colorCompoundBrandBackgroundHover, + ...shorthands.borderColor(tokens.colorCompoundBrandBackgroundHover), }, }, - ':active:hover': { - [`& ~ .${checkboxClassNames.indicator}`]: { + ':active': { + [`> .${checkboxClassNames.indicator}`]: { backgroundColor: tokens.colorCompoundBrandBackgroundPressed, - borderColor: tokens.colorCompoundBrandBackgroundPressed, + ...shorthands.borderColor(tokens.colorCompoundBrandBackgroundPressed), }, }, }, - // Colors for the mixed state - ':enabled:indeterminate': { - [`& ~ .${checkboxClassNames.label}`]: { - color: tokens.colorNeutralForeground1, - }, - [`& ~ .${checkboxClassNames.indicator}`]: { - borderColor: tokens.colorCompoundBrandStroke, + mixed: { + color: tokens.colorNeutralForeground1, + + [`> .${checkboxClassNames.indicator}`]: { + ...shorthands.borderColor(tokens.colorCompoundBrandStroke), color: tokens.colorCompoundBrandForeground1, }, ':hover': { - [`& ~ .${checkboxClassNames.indicator}`]: { - borderColor: tokens.colorCompoundBrandStrokeHover, + [`> .${checkboxClassNames.indicator}`]: { + ...shorthands.borderColor(tokens.colorCompoundBrandStrokeHover), color: tokens.colorCompoundBrandForeground1Hover, }, }, - ':active:hover': { - [`& ~ .${checkboxClassNames.indicator}`]: { - borderColor: tokens.colorCompoundBrandStrokePressed, + ':active': { + [`> .${checkboxClassNames.indicator}`]: { + ...shorthands.borderColor(tokens.colorCompoundBrandStrokePressed), color: tokens.colorCompoundBrandForeground1Pressed, }, }, }, - ':disabled': { + disabled: { cursor: 'default', - [`& ~ .${checkboxClassNames.label}`]: { - cursor: 'default', + color: tokens.colorNeutralForegroundDisabled, + + [`> .${checkboxClassNames.indicator}`]: { + ...shorthands.borderColor(tokens.colorNeutralStrokeDisabled), color: tokens.colorNeutralForegroundDisabled, - '@media (forced-colors: active)': { - color: 'GrayText', - }, }, - [`& ~ .${checkboxClassNames.indicator}`]: { - borderColor: tokens.colorNeutralStrokeDisabled, - color: tokens.colorNeutralForegroundDisabled, - '@media (forced-colors: active)': { + + '@media (forced-colors: active)': { + color: 'GrayText', + [`> .${checkboxClassNames.indicator}`]: { color: 'GrayText', }, - }, - [`& ~ .${checkboxClassNames.indicator} svg`]: { - '@media (forced-colors: active)': { + [`> .${checkboxClassNames.indicator} svg`]: { color: 'GrayText', }, }, }, }); +const useInputBaseClassName = makeResetStyles({ + boxSizing: 'border-box', + cursor: 'inherit', + height: '100%', + margin: 0, + opacity: 0, + position: 'absolute', + top: 0, + // Calculate the width of the hidden input by taking into account the size of the indicator + the padding around it. + // This is done so that clicking on that "empty space" still toggles the checkbox. + width: `calc(${indicatorSizeMedium} + 2 * ${tokens.spacingHorizontalS})`, +}); + const useInputStyles = makeStyles({ before: { right: 0, @@ -165,7 +145,7 @@ const useIndicatorBaseClassName = makeResetStyles({ justifyContent: 'center', overflow: 'hidden', - border: tokens.strokeWidthThin + ' solid', + border: tokens.strokeWidthThin + ' solid ' + tokens.colorNeutralStrokeAccessible, borderRadius: tokens.borderRadiusSmall, margin: tokens.spacingVerticalS + ' ' + tokens.spacingHorizontalS, fill: 'currentColor', @@ -192,7 +172,8 @@ const useIndicatorStyles = makeStyles({ const useLabelStyles = makeStyles({ base: { alignSelf: 'center', - cursor: 'pointer', + color: 'inherit', + cursor: 'inherit', ...shorthands.padding(tokens.spacingVerticalS, tokens.spacingHorizontalS), }, @@ -219,10 +200,22 @@ const useLabelStyles = makeStyles({ * Apply styling to the Checkbox slots based on the state */ export const useCheckboxStyles_unstable = (state: CheckboxState): CheckboxState => { - const { labelPosition, shape, size } = state; + const { checked, disabled, labelPosition, shape, size } = state; const rootBaseClassName = useRootBaseClassName(); - state.root.className = mergeClasses(checkboxClassNames.root, rootBaseClassName, state.root.className); + const rootStyles = useRootStyles(); + state.root.className = mergeClasses( + checkboxClassNames.root, + rootBaseClassName, + disabled + ? rootStyles.disabled + : checked === 'mixed' + ? rootStyles.mixed + : checked + ? rootStyles.checked + : rootStyles.unchecked, + state.root.className, + ); const inputBaseClassName = useInputBaseClassName(); const inputStyles = useInputStyles(); From 5fe8592b6423bd29ab2894bf80de99fea420b40f Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Fri, 6 Jan 2023 15:37:25 -0800 Subject: [PATCH 2/9] Update api.md --- .../react-components/react-checkbox/etc/react-checkbox.api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-components/react-checkbox/etc/react-checkbox.api.md b/packages/react-components/react-checkbox/etc/react-checkbox.api.md index a9f6ea7b129e6..de80a8dea3a2e 100644 --- a/packages/react-components/react-checkbox/etc/react-checkbox.api.md +++ b/packages/react-components/react-checkbox/etc/react-checkbox.api.md @@ -61,7 +61,7 @@ export type CheckboxSlots = { }; // @public -export type CheckboxState = ComponentState & Required>; +export type CheckboxState = ComponentState & Required>; // @public (undocumented) export const renderCheckbox_unstable: (state: CheckboxState) => JSX.Element; From 0988e8f76c1dd717f0cbbcfac3dc2b474253ccb3 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Fri, 6 Jan 2023 15:41:31 -0800 Subject: [PATCH 3/9] Test snapshots --- .../__snapshots__/Checkbox.test.tsx.snap | 34 ++----------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/packages/react-components/react-checkbox/src/components/Checkbox/__snapshots__/Checkbox.test.tsx.snap b/packages/react-components/react-checkbox/src/components/Checkbox/__snapshots__/Checkbox.test.tsx.snap index e553f8f6b3b3a..922705a2db39f 100644 --- a/packages/react-components/react-checkbox/src/components/Checkbox/__snapshots__/Checkbox.test.tsx.snap +++ b/packages/react-components/react-checkbox/src/components/Checkbox/__snapshots__/Checkbox.test.tsx.snap @@ -41,22 +41,7 @@ exports[`Checkbox renders a default state 1`] = ` + /> `; @@ -153,22 +138,7 @@ exports[`Checkbox renders with a label 1`] = ` + />