From 8743c1ce57ed96695d0a8ea7a6f797c7b5d4d50d Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Fri, 21 Mar 2025 13:35:00 -0500 Subject: [PATCH 1/7] Removes feature flag for FormControl component. --- .../react/src/FormControl/FormControl.tsx | 186 ++++++------------ 1 file changed, 61 insertions(+), 125 deletions(-) diff --git a/packages/react/src/FormControl/FormControl.tsx b/packages/react/src/FormControl/FormControl.tsx index e66af31eb61..4d21c8cc955 100644 --- a/packages/react/src/FormControl/FormControl.tsx +++ b/packages/react/src/FormControl/FormControl.tsx @@ -20,12 +20,8 @@ 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' +import {defaultSxProp} from '../utils/defaultSxProp' export type FormControlProps = { children?: React.ReactNode @@ -51,7 +47,6 @@ 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, @@ -122,6 +117,46 @@ const FormControl = React.forwardRef( } const isLabelHidden = slots.label?.props.visuallyHidden + const LayoutChildren = ( + <> +
+ {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 ? ( +
+ {slots.leadingVisual} +
+ ) : null} +
+ {slots.label} + {slots.caption} +
+ + ) return ( ( }} > {isChoiceInput || layout === 'horizontal' ? ( - - - {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 ? ( - - {slots.leadingVisual} - - ) : null} - - {slots.label} - {slots.caption} - - + sx !== defaultSxProp ? ( + + {LayoutChildren} + + ) : ( +
+ {LayoutChildren} +
+ ) ) : ( ( display="flex" flexDirection="column" alignItems="flex-start" - sx={ - enabled - ? sx - : {...(isLabelHidden ? {'> *:not(label) + *': {marginTop: 1}} : {'> * + *': {marginTop: 1}}), ...sx} - } - className={clsx(className, { - [classes.ControlVerticalLayout]: enabled, - })} + sx={sx} + className={clsx(className, classes.ControlVerticalLayout)} > {slots.label} {React.isValidElement(InputComponent) && @@ -229,69 +228,6 @@ const FormControl = React.forwardRef( }, ) -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 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, From 2462383ace4d8bc16533f410cce9ea8685196b5c Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Fri, 21 Mar 2025 13:43:27 -0500 Subject: [PATCH 2/7] Slightly better naming. --- packages/react/src/FormControl/FormControl.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react/src/FormControl/FormControl.tsx b/packages/react/src/FormControl/FormControl.tsx index 4d21c8cc955..91d6b9dc0e5 100644 --- a/packages/react/src/FormControl/FormControl.tsx +++ b/packages/react/src/FormControl/FormControl.tsx @@ -117,7 +117,7 @@ const FormControl = React.forwardRef( } const isLabelHidden = slots.label?.props.visuallyHidden - const LayoutChildren = ( + const InputChildren = ( <>
{React.isValidElement(InputComponent) @@ -176,7 +176,7 @@ const FormControl = React.forwardRef( sx={sx} className={clsx(className, classes.ControlHorizontalLayout)} > - {LayoutChildren} + {InputChildren} ) : (
( data-has-leading-visual={slots.leadingVisual ? '' : undefined} className={clsx(className, classes.ControlHorizontalLayout)} > - {LayoutChildren} + {InputChildren}
) ) : ( From 9132b85745e2f4ea3d6459bcf7646dd5048d67ad Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Fri, 21 Mar 2025 13:43:59 -0500 Subject: [PATCH 3/7] Changeset. --- .changeset/hungry-pumas-poke.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/hungry-pumas-poke.md diff --git a/.changeset/hungry-pumas-poke.md b/.changeset/hungry-pumas-poke.md new file mode 100644 index 00000000000..1a82f7332bb --- /dev/null +++ b/.changeset/hungry-pumas-poke.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Removes feature flag for FormControl From 98703b6a2fc300f4e24e5e7304ce8adcfd9482b0 Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Tue, 25 Mar 2025 17:26:57 +0000 Subject: [PATCH 4/7] Swaps out Input Validation, starts work on Input Label. --- .../src/internal/components/InputLabel.tsx | 22 +------ .../internal/components/InputValidation.tsx | 63 +++---------------- 2 files changed, 13 insertions(+), 72 deletions(-) diff --git a/packages/react/src/internal/components/InputLabel.tsx b/packages/react/src/internal/components/InputLabel.tsx index 54002f3a3fb..1aaa6853512 100644 --- a/packages/react/src/internal/components/InputLabel.tsx +++ b/packages/react/src/internal/components/InputLabel.tsx @@ -2,8 +2,6 @@ import {clsx} from 'clsx' import React from 'react' import styled from 'styled-components' import sx, {type SxProp} from '../../sx' -import {cssModulesFlag} from '../../FormControl/feature-flags' -import {useFeatureFlag} from '../../FeatureFlags' import classes from './InputLabel.module.css' import {toggleStyledComponent} from '../utils/toggleStyledComponent' @@ -43,7 +41,6 @@ function InputLabel({ className, ...props }: Props) { - const enabled = useFeatureFlag(cssModulesFlag) return ( {required || requiredText ? ( - + {children} {requiredText ?? '*'} - + ) : ( children )} @@ -74,7 +67,7 @@ function InputLabel({ } const StyledLabel = toggleStyledComponent( - cssModulesFlag, + '', 'label', styled.label` align-self: flex-start; @@ -106,13 +99,4 @@ const StyledLabel = toggleStyledComponent( `, ) -const StyledRequiredText = toggleStyledComponent( - cssModulesFlag, - 'span', - styled.span` - display: flex; - column-gap: var(--base-size-4); - `, -) - export {InputLabel} diff --git a/packages/react/src/internal/components/InputValidation.tsx b/packages/react/src/internal/components/InputValidation.tsx index d7224c429fa..05542c65571 100644 --- a/packages/react/src/internal/components/InputValidation.tsx +++ b/packages/react/src/internal/components/InputValidation.tsx @@ -2,14 +2,11 @@ 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 {useFeatureFlag} from '../../FeatureFlags' import classes from './InputValidation.module.css' +import {defaultSxProp} from '../../utils/defaultSxProp' type Props = { id: string @@ -25,7 +22,6 @@ 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 @@ -35,19 +31,15 @@ const InputValidation: React.FC> = ({children, id const iconBoxMinHeight = iconSize * captionLineHeight return ( - {IconComponent ? ( - + ) : null} - {children} - - + + ) } -const StyledInputValidation = styled(Text)` - color: var(--inputValidation-fgColor); - display: flex; - font-size: var(--text-body-size-small); - font-weight: 600; - - & :where(a) { - color: currentColor; - text-dectoration: underline; - } - - &:where([data-validation-status='success']) { - --inputValidation-fgColor: var(--fgColor-success); - } - - &:where([data-validation-status='error']) { - --inputValidation-fgColor: var(--fgColor-danger); - } - - ${sx} -` - -const StyledValidationIcon = styled.span` - align-items: center; - display: flex; - margin-inline-end: var(--base-size-4); - min-height: var(--inputValidation-iconSize); -` - -const StyledValidationText = styled.span` - line-height: var(--inputValidation-lineHeight); -` - export default InputValidation From 48526ac3a34285bf497e990bb4a9c9ebeb956906 Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Tue, 25 Mar 2025 16:05:22 -0500 Subject: [PATCH 5/7] Swap out input label component. --- .../src/internal/components/InputLabel.tsx | 46 ++----------------- 1 file changed, 5 insertions(+), 41 deletions(-) diff --git a/packages/react/src/internal/components/InputLabel.tsx b/packages/react/src/internal/components/InputLabel.tsx index 1aaa6853512..75a07d4436f 100644 --- a/packages/react/src/internal/components/InputLabel.tsx +++ b/packages/react/src/internal/components/InputLabel.tsx @@ -4,7 +4,7 @@ import styled from 'styled-components' import sx, {type SxProp} from '../../sx' import classes from './InputLabel.module.css' import {toggleStyledComponent} from '../utils/toggleStyledComponent' - +import {toggleSxComponent} from '../utils/toggleSxComponent' type BaseProps = SxProp & { disabled?: boolean required?: boolean @@ -41,17 +41,14 @@ function InputLabel({ className, ...props }: Props) { + const Label = toggleSxComponent({sx}, as) as React.ComponentType return ( - {required || requiredText ? ( @@ -62,41 +59,8 @@ function InputLabel({ ) : ( children )} - + ) } -const StyledLabel = toggleStyledComponent( - '', - 'label', - styled.label` - align-self: flex-start; - display: block; - color: var(--fgColor-default); - cursor: pointer; - font-weight: 600; - font-size: var(--text-body-size-medium); - - &:where([data-control-disabled]) { - color: var(--fgColor-muted); - cursor: not-allowed; - } - - &:where([data-visually-hidden]) { - border: 0; - clip: rect(0 0 0 0); - clip-path: inset(50%); - height: 1px; - margin: -1px; - overflow: hidden; - padding: 0; - position: absolute; - white-space: nowrap; - width: 1px; - } - - ${sx} - `, -) - export {InputLabel} From db982acf547431d805bbf79905698260ed2b7506 Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Tue, 25 Mar 2025 16:06:45 -0500 Subject: [PATCH 6/7] Adds util function to account for SX dependent components to ease migration. --- .../__tests__/toggleSxComponent.test.tsx | 38 +++++++++++++++++++ .../src/internal/utils/toggleSxComponent.tsx | 35 +++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 packages/react/src/internal/utils/__tests__/toggleSxComponent.test.tsx create mode 100644 packages/react/src/internal/utils/toggleSxComponent.tsx diff --git a/packages/react/src/internal/utils/__tests__/toggleSxComponent.test.tsx b/packages/react/src/internal/utils/__tests__/toggleSxComponent.test.tsx new file mode 100644 index 00000000000..f077b39159d --- /dev/null +++ b/packages/react/src/internal/utils/__tests__/toggleSxComponent.test.tsx @@ -0,0 +1,38 @@ +import React from 'react' +import {render} from '@testing-library/react' +import {toggleSxComponent} from '../toggleSxComponent' + +const customSx = {color: 'red', p: 2} + +describe('toggleSxComponent', () => { + test('renders the plain component when no sx', () => { + const TestComponent = toggleSxComponent({}, 'span') + const {container} = render() + expect(container.firstChild).toBeInstanceOf(HTMLSpanElement) + }) + + test('renders Box with `as` if `sx` is provided', () => { + const TestComponent = toggleSxComponent(customSx, 'div') + const {container} = render() + + expect(container.firstChild).toBeInstanceOf(HTMLButtonElement) + expect(container.firstChild).toHaveStyle('color: red') + }) + + test('swaps out component if `sx` is not the default', () => { + const Label = toggleSxComponent(customSx, 'label') as React.ComponentType<{htmlFor: string}> + const {container} = render(