From 4776d13951bf72c2dfad427dcd07fadcce2c40b3 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 18 Dec 2024 20:32:19 -0500 Subject: [PATCH] refactor(FormControl): update FormControl to use CSS Modules behind flag (#5438) * refactor: move form control test files into folder * refactor(FormControl): update FormControl to use CSS Modules behind flag * refactor(InputLabel): update InputLabel to use CSS Modules behind feature flag * refactor(FormControl): update Caption to use CSS Modules behind feature flag * refactor(InputValidation): refactor InputValidation to CSS Modules behind feature flag * chore: add changeset * Apply suggestions from code review Co-authored-by: Katie Langerman <18661030+langermank@users.noreply.github.com> * chore: update stylelint violations * fix: update selector for leading visual * add missing sx prop --------- Co-authored-by: Katie Langerman <18661030+langermank@users.noreply.github.com> Co-authored-by: Hussam Ghazzi --- .changeset/gentle-stingrays-search.md | 5 + .../src/FormControl/FormControl.module.css | 57 ++++++ .../react/src/FormControl/FormControl.tsx | 186 +++++++++++++----- .../FormControl/FormControlCaption.module.css | 9 + .../src/FormControl/FormControlCaption.tsx | 50 +++-- .../__tests__/FormControl.test.tsx | 54 +---- .../useFormControlForwardedProps.test.tsx | 53 +++++ .../react/src/FormControl/feature-flags.ts | 1 + .../internal/components/InputLabel.module.css | 32 +++ .../src/internal/components/InputLabel.tsx | 28 ++- .../components/InputValidation.module.css | 32 +++ .../internal/components/InputValidation.tsx | 37 +++- 12 files changed, 404 insertions(+), 140 deletions(-) create mode 100644 .changeset/gentle-stingrays-search.md create mode 100644 packages/react/src/FormControl/FormControl.module.css create mode 100644 packages/react/src/FormControl/FormControlCaption.module.css rename packages/react/src/{ => FormControl}/__tests__/FormControl.test.tsx (88%) create mode 100644 packages/react/src/FormControl/__tests__/useFormControlForwardedProps.test.tsx create mode 100644 packages/react/src/FormControl/feature-flags.ts create mode 100644 packages/react/src/internal/components/InputLabel.module.css create mode 100644 packages/react/src/internal/components/InputValidation.module.css diff --git a/.changeset/gentle-stingrays-search.md b/.changeset/gentle-stingrays-search.md new file mode 100644 index 00000000000..6ff383c37e5 --- /dev/null +++ b/.changeset/gentle-stingrays-search.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +Update FormControl to use CSS Modules behind feature flag diff --git a/packages/react/src/FormControl/FormControl.module.css b/packages/react/src/FormControl/FormControl.module.css new file mode 100644 index 00000000000..911f04bbde7 --- /dev/null +++ b/packages/react/src/FormControl/FormControl.module.css @@ -0,0 +1,57 @@ +.ControlHorizontalLayout { + display: flex; + + &:where([data-has-leading-visual]) { + align-items: center; + } +} + +.ControlVerticalLayout { + display: flex; + flex-direction: column; + align-items: flex-start; + + & > *:not(label) + * { + margin-top: var(--base-size-4); + } + + &[data-has-label] > * + * { + margin-top: var(--base-size-4); + } +} + +.ControlChoiceInputs > input { + margin-right: 0; + margin-left: 0; +} + +.LabelContainer { + > * { + /* stylelint-disable-next-line primer/spacing */ + padding-left: var(--stack-gap-condensed); + } + + > label { + font-weight: var(--base-text-weight-normal); + } +} + +.LeadingVisual { + margin-left: var(--base-size-8); + color: var(--fgColor-muted); + + &:where([data-disabled]) { + color: var(--control-fgColor-disabled); + } + + > * { + min-width: var(--text-body-size-large); + min-height: var(--text-body-size-large); + fill: currentColor; + } + + > *:where([data-has-caption]) { + min-width: var(--base-size-24); + min-height: var(--base-size-24); + } +} diff --git a/packages/react/src/FormControl/FormControl.tsx b/packages/react/src/FormControl/FormControl.tsx index 69d4b9bf871..8315f7be97a 100644 --- a/packages/react/src/FormControl/FormControl.tsx +++ b/packages/react/src/FormControl/FormControl.tsx @@ -1,6 +1,6 @@ +import {clsx} from 'clsx' import React, {useContext} from 'react' import Autocomplete from '../Autocomplete' -import Box from '../Box' import Checkbox from '../Checkbox' import Radio from '../Radio' import Select from '../Select/Select' @@ -10,7 +10,6 @@ import TextInputWithTokens from '../TextInputWithTokens' import Textarea from '../Textarea' import {CheckboxOrRadioGroupContext} from '../internal/components/CheckboxOrRadioGroup' import ValidationAnimationContainer from '../internal/components/ValidationAnimationContainer' -import {get} from '../constants' import {useSlots} from '../hooks/useSlots' import type {SxProp} from '../sx' import {useId} from '../hooks/useId' @@ -20,6 +19,12 @@ import FormControlLeadingVisual from './FormControlLeadingVisual' import FormControlValidation from './_FormControlValidation' import {FormControlContextProvider} from './_FormControlContext' import {warning} from '../utils/warning' +import styled from 'styled-components' +import sx from '../sx' +import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent' +import {cssModulesFlag} from './feature-flags' +import {useFeatureFlag} from '../FeatureFlags' +import classes from './FormControl.module.css' export type FormControlProps = { children?: React.ReactNode @@ -45,6 +50,7 @@ export type FormControlProps = { const FormControl = React.forwardRef( ({children, disabled: disabledProp, layout = 'vertical', id: idProp, required, sx, className}, ref) => { + const enabled = useFeatureFlag(cssModulesFlag) const [slots, childrenWithoutSlots] = useSlots(children, { caption: FormControlCaption, label: FormControlLabel, @@ -127,69 +133,62 @@ const FormControl = React.forwardRef( }} > {isChoiceInput || layout === 'horizontal' ? ( - - input': {marginLeft: 0, marginRight: 0}}}> - {React.isValidElement(InputComponent) && - React.cloneElement( - InputComponent as React.ReactElement<{ - id: string - disabled: boolean - required: boolean - ['aria-describedby']: string - }>, - { - id, - disabled, - // allow checkboxes to be required - required: required && !isRadioInput, - ['aria-describedby']: captionId as string, - }, - )} + + {React.isValidElement(InputComponent) + ? React.cloneElement( + InputComponent as React.ReactElement<{ + id: string + disabled: boolean + required: boolean + ['aria-describedby']: string + }>, + { + id, + disabled, + // allow checkboxes to be required + required: required && !isRadioInput, + ['aria-describedby']: captionId as string, + }, + ) + : null} {childrenWithoutSlots.filter( child => React.isValidElement(child) && ![Checkbox, Radio].some(inputComponent => child.type === inputComponent), )} - - {slots.leadingVisual && ( - *': { - minWidth: slots.caption ? get('fontSizes.4') : get('fontSizes.2'), - minHeight: slots.caption ? get('fontSizes.4') : get('fontSizes.2'), - fill: 'currentColor', - }, - }} - ml={2} + + {slots.leadingVisual ? ( + {slots.leadingVisual} - - )} - *': {paddingLeft: 'var(--stack-gap-condensed)'}, - '> label': {fontWeight: 'var(--base-text-weight-normal)'}, - }} - > + + ) : null} + {slots.label} {slots.caption} - - + + ) : ( - *:not(label) + *': {marginTop: 1}} : {'> * + *': {marginTop: 1}}), ...sx}} - className={className} + data-has-label={!isLabelHidden ? '' : undefined} + sx={sx} + className={clsx(className, { + [classes.ControlVerticalLayout]: enabled, + })} > {slots.label} {React.isValidElement(InputComponent) && @@ -215,13 +214,96 @@ const FormControl = React.forwardRef( {slots.validation} ) : null} {slots.caption} - + )} ) }, ) +const StyledHorizontalLayout = toggleStyledComponent( + cssModulesFlag, + 'div', + styled.div` + display: flex; + + &:where([data-has-leading-visual]) { + align-items: center; + } + + ${sx} + `, +) + +const StyledChoiceInputs = toggleStyledComponent( + cssModulesFlag, + 'div', + styled.div` + > input { + margin-left: 0; + margin-right: 0; + } + `, +) + +const StyledLabelContainer = toggleStyledComponent( + cssModulesFlag, + 'div', + styled.div` + > * { + padding-left: var(--stack-gap-condensed); + } + + > label { + font-weight: var(--base-text-weight-normal); + } + `, +) + +const StyledVerticalLayout = toggleStyledComponent( + cssModulesFlag, + 'div', + styled.div` + display: flex; + flex-direction: column; + align-items: flex-start; + + & > *:not(label) + * { + margin-top: var(--base-size-4); + } + + &:where([data-has-label]) > * + * { + margin-top: var(--base-size-4); + } + + ${sx} + `, +) + +const StyledLeadingVisual = toggleStyledComponent( + cssModulesFlag, + 'div', + styled.div` + color: var(--fgColor-default); + margin-left: var(--base-size-8); + + &:where([data-disabled]) { + color: var(--fgColor-muted); + } + + > * { + fill: currentColor; + min-width: var(--text-body-size-large); + min-height: var(--text-body-size-large); + } + + > *:where([data-has-caption]) { + min-width: var(--base-size-24); + min-height: var(--base-size-24); + } + `, +) + export default Object.assign(FormControl, { Caption: FormControlCaption, Label: FormControlLabel, diff --git a/packages/react/src/FormControl/FormControlCaption.module.css b/packages/react/src/FormControl/FormControlCaption.module.css new file mode 100644 index 00000000000..b51b54b53ca --- /dev/null +++ b/packages/react/src/FormControl/FormControlCaption.module.css @@ -0,0 +1,9 @@ +.Caption { + display: block; + font-size: var(--text-body-size-small); + color: var(--fgColor-muted); + + &:where([data-control-disabled]) { + color: var(--control-fgColor-disabled); + } +} diff --git a/packages/react/src/FormControl/FormControlCaption.tsx b/packages/react/src/FormControl/FormControlCaption.tsx index dc05e1a3bad..0b9ee2f5c64 100644 --- a/packages/react/src/FormControl/FormControlCaption.tsx +++ b/packages/react/src/FormControl/FormControlCaption.tsx @@ -1,22 +1,14 @@ +import {clsx} from 'clsx' import React from 'react' -import type {SxProp} from '../sx' -import {useFormControlContext} from './_FormControlContext' -import Text from '../Text' import styled from 'styled-components' -import {get} from '../constants' +import {cssModulesFlag} from './feature-flags' +import {useFeatureFlag} from '../FeatureFlags' +import Text from '../Text' import sx from '../sx' - -const StyledCaption = styled(Text)` - color: var(--fgColor-muted); - display: block; - font-size: ${get('fontSizes.0')}; - - &:where([data-control-disabled]) { - color: var(--control-fgColor-disabled); - } - - ${sx} -` +import type {SxProp} from '../sx' +import classes from './FormControlCaption.module.css' +import {useFormControlContext} from './_FormControlContext' +import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent' type FormControlCaptionProps = React.PropsWithChildren< { @@ -25,12 +17,36 @@ type FormControlCaptionProps = React.PropsWithChildren< > function FormControlCaption({id, children, sx}: FormControlCaptionProps) { + const enabled = useFeatureFlag(cssModulesFlag) const {captionId, disabled} = useFormControlContext() return ( - + {children} ) } +const StyledCaption = toggleStyledComponent( + cssModulesFlag, + Text, + styled(Text)` + color: var(--fgColor-muted); + display: block; + font-size: var(--text-body-size-small); + + &:where([data-control-disabled]) { + color: var(--control-fgColor-disabled); + } + + ${sx} + `, +) + export {FormControlCaption} diff --git a/packages/react/src/__tests__/FormControl.test.tsx b/packages/react/src/FormControl/__tests__/FormControl.test.tsx similarity index 88% rename from packages/react/src/__tests__/FormControl.test.tsx rename to packages/react/src/FormControl/__tests__/FormControl.test.tsx index 203f40d12c7..ba820dd28f7 100644 --- a/packages/react/src/__tests__/FormControl.test.tsx +++ b/packages/react/src/FormControl/__tests__/FormControl.test.tsx @@ -1,6 +1,5 @@ import React from 'react' import {render} from '@testing-library/react' -import {renderHook} from '@testing-library/react-hooks' import axe from 'axe-core' import { Autocomplete, @@ -12,8 +11,7 @@ import { Textarea, TextInput, TextInputWithTokens, - useFormControlForwardedProps, -} from '..' +} from '../..' import {MarkGithubIcon} from '@primer/octicons-react' const LABEL_TEXT = 'Form control' @@ -454,53 +452,3 @@ describe('FormControl', () => { }) }) }) - -describe('useFormControlForwardedProps', () => { - describe('when used outside FormControl', () => { - test('returns empty object when no props object passed', () => { - const result = renderHook(() => useFormControlForwardedProps({})) - expect(result.result.current).toEqual({}) - }) - - test('returns passed props object instance when passed', () => { - const props = {id: 'test-id'} - const result = renderHook(() => useFormControlForwardedProps(props)) - expect(result.result.current).toBe(props) - }) - }) - - test('provides context value when no props object is passed', () => { - const id = 'test-id' - - const {result} = renderHook(() => useFormControlForwardedProps({}), { - wrapper: ({children}: {children: React.ReactNode}) => ( - - Label - {children} - - ), - }) - - expect(result.current.disabled).toBe(true) - expect(result.current.id).toBe(id) - expect(result.current.required).toBe(true) - }) - - test('merges with props object, overriding to prioritize props when conflicting', () => { - const props = {id: 'override-id', xyz: 'someValue'} - - const {result} = renderHook(() => useFormControlForwardedProps(props), { - wrapper: ({children}: {children: React.ReactNode}) => ( - - Label - {children} - - ), - }) - - expect(result.current.disabled).toBe(true) - expect(result.current.id).toBe(props.id) - expect(result.current.required).toBeFalsy() - expect(result.current.xyz).toBe(props.xyz) - }) -}) diff --git a/packages/react/src/FormControl/__tests__/useFormControlForwardedProps.test.tsx b/packages/react/src/FormControl/__tests__/useFormControlForwardedProps.test.tsx new file mode 100644 index 00000000000..b71ad163989 --- /dev/null +++ b/packages/react/src/FormControl/__tests__/useFormControlForwardedProps.test.tsx @@ -0,0 +1,53 @@ +import React from 'react' +import {renderHook} from '@testing-library/react-hooks' +import FormControl, {useFormControlForwardedProps} from '../../FormControl' + +describe('useFormControlForwardedProps', () => { + describe('when used outside FormControl', () => { + test('returns empty object when no props object passed', () => { + const result = renderHook(() => useFormControlForwardedProps({})) + expect(result.result.current).toEqual({}) + }) + + test('returns passed props object instance when passed', () => { + const props = {id: 'test-id'} + const result = renderHook(() => useFormControlForwardedProps(props)) + expect(result.result.current).toBe(props) + }) + }) + + test('provides context value when no props object is passed', () => { + const id = 'test-id' + + const {result} = renderHook(() => useFormControlForwardedProps({}), { + wrapper: ({children}: {children: React.ReactNode}) => ( + + Label + {children} + + ), + }) + + expect(result.current.disabled).toBe(true) + expect(result.current.id).toBe(id) + expect(result.current.required).toBe(true) + }) + + test('merges with props object, overriding to prioritize props when conflicting', () => { + const props = {id: 'override-id', xyz: 'someValue'} + + const {result} = renderHook(() => useFormControlForwardedProps(props), { + wrapper: ({children}: {children: React.ReactNode}) => ( + + Label + {children} + + ), + }) + + expect(result.current.disabled).toBe(true) + expect(result.current.id).toBe(props.id) + expect(result.current.required).toBeFalsy() + expect(result.current.xyz).toBe(props.xyz) + }) +}) diff --git a/packages/react/src/FormControl/feature-flags.ts b/packages/react/src/FormControl/feature-flags.ts new file mode 100644 index 00000000000..0ea24433c23 --- /dev/null +++ b/packages/react/src/FormControl/feature-flags.ts @@ -0,0 +1 @@ +export const cssModulesFlag = 'primer_react_css_modules_team' diff --git a/packages/react/src/internal/components/InputLabel.module.css b/packages/react/src/internal/components/InputLabel.module.css new file mode 100644 index 00000000000..25b655a8b82 --- /dev/null +++ b/packages/react/src/internal/components/InputLabel.module.css @@ -0,0 +1,32 @@ +.Label { + display: block; + font-size: var(--text-body-size-medium); + font-weight: var(--base-text-weight-semibold); + color: var(--fgColor-default); + cursor: pointer; + align-self: flex-start; + + &:where([data-control-disabled]) { + color: var(--control-fgColor-disabled); + cursor: not-allowed; + } + + &:where([data-visually-hidden]) { + position: absolute; + width: 1px; + height: 1px; + padding: 0; + /* stylelint-disable-next-line primer/spacing */ + margin: -1px; + overflow: hidden; + clip: rect(0 0 0 0); + white-space: nowrap; + border: 0; + clip-path: inset(50%); + } +} + +.RequiredText { + display: flex; + column-gap: var(--base-size-4); +} diff --git a/packages/react/src/internal/components/InputLabel.tsx b/packages/react/src/internal/components/InputLabel.tsx index 9126d192086..b14f5366bfa 100644 --- a/packages/react/src/internal/components/InputLabel.tsx +++ b/packages/react/src/internal/components/InputLabel.tsx @@ -1,7 +1,10 @@ +import {clsx} from 'clsx' import React from 'react' import styled from 'styled-components' -import {get} from '../../constants' import sx, {type SxProp} from '../../sx' +import {cssModulesFlag} from '../../FormControl/feature-flags' +import {useFeatureFlag} from '../../FeatureFlags' +import classes from './InputLabel.module.css' type BaseProps = SxProp & { disabled?: boolean @@ -39,6 +42,7 @@ function InputLabel({ className, ...props }: Props) { + const enabled = useFeatureFlag(cssModulesFlag) return ( {required || requiredText ? ( - + {children} {requiredText ?? '*'} @@ -62,18 +72,13 @@ function InputLabel({ ) } -const StyledRequiredText = styled.span` - display: flex; - column-gap: ${get('space.1')}; -` - const StyledLabel = styled.label` align-self: flex-start; display: block; color: var(--fgColor-default); cursor: pointer; font-weight: 600; - font-size: ${get('fontSizes.1')}; + font-size: var(--text-body-size-medium); &:where([data-control-disabled]) { color: var(--fgColor-muted); @@ -96,4 +101,9 @@ const StyledLabel = styled.label` ${sx} ` +const StyledRequiredText = styled.span` + display: flex; + column-gap: var(--base-size-4); +` + export {InputLabel} diff --git a/packages/react/src/internal/components/InputValidation.module.css b/packages/react/src/internal/components/InputValidation.module.css new file mode 100644 index 00000000000..97428fbb395 --- /dev/null +++ b/packages/react/src/internal/components/InputValidation.module.css @@ -0,0 +1,32 @@ +.InputValidation { + display: flex; + font-size: var(--text-body-size-small); + font-weight: var(--base-text-weight-semibold); + /* stylelint-disable-next-line primer/colors */ + color: var(--inputValidation-fgColor); + + & :where(a) { + color: currentColor; + text-decoration: underline; + } + + &:where([data-validation-status='success']) { + --inputValidation-fgColor: var(--fgColor-success); + } + + &:where([data-validation-status='error']) { + --inputValidation-fgColor: var(--fgColor-danger); + } +} + +.ValidationIcon { + align-items: center; + display: flex; + margin-inline-end: var(--base-size-4); + min-height: var(--inputValidation-iconSize); +} + +.ValidationText { + /* stylelint-disable-next-line primer/typography */ + line-height: var(--inputValidation-lineHeight); +} diff --git a/packages/react/src/internal/components/InputValidation.tsx b/packages/react/src/internal/components/InputValidation.tsx index d23fe867cb5..d7224c429fa 100644 --- a/packages/react/src/internal/components/InputValidation.tsx +++ b/packages/react/src/internal/components/InputValidation.tsx @@ -1,12 +1,15 @@ import type {IconProps} from '@primer/octicons-react' import {AlertFillIcon, CheckCircleFillIcon} from '@primer/octicons-react' +import {clsx} from 'clsx' import React from 'react' +import styled from 'styled-components' import Text from '../../Text' +import sx from '../../sx' import type {SxProp} from '../../sx' +import {cssModulesFlag} from '../../FormControl/feature-flags' import type {FormValidationStatus} from '../../utils/types/FormValidationStatus' -import styled from 'styled-components' -import {get} from '../../constants' -import sx from '../../sx' +import {useFeatureFlag} from '../../FeatureFlags' +import classes from './InputValidation.module.css' type Props = { id: string @@ -22,6 +25,7 @@ const validationIconMap: Record< } const InputValidation: React.FC> = ({children, id, validationStatus, sx}) => { + const enabled = useFeatureFlag(cssModulesFlag) const IconComponent = validationStatus ? validationIconMap[validationStatus] : undefined // TODO: use `text-caption-lineHeight` token as a custom property when it's available @@ -31,10 +35,19 @@ const InputValidation: React.FC> = ({children, id const iconBoxMinHeight = iconSize * captionLineHeight return ( - + {IconComponent ? ( ) : null} - + {children} @@ -54,7 +73,7 @@ const InputValidation: React.FC> = ({children, id const StyledInputValidation = styled(Text)` color: var(--inputValidation-fgColor); display: flex; - font-size: ${get('fontSizes.0')}; + font-size: var(--text-body-size-small); font-weight: 600; & :where(a) { @@ -63,11 +82,11 @@ const StyledInputValidation = styled(Text)` } &:where([data-validation-status='success']) { - --inputValidation-fgColor: ${get('colors.success.fg')}; + --inputValidation-fgColor: var(--fgColor-success); } &:where([data-validation-status='error']) { - --inputValidation-fgColor: ${get('colors.danger.fg')}; + --inputValidation-fgColor: var(--fgColor-danger); } ${sx} @@ -76,7 +95,7 @@ const StyledInputValidation = styled(Text)` const StyledValidationIcon = styled.span` align-items: center; display: flex; - margin-inline-end: ${get('space.1')}; + margin-inline-end: var(--base-size-4); min-height: var(--inputValidation-iconSize); `