From 69866a18fdec2ef759e66899c60d73127e6e2d09 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Wed, 15 Feb 2023 13:13:17 -0800 Subject: [PATCH 1/6] chore: Clean up Input's interactive styles --- .../src/stories/Input.stories.tsx | 10 ++- .../src/components/Input/useInputStyles.ts | 68 +++++++------------ 2 files changed, 34 insertions(+), 44 deletions(-) diff --git a/apps/vr-tests-react-components/src/stories/Input.stories.tsx b/apps/vr-tests-react-components/src/stories/Input.stories.tsx index ad5bec6335ff11..8c7da1d42d22d3 100644 --- a/apps/vr-tests-react-components/src/stories/Input.stories.tsx +++ b/apps/vr-tests-react-components/src/stories/Input.stories.tsx @@ -40,7 +40,15 @@ storiesOf('Input Converged', module) )) - .addStory('Disabled', () => ) + .addStory('Disabled: outline', () => ) + .addStory('Disabled: underline', () => ) + .addStory('Disabled: filled-darker', () => ) + .addStory('Disabled: filled-lighter', () => ( + // filledLighter requires a background to show up (this is colorNeutralBackground3 in web light theme) +
+ +
+ )) .addStory('With value', () => ); // Non-interactive stories 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 42a2ecbdc43997..de0f81087a86c4 100644 --- a/packages/react-components/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-components/react-input/src/components/Input/useInputStyles.ts @@ -1,6 +1,5 @@ import { tokens, typographyStyles } from '@fluentui/react-theme'; 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'; @@ -18,7 +17,7 @@ const fieldHeights = { large: '40px', }; -const rootBaseStyles: GriffelResetStyle = { +const useRootClassName = makeResetStyles({ display: 'inline-flex', alignItems: 'center', flexWrap: 'nowrap', @@ -36,9 +35,15 @@ const rootBaseStyles: GriffelResetStyle = { backgroundColor: tokens.colorNeutralBackground1, border: `1px solid ${tokens.colorNeutralStroke1}`, borderBottomColor: tokens.colorNeutralStrokeAccessible, -}; + ':hover': { + borderColor: tokens.colorNeutralStroke1Hover, + borderBottomColor: tokens.colorNeutralStrokeAccessibleHover, + }, + ':active,:focus-within': { + borderColor: tokens.colorNeutralStroke1Pressed, + borderBottomColor: tokens.colorNeutralStrokeAccessiblePressed, + }, -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': { @@ -94,10 +99,7 @@ const rootInteractiveStyles: GriffelResetStyle = { ':focus-within': { outline: '2px solid transparent', }, -}; - -const useRootNonInteractiveClassName = makeResetStyles(rootBaseStyles); -const useRootInteractiveClassName = makeResetStyles({ ...rootBaseStyles, ...rootInteractiveStyles }); +}); const useRootStyles = makeStyles({ small: { @@ -119,42 +121,19 @@ const useRootStyles = makeStyles({ outline: { // included in rootBaseStyles }, - outlineInteractive: { - ':hover': { - ...shorthands.borderColor(tokens.colorNeutralStroke1Hover), - borderBottomColor: tokens.colorNeutralStrokeAccessibleHover, - }, - // DO NOT add a space between the selectors! It changes the behavior of make-styles. - ':active,:focus-within': { - ...shorthands.borderColor(tokens.colorNeutralStroke1Pressed), - borderBottomColor: tokens.colorNeutralStrokeAccessiblePressed, - }, - }, underline: { backgroundColor: tokens.colorTransparentBackground, - ...shorthands.borderRadius(0), // corners look strange if rounded + '&,::after': shorthands.borderRadius(0), // remove rounded corners on the underline and the ::after focus underline // border is specified in rootBaseStyles, but we only want a bottom border here borderTopStyle: 'none', borderRightStyle: 'none', borderLeftStyle: 'none', }, - underlineInteractive: { - ':hover': { - borderBottomColor: tokens.colorNeutralStrokeAccessibleHover, - }, - // DO NOT add a space between the selectors! It changes the behavior of make-styles. - ':active,:focus-within': { - borderBottomColor: tokens.colorNeutralStrokeAccessiblePressed, - }, - '::after': shorthands.borderRadius(0), // remove rounded corners from focus underline - }, filled: { ...shorthands.borderColor(tokens.colorTransparentStroke), - }, - filledInteractive: { + // DO NOT add a space between the selectors! It changes the behavior of make-styles. - ':hover,:focus-within': { - // also handles pressed border color (:active) + ':hover,:active,:focus-within': { ...shorthands.borderColor(tokens.colorTransparentStrokeInteractive), }, }, @@ -180,9 +159,19 @@ const useRootStyles = makeStyles({ disabled: { cursor: 'not-allowed', backgroundColor: tokens.colorTransparentBackground, - ...shorthands.borderColor(tokens.colorNeutralStrokeDisabled), + '&,:hover,:active,:focus-within': { + ...shorthands.borderColor(tokens.colorNeutralStrokeDisabled), + }, '@media (forced-colors: active)': { - ...shorthands.borderColor('GrayText'), + '&,:hover,:active,:focus-within': { + ...shorthands.borderColor('GrayText'), + }, + }, + '::after': { + content: 'unset', // remove the focus border + }, + ':focus-within': { + outlineStyle: 'none', // remove the focus outline }, }, }); @@ -260,10 +249,6 @@ 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 is functionally identical, but 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(); @@ -273,9 +258,6 @@ export const useInputStyles_unstable = (state: InputState): InputState => { useRootClassName(), rootStyles[size], rootStyles[appearance], - !disabled && appearance === 'outline' && rootStyles.outlineInteractive, - !disabled && appearance === 'underline' && rootStyles.underlineInteractive, - !disabled && filled && rootStyles.filledInteractive, filled && rootStyles.filled, !disabled && invalid && rootStyles.invalid, disabled && rootStyles.disabled, From 5bb1313f9a540dbd2ba7f1b35e7bab3828165845 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Wed, 15 Feb 2023 13:13:41 -0800 Subject: [PATCH 2/6] change file --- ...i-react-input-80b38eae-aa0c-4ba8-b5c8-6f523ded4190.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-input-80b38eae-aa0c-4ba8-b5c8-6f523ded4190.json diff --git a/change/@fluentui-react-input-80b38eae-aa0c-4ba8-b5c8-6f523ded4190.json b/change/@fluentui-react-input-80b38eae-aa0c-4ba8-b5c8-6f523ded4190.json new file mode 100644 index 00000000000000..72b1428ccedfff --- /dev/null +++ b/change/@fluentui-react-input-80b38eae-aa0c-4ba8-b5c8-6f523ded4190.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "chore: Clean up Input's interactive styles", + "packageName": "@fluentui/react-input", + "email": "behowell@microsoft.com", + "dependentChangeType": "patch" +} From 696c2486a4d09f2dbab48df6232606d9f22e0673 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Wed, 15 Feb 2023 13:15:36 -0800 Subject: [PATCH 3/6] Change VR stories to keep existing disabled story --- .../src/stories/Input.stories.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/vr-tests-react-components/src/stories/Input.stories.tsx b/apps/vr-tests-react-components/src/stories/Input.stories.tsx index 8c7da1d42d22d3..68d5884464d212 100644 --- a/apps/vr-tests-react-components/src/stories/Input.stories.tsx +++ b/apps/vr-tests-react-components/src/stories/Input.stories.tsx @@ -40,10 +40,11 @@ storiesOf('Input Converged', module) )) - .addStory('Disabled: outline', () => ) - .addStory('Disabled: underline', () => ) - .addStory('Disabled: filled-darker', () => ) - .addStory('Disabled: filled-lighter', () => ( + .addStory('Disabled', () => ) + .addStory('Disabled with value: outline', () => ) + .addStory('Disabled with value: underline', () => ) + .addStory('Disabled with value: filled-darker', () => ) + .addStory('Disabled with value: filled-lighter', () => ( // filledLighter requires a background to show up (this is colorNeutralBackground3 in web light theme)
From 3eb23b57a18a27f6a6233d7cc223f8a8dbcb3e84 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Wed, 15 Feb 2023 16:29:32 -0800 Subject: [PATCH 4/6] Scale back the changes to interactive styles: only the root styles and not all of the focus, etc. styles. --- .../src/components/Input/useInputStyles.ts | 60 +++++++++++++------ 1 file changed, 41 insertions(+), 19 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 de0f81087a86c4..7ddc40c4f3c7da 100644 --- a/packages/react-components/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-components/react-input/src/components/Input/useInputStyles.ts @@ -35,14 +35,6 @@ const useRootClassName = makeResetStyles({ backgroundColor: tokens.colorNeutralBackground1, border: `1px solid ${tokens.colorNeutralStroke1}`, borderBottomColor: tokens.colorNeutralStrokeAccessible, - ':hover': { - borderColor: tokens.colorNeutralStroke1Hover, - borderBottomColor: tokens.colorNeutralStrokeAccessibleHover, - }, - ':active,:focus-within': { - borderColor: tokens.colorNeutralStroke1Pressed, - borderBottomColor: tokens.colorNeutralStrokeAccessiblePressed, - }, // 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. @@ -121,19 +113,48 @@ const useRootStyles = makeStyles({ outline: { // included in rootBaseStyles }, + outlineInteractive: { + ':hover': { + ...shorthands.borderColor(tokens.colorNeutralStroke1Hover), + borderBottomColor: tokens.colorNeutralStrokeAccessibleHover, + }, + // DO NOT add a space between the selectors! It changes the behavior of make-styles. + ':active,:focus-within': { + ...shorthands.borderColor(tokens.colorNeutralStroke1Pressed), + borderBottomColor: tokens.colorNeutralStrokeAccessiblePressed, + }, + }, underline: { backgroundColor: tokens.colorTransparentBackground, - '&,::after': shorthands.borderRadius(0), // remove rounded corners on the underline and the ::after focus underline + // corners look strange if rounded + borderBottomLeftRadius: 0, + borderBottomRightRadius: 0, // border is specified in rootBaseStyles, but we only want a bottom border here borderTopStyle: 'none', borderRightStyle: 'none', borderLeftStyle: 'none', }, + underlineInteractive: { + ':hover': { + borderBottomColor: tokens.colorNeutralStrokeAccessibleHover, + }, + // DO NOT add a space between the selectors! It changes the behavior of make-styles. + ':active,:focus-within': { + borderBottomColor: tokens.colorNeutralStrokeAccessiblePressed, + }, + // remove rounded corners from focus underline + '::after': { + borderBottomLeftRadius: 0, + borderBottomRightRadius: 0, + }, + }, filled: { ...shorthands.borderColor(tokens.colorTransparentStroke), - + }, + filledInteractive: { // DO NOT add a space between the selectors! It changes the behavior of make-styles. - ':hover,:active,:focus-within': { + ':hover,:focus-within': { + // also handles pressed border color (:active) ...shorthands.borderColor(tokens.colorTransparentStrokeInteractive), }, }, @@ -159,19 +180,17 @@ const useRootStyles = makeStyles({ disabled: { cursor: 'not-allowed', backgroundColor: tokens.colorTransparentBackground, - '&,:hover,:active,:focus-within': { - ...shorthands.borderColor(tokens.colorNeutralStrokeDisabled), - }, + ...shorthands.borderColor(tokens.colorNeutralStrokeDisabled), '@media (forced-colors: active)': { - '&,:hover,:active,:focus-within': { - ...shorthands.borderColor('GrayText'), - }, + ...shorthands.borderColor('GrayText'), }, + // remove the focus border '::after': { - content: 'unset', // remove the focus border + content: 'unset', }, + // remove the focus outline ':focus-within': { - outlineStyle: 'none', // remove the focus outline + outlineStyle: 'none', }, }, }); @@ -258,6 +277,9 @@ export const useInputStyles_unstable = (state: InputState): InputState => { useRootClassName(), rootStyles[size], rootStyles[appearance], + !disabled && appearance === 'outline' && rootStyles.outlineInteractive, + !disabled && appearance === 'underline' && rootStyles.underlineInteractive, + !disabled && filled && rootStyles.filledInteractive, filled && rootStyles.filled, !disabled && invalid && rootStyles.invalid, disabled && rootStyles.disabled, From 18eb079fa7d52c7d18085657a192e700f21a3648 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Thu, 16 Feb 2023 16:48:50 -0800 Subject: [PATCH 5/6] Revert changes to outline's border radius --- .../react-input/src/components/Input/useInputStyles.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 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 7ddc40c4f3c7da..0b016fc341580c 100644 --- a/packages/react-components/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-components/react-input/src/components/Input/useInputStyles.ts @@ -126,9 +126,7 @@ const useRootStyles = makeStyles({ }, underline: { backgroundColor: tokens.colorTransparentBackground, - // corners look strange if rounded - borderBottomLeftRadius: 0, - borderBottomRightRadius: 0, + ...shorthands.borderRadius(0), // corners look strange if rounded // border is specified in rootBaseStyles, but we only want a bottom border here borderTopStyle: 'none', borderRightStyle: 'none', @@ -142,11 +140,7 @@ const useRootStyles = makeStyles({ ':active,:focus-within': { borderBottomColor: tokens.colorNeutralStrokeAccessiblePressed, }, - // remove rounded corners from focus underline - '::after': { - borderBottomLeftRadius: 0, - borderBottomRightRadius: 0, - }, + '::after': shorthands.borderRadius(0), // remove rounded corners from focus underline }, filled: { ...shorthands.borderColor(tokens.colorTransparentStroke), From 0116bcaf2181a5b35a21184e3ccdd98121180e93 Mon Sep 17 00:00:00 2001 From: Ben Howell Date: Thu, 16 Feb 2023 16:49:08 -0800 Subject: [PATCH 6/6] Rename disabled and invalid VR tests --- .../src/stories/Input.stories.tsx | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/apps/vr-tests-react-components/src/stories/Input.stories.tsx b/apps/vr-tests-react-components/src/stories/Input.stories.tsx index 68d5884464d212..66dba5b304ccca 100644 --- a/apps/vr-tests-react-components/src/stories/Input.stories.tsx +++ b/apps/vr-tests-react-components/src/stories/Input.stories.tsx @@ -31,20 +31,23 @@ storiesOf('Input Converged', module)
)) - .addStory('Invalid: outline', () => ) - .addStory('Invalid: underline', () => ) - .addStory('Invalid: filled-darker', () => ) - .addStory('Invalid: filled-lighter', () => ( + .addStory('Invalid, appearance: outline', () => ) + .addStory('Invalid, appearance: underline', () => ( + + )) + .addStory('Invalid, appearance: filled-darker', () => ( + + )) + .addStory('Invalid, appearance: filled-lighter', () => ( // filledLighter requires a background to show up (this is colorNeutralBackground3 in web light theme)
)) - .addStory('Disabled', () => ) - .addStory('Disabled with value: outline', () => ) - .addStory('Disabled with value: underline', () => ) - .addStory('Disabled with value: filled-darker', () => ) - .addStory('Disabled with value: filled-lighter', () => ( + .addStory('Disabled, appearance: outline', () => ) + .addStory('Disabled, appearance: underline', () => ) + .addStory('Disabled, appearance: filled-darker', () => ) + .addStory('Disabled, appearance: filled-lighter', () => ( // filledLighter requires a background to show up (this is colorNeutralBackground3 in web light theme)