From bdf794fd6bc4957e5cc1ec198fc8dada686db650 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Fri, 10 Feb 2023 11:37:35 -0800 Subject: [PATCH 1/7] Update Input to use makeResetStyles --- .../src/components/Input/useInputStyles.ts | 207 ++++++++++-------- 1 file changed, 113 insertions(+), 94 deletions(-) diff --git a/packages/react-components/react-input/src/components/Input/useInputStyles.ts b/packages/react-components/react-input/src/components/Input/useInputStyles.ts index f55c51e865c79d..b6027fb316b897 100644 --- a/packages/react-components/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-components/react-input/src/components/Input/useInputStyles.ts @@ -1,7 +1,8 @@ -import { makeStyles, mergeClasses, shorthands } from '@griffel/react'; import { tokens, typographyStyles } from '@fluentui/react-theme'; -import type { InputSlots, InputState } from './Input.types'; import type { SlotClassNames } from '@fluentui/react-utilities'; +import type { GriffelResetStyle } from '@griffel/react'; +import { makeResetStyles, makeStyles, mergeClasses, shorthands } from '@griffel/react'; +import type { InputSlots, InputState } from './Input.types'; export const inputClassNames: SlotClassNames = { root: 'fui-Input', @@ -17,85 +18,92 @@ const fieldHeights = { large: '40px', }; -const useRootStyles = makeStyles({ - base: { - display: 'inline-flex', - alignItems: 'center', - flexWrap: 'nowrap', - ...shorthands.gap(tokens.spacingHorizontalXXS), - fontFamily: tokens.fontFamilyBase, - ...shorthands.borderRadius(tokens.borderRadiusMedium), // used for all but underline - position: 'relative', +const rootBaseStyles: GriffelResetStyle = { + display: 'inline-flex', + alignItems: 'center', + flexWrap: 'nowrap', + gap: tokens.spacingHorizontalXXS, + borderRadius: tokens.borderRadiusMedium, // used for all but underline + position: 'relative', + boxSizing: 'border-box', + + // size: medium (default) + minHeight: fieldHeights.medium, + padding: `0 ${tokens.spacingHorizontalMNudge}`, + ...typographyStyles.body1, +}; + +const rootInteractiveStyles: GriffelResetStyle = { + // This is all for the bottom focus border. + // It's supposed to be 2px flat all the way across and match the radius of the field's corners. + '::after': { boxSizing: 'border-box', - }, - interactive: { - // This is all for the bottom focus border. - // It's supposed to be 2px flat all the way across and match the radius of the field's corners. - '::after': { - boxSizing: 'border-box', - content: '""', - position: 'absolute', - left: '-1px', - bottom: '-1px', - right: '-1px', + content: '""', + position: 'absolute', + left: '-1px', + bottom: '-1px', + right: '-1px', - // Maintaining the correct corner radius: - // Use the whole border-radius as the height and only put radii on the bottom corners. - // (Otherwise the radius would be automatically reduced to fit available space.) - // max() ensures the focus border still shows up even if someone sets tokens.borderRadiusMedium to 0. - height: `max(2px, ${tokens.borderRadiusMedium})`, - borderBottomLeftRadius: tokens.borderRadiusMedium, - borderBottomRightRadius: tokens.borderRadiusMedium, + // Maintaining the correct corner radius: + // Use the whole border-radius as the height and only put radii on the bottom corners. + // (Otherwise the radius would be automatically reduced to fit available space.) + // max() ensures the focus border still shows up even if someone sets tokens.borderRadiusMedium to 0. + height: `max(2px, ${tokens.borderRadiusMedium})`, + borderBottomLeftRadius: tokens.borderRadiusMedium, + borderBottomRightRadius: tokens.borderRadiusMedium, - // Flat 2px border: - // By default borderBottom will cause little "horns" on the ends. The clipPath trims them off. - // (This could be done without trimming using `background: linear-gradient(...)`, but using - // borderBottom makes it easier for people to override the color if needed.) - ...shorthands.borderBottom('2px', 'solid', tokens.colorCompoundBrandStroke), - clipPath: 'inset(calc(100% - 2px) 0 0 0)', + // Flat 2px border: + // By default borderBottom will cause little "horns" on the ends. The clipPath trims them off. + // (This could be done without trimming using `background: linear-gradient(...)`, but using + // borderBottom makes it easier for people to override the color if needed.) + borderBottom: `2px solid ${tokens.colorCompoundBrandStroke}`, + clipPath: 'inset(calc(100% - 2px) 0 0 0)', - // Animation for focus OUT - transform: 'scaleX(0)', - transitionProperty: 'transform', - transitionDuration: tokens.durationUltraFast, - transitionDelay: tokens.curveAccelerateMid, + // Animation for focus OUT + transform: 'scaleX(0)', + transitionProperty: 'transform', + transitionDuration: tokens.durationUltraFast, + transitionDelay: tokens.curveAccelerateMid, - '@media screen and (prefers-reduced-motion: reduce)': { - transitionDuration: '0.01ms', - transitionDelay: '0.01ms', - }, + '@media screen and (prefers-reduced-motion: reduce)': { + transitionDuration: '0.01ms', + transitionDelay: '0.01ms', }, - ':focus-within::after': { - // Animation for focus IN - transform: 'scaleX(1)', - transitionProperty: 'transform', - transitionDuration: tokens.durationNormal, - transitionDelay: tokens.curveDecelerateMid, + }, + ':focus-within::after': { + // Animation for focus IN + transform: 'scaleX(1)', + transitionProperty: 'transform', + transitionDuration: tokens.durationNormal, + transitionDelay: tokens.curveDecelerateMid, - '@media screen and (prefers-reduced-motion: reduce)': { - transitionDuration: '0.01ms', - transitionDelay: '0.01ms', - }, - }, - ':focus-within:active::after': { - // This is if the user clicks the field again while it's already focused - borderBottomColor: tokens.colorCompoundBrandStrokePressed, - }, - ':focus-within': { - outlineWidth: '2px', - outlineStyle: 'solid', - outlineColor: 'transparent', + '@media screen and (prefers-reduced-motion: reduce)': { + transitionDuration: '0.01ms', + transitionDelay: '0.01ms', }, }, + ':focus-within:active::after': { + // This is if the user clicks the field again while it's already focused + borderBottomColor: tokens.colorCompoundBrandStrokePressed, + }, + ':focus-within': { + outlineWidth: '2px', + outlineStyle: 'solid', + outlineColor: 'transparent', + }, +}; + +const useRootNonInteractiveClassName = makeResetStyles(rootBaseStyles); +const useRootInteractiveClassName = makeResetStyles({ ...rootBaseStyles, ...rootInteractiveStyles }); + +const useRootStyles = makeStyles({ small: { minHeight: fieldHeights.small, ...shorthands.padding('0', tokens.spacingHorizontalSNudge), ...typographyStyles.caption1, }, medium: { - minHeight: fieldHeights.medium, - ...shorthands.padding('0', tokens.spacingHorizontalMNudge), - ...typographyStyles.body1, + // included in rootBaseStyles }, large: { minHeight: fieldHeights.large, @@ -173,30 +181,34 @@ const useRootStyles = makeStyles({ }, }); -const useInputElementStyles = makeStyles({ - base: { - boxSizing: 'border-box', - flexGrow: 1, - minWidth: 0, // required to make the input shrink to fit the wrapper - ...shorthands.borderStyle('none'), // input itself never has a border (this is handled by inputWrapper) - ...shorthands.padding('0', tokens.spacingHorizontalXXS), - color: tokens.colorNeutralForeground1, - // Use literal "transparent" (not from the theme) to always let the color from the root show through - backgroundColor: 'transparent', - - '::placeholder': { - color: tokens.colorNeutralForeground4, - opacity: 1, // browser style override - }, +const useInputClassName = makeResetStyles({ + boxSizing: 'border-box', + flexGrow: 1, + minWidth: 0, // required to make the input shrink to fit the wrapper + borderStyle: 'none', // input itself never has a border (this is handled by inputWrapper) + padding: `0 ${tokens.spacingHorizontalXXS}`, + color: tokens.colorNeutralForeground1, + // Use literal "transparent" (not from the theme) to always let the color from the root show through + backgroundColor: 'transparent', - outlineStyle: 'none', // disable default browser outline + '::placeholder': { + color: tokens.colorNeutralForeground4, + opacity: 1, // browser style override }, + + outlineStyle: 'none', // disable default browser outline + + // size: medium (default) + ...typographyStyles.body1, +}); + +const useInputElementStyles = makeStyles({ small: { // This is set on root but doesn't inherit ...typographyStyles.caption1, }, medium: { - ...typographyStyles.body1, + // included in useInputClassName }, large: { ...typographyStyles.body2, @@ -212,13 +224,17 @@ const useInputElementStyles = makeStyles({ }, }); +const useContentClassName = makeResetStyles({ + boxSizing: 'border-box', + color: tokens.colorNeutralForeground3, // "icon color" in design spec + display: 'flex', + + // special case styling for icons (most common case) to ensure they're centered vertically + // size: medium (default) + '> svg': { fontSize: '20px' }, +}); + const useContentStyles = makeStyles({ - base: { - boxSizing: 'border-box', - color: tokens.colorNeutralForeground3, // "icon color" in design spec - display: 'flex', - // special case styling for icons (most common case) to ensure they're centered vertically - }, disabled: { color: tokens.colorNeutralForegroundDisabled, }, @@ -227,7 +243,7 @@ const useContentStyles = makeStyles({ '> svg': { fontSize: '16px' }, }, medium: { - '> svg': { fontSize: '20px' }, + // included in useContentClassName }, large: { '> svg': { fontSize: '24px' }, @@ -243,16 +259,19 @@ export const useInputStyles_unstable = (state: InputState): InputState => { const invalid = `${state.input['aria-invalid']}` === 'true'; const filled = appearance.startsWith('filled'); + // Call exactly one of the two base className hooks. Each of these hooks functionally identical, except with + // different styles applied, which makes it ok to conditionally change which hook is called. + const useRootClassName = disabled ? useRootNonInteractiveClassName : useRootInteractiveClassName; + const rootStyles = useRootStyles(); const inputStyles = useInputElementStyles(); const contentStyles = useContentStyles(); state.root.className = mergeClasses( inputClassNames.root, - rootStyles.base, + useRootClassName(), rootStyles[size], rootStyles[appearance], - !disabled && rootStyles.interactive, !disabled && appearance === 'outline' && rootStyles.outlineInteractive, !disabled && appearance === 'underline' && rootStyles.underlineInteractive, !disabled && filled && rootStyles.filledInteractive, @@ -264,13 +283,13 @@ export const useInputStyles_unstable = (state: InputState): InputState => { state.input.className = mergeClasses( inputClassNames.input, - inputStyles.base, + useInputClassName(), inputStyles[size], disabled && inputStyles.disabled, state.input.className, ); - const contentClasses = [contentStyles.base, disabled && contentStyles.disabled, contentStyles[size]]; + const contentClasses = [useContentClassName(), disabled && contentStyles.disabled, contentStyles[size]]; if (state.contentBefore) { state.contentBefore.className = mergeClasses( inputClassNames.contentBefore, From a50916231971548801050f59cc07d3d692c13bd1 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Fri, 10 Feb 2023 11:38:23 -0800 Subject: [PATCH 2/7] change file --- ...i-react-input-b3fdbd86-72a0-4b77-bea5-04c59b490021.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-input-b3fdbd86-72a0-4b77-bea5-04c59b490021.json diff --git a/change/@fluentui-react-input-b3fdbd86-72a0-4b77-bea5-04c59b490021.json b/change/@fluentui-react-input-b3fdbd86-72a0-4b77-bea5-04c59b490021.json new file mode 100644 index 00000000000000..dfbd6cb21fa60f --- /dev/null +++ b/change/@fluentui-react-input-b3fdbd86-72a0-4b77-bea5-04c59b490021.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "chore: Update Input to use makeResetStyles and reduce the number of classes applied", + "packageName": "@fluentui/react-input", + "email": "behowell@microsoft.com", + "dependentChangeType": "patch" +} From e6e96a2752939fd9442feebf0304449514267599 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Fri, 10 Feb 2023 14:07:21 -0800 Subject: [PATCH 3/7] Update styles --- .../src/components/Input/useInputStyles.ts | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/react-components/react-input/src/components/Input/useInputStyles.ts b/packages/react-components/react-input/src/components/Input/useInputStyles.ts index b6027fb316b897..85baaf4efb782e 100644 --- a/packages/react-components/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-components/react-input/src/components/Input/useInputStyles.ts @@ -198,21 +198,17 @@ const useInputClassName = makeResetStyles({ outlineStyle: 'none', // disable default browser outline - // size: medium (default) - ...typographyStyles.body1, + // Inherit typography styles from parent + fontFamily: 'inherit', + fontSize: 'inherit', + fontWeight: 'inherit', + lineHeight: 'inherit', }); const useInputElementStyles = makeStyles({ - small: { - // This is set on root but doesn't inherit - ...typographyStyles.caption1, - }, - medium: { - // included in useInputClassName - }, large: { - ...typographyStyles.body2, - ...shorthands.padding('0', tokens.spacingHorizontalSNudge), + paddingLeft: tokens.spacingHorizontalSNudge, + paddingRight: tokens.spacingHorizontalSNudge, }, disabled: { color: tokens.colorNeutralForegroundDisabled, @@ -284,7 +280,7 @@ export const useInputStyles_unstable = (state: InputState): InputState => { state.input.className = mergeClasses( inputClassNames.input, useInputClassName(), - inputStyles[size], + size === 'large' && inputStyles.large, disabled && inputStyles.disabled, state.input.className, ); From 775a576e1950ba75351190be4359f5624f9a9fc3 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Fri, 10 Feb 2023 14:35:46 -0800 Subject: [PATCH 4/7] Optimize paddingLeft/paddingRight --- .../react-input/src/components/Input/useInputStyles.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-components/react-input/src/components/Input/useInputStyles.ts b/packages/react-components/react-input/src/components/Input/useInputStyles.ts index 85baaf4efb782e..2339095fe90355 100644 --- a/packages/react-components/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-components/react-input/src/components/Input/useInputStyles.ts @@ -99,7 +99,8 @@ const useRootInteractiveClassName = makeResetStyles({ ...rootBaseStyles, ...root const useRootStyles = makeStyles({ small: { minHeight: fieldHeights.small, - ...shorthands.padding('0', tokens.spacingHorizontalSNudge), + paddingLeft: tokens.spacingHorizontalSNudge, + paddingRight: tokens.spacingHorizontalSNudge, ...typographyStyles.caption1, }, medium: { @@ -107,7 +108,8 @@ const useRootStyles = makeStyles({ }, large: { minHeight: fieldHeights.large, - ...shorthands.padding('0', tokens.spacingHorizontalM), + paddingLeft: tokens.spacingHorizontalM, + paddingRight: tokens.spacingHorizontalM, ...typographyStyles.body2, ...shorthands.gap(tokens.spacingHorizontalSNudge), }, From 7d2d661289a95eb4a117ee1e10e24334bf021063 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Fri, 10 Feb 2023 14:49:40 -0800 Subject: [PATCH 5/7] Merge appearance styles into the base styles --- .../src/components/Input/useInputStyles.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/react-components/react-input/src/components/Input/useInputStyles.ts b/packages/react-components/react-input/src/components/Input/useInputStyles.ts index 2339095fe90355..b651350756c290 100644 --- a/packages/react-components/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-components/react-input/src/components/Input/useInputStyles.ts @@ -31,6 +31,11 @@ const rootBaseStyles: GriffelResetStyle = { minHeight: fieldHeights.medium, padding: `0 ${tokens.spacingHorizontalMNudge}`, ...typographyStyles.body1, + + // appearance: outline (default) + backgroundColor: tokens.colorNeutralBackground1, + border: `1px solid ${tokens.colorNeutralStroke1}`, + borderBottomColor: tokens.colorNeutralStrokeAccessible, }; const rootInteractiveStyles: GriffelResetStyle = { @@ -114,9 +119,7 @@ const useRootStyles = makeStyles({ ...shorthands.gap(tokens.spacingHorizontalSNudge), }, outline: { - backgroundColor: tokens.colorNeutralBackground1, - ...shorthands.border('1px', 'solid', tokens.colorNeutralStroke1), - borderBottomColor: tokens.colorNeutralStrokeAccessible, + // included in rootBaseStyles }, outlineInteractive: { ':hover': { @@ -132,7 +135,10 @@ const useRootStyles = makeStyles({ underline: { backgroundColor: tokens.colorTransparentBackground, ...shorthands.borderRadius(0), // corners look strange if rounded - ...shorthands.borderBottom('1px', 'solid', tokens.colorNeutralStrokeAccessible), + // border is specified in rootBaseStyles, but we only want a bottom border here + borderTopStyle: 'none', + borderRightStyle: 'none', + borderLeftStyle: 'none', }, underlineInteractive: { ':hover': { @@ -145,7 +151,7 @@ const useRootStyles = makeStyles({ '::after': shorthands.borderRadius(0), // remove rounded corners from focus underline }, filled: { - ...shorthands.border('1px', 'solid', tokens.colorTransparentStroke), + ...shorthands.borderColor(tokens.colorTransparentStroke), }, filledInteractive: { // DO NOT add a space between the selectors! It changes the behavior of make-styles. From ddfb964da4776c31499070a88c1dd4206b13f97a Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Mon, 13 Feb 2023 10:43:30 -0800 Subject: [PATCH 6/7] Clean up comments --- .../react-input/src/components/Input/useInputStyles.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-components/react-input/src/components/Input/useInputStyles.ts b/packages/react-components/react-input/src/components/Input/useInputStyles.ts index b651350756c290..5044c6151dfece 100644 --- a/packages/react-components/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-components/react-input/src/components/Input/useInputStyles.ts @@ -206,7 +206,7 @@ const useInputClassName = makeResetStyles({ outlineStyle: 'none', // disable default browser outline - // Inherit typography styles from parent + // Inherit typography styles from root fontFamily: 'inherit', fontSize: 'inherit', fontWeight: 'inherit', @@ -232,7 +232,6 @@ const useContentClassName = makeResetStyles({ boxSizing: 'border-box', color: tokens.colorNeutralForeground3, // "icon color" in design spec display: 'flex', - // special case styling for icons (most common case) to ensure they're centered vertically // size: medium (default) '> svg': { fontSize: '20px' }, @@ -263,7 +262,7 @@ export const useInputStyles_unstable = (state: InputState): InputState => { const invalid = `${state.input['aria-invalid']}` === 'true'; const filled = appearance.startsWith('filled'); - // Call exactly one of the two base className hooks. Each of these hooks functionally identical, except with + // Call exactly one of the two base className hooks. Each of these hooks is functionally identical, but with // different styles applied, which makes it ok to conditionally change which hook is called. const useRootClassName = disabled ? useRootNonInteractiveClassName : useRootInteractiveClassName; From a60f0b3e3ed740acc7eae4f909dc9725e04a4f50 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Mon, 13 Feb 2023 11:49:53 -0800 Subject: [PATCH 7/7] Update from PR comment --- .../react-input/src/components/Input/useInputStyles.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/react-components/react-input/src/components/Input/useInputStyles.ts b/packages/react-components/react-input/src/components/Input/useInputStyles.ts index 5044c6151dfece..42a2ecbdc43997 100644 --- a/packages/react-components/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-components/react-input/src/components/Input/useInputStyles.ts @@ -92,9 +92,7 @@ const rootInteractiveStyles: GriffelResetStyle = { borderBottomColor: tokens.colorCompoundBrandStrokePressed, }, ':focus-within': { - outlineWidth: '2px', - outlineStyle: 'solid', - outlineColor: 'transparent', + outline: '2px solid transparent', }, };