From 591bc87ca6dfee79ee2e5921f97fbe04179e2993 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Thu, 3 Oct 2024 23:57:46 +0000 Subject: [PATCH 1/6] Remove css modules feature flag from Text --- e2e/components/Text.test.ts | 36 -------------- packages/react/src/Text/Text.tsx | 81 ++++---------------------------- 2 files changed, 9 insertions(+), 108 deletions(-) diff --git a/e2e/components/Text.test.ts b/e2e/components/Text.test.ts index e039bac12c4..031a4adf876 100644 --- a/e2e/components/Text.test.ts +++ b/e2e/components/Text.test.ts @@ -44,25 +44,6 @@ test.describe('Text', () => { test('default @vrt', async ({page}) => { await visit(page, { id: story.id, - globals: { - featureFlags: { - primer_react_css_modules_ga: true, - }, - }, - }) - - // Default state - expect(await page.screenshot()).toMatchSnapshot(`Text.${story.title}.png`) - }) - - test('default (styled-system) @vrt', async ({page}) => { - await visit(page, { - id: story.id, - globals: { - featureFlags: { - primer_react_css_modules_ga: false, - }, - }, }) // Default state @@ -72,23 +53,6 @@ test.describe('Text', () => { test('axe @aat', async ({page}) => { await visit(page, { id: story.id, - globals: { - featureFlags: { - primer_react_css_modules_ga: true, - }, - }, - }) - await expect(page).toHaveNoViolations() - }) - - test('axe (styled-system) @aat', async ({page}) => { - await visit(page, { - id: story.id, - globals: { - featureFlags: { - primer_react_css_modules_ga: false, - }, - }, }) await expect(page).toHaveNoViolations() }) diff --git a/packages/react/src/Text/Text.tsx b/packages/react/src/Text/Text.tsx index 3f7ee333b97..54429e2faf5 100644 --- a/packages/react/src/Text/Text.tsx +++ b/packages/react/src/Text/Text.tsx @@ -1,17 +1,13 @@ import {clsx} from 'clsx' -import styled, {type StyledComponent} from 'styled-components' +import {type StyledComponent} from 'styled-components' import React, {forwardRef} from 'react' import type {SystemCommonProps, SystemTypographyProps} from '../constants' -import {COMMON, TYPOGRAPHY} from '../constants' import type {SxProp} from '../sx' -import sx from '../sx' -import {useFeatureFlag} from '../FeatureFlags' import Box from '../Box' import {useRefObjectAsForwardedRef} from '../hooks' import classes from './Text.module.css' -import type {ComponentProps} from '../utils/types' -type StyledTextProps = { +export type TextProps = { // eslint-disable-next-line @typescript-eslint/no-explicit-any as?: React.ComponentType | keyof JSX.IntrinsicElements size?: 'large' | 'medium' | 'small' @@ -20,95 +16,36 @@ type StyledTextProps = { SystemCommonProps & SxProp -const StyledText = styled.span` - ${TYPOGRAPHY}; - ${COMMON}; - - &:where([data-size='small']) { - font-size: var(--text-body-size-small, 0.75rem); - line-height: var(--text-body-lineHeight-small, 1.6666); - } - - &:where([data-size='medium']) { - font-size: var(--text-body-size-medium, 0.875rem); - line-height: var(--text-body-lineHeight-medium, 1.4285); - } - - &:where([data-size='large']) { - font-size: var(--text-body-size-large, 1rem); - line-height: var(--text-body-lineHeight-large, 1.5); - } - - &:where([data-weight='light']) { - font-weight: var(--base-text-weight-light, 300); - } - - &:where([data-weight='normal']) { - font-weight: var(--base-text-weight-normal, 400); - } - - &:where([data-weight='medium']) { - font-weight: var(--base-text-weight-medium, 500); - } - - &:where([data-weight='semibold']) { - font-weight: var(--base-text-weight-semibold, 600); - } - - ${sx}; -` - const Text = forwardRef(({as: Component = 'span', className, size, weight, ...props}, forwardedRef) => { - const enabled = useFeatureFlag('primer_react_css_modules_ga') - const innerRef = React.useRef(null) useRefObjectAsForwardedRef(forwardedRef, innerRef) - if (enabled) { - if (props.sx) { - return ( - // @ts-ignore shh - - ) - } - + if (props.sx) { return ( - // @ts-ignore shh - ) } return ( - // @ts-ignore shh - ) + // eslint-disable-next-line @typescript-eslint/no-explicit-any -}) as StyledComponent<'span', any, StyledTextProps, never> +}) as StyledComponent<'span', any, TextProps, never> Text.displayName = 'Text' -export type TextProps = ComponentProps export default Text From bd1d55e5b7db645e4cc3ad07b607e2bcce304169 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Thu, 3 Oct 2024 17:03:33 -0700 Subject: [PATCH 2/6] Create green-rice-clean.md --- .changeset/green-rice-clean.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/green-rice-clean.md diff --git a/.changeset/green-rice-clean.md b/.changeset/green-rice-clean.md new file mode 100644 index 00000000000..b8242120c03 --- /dev/null +++ b/.changeset/green-rice-clean.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +Remove CSS modules feature flag from Text component From f2637cba4297c5f240f1c5f3f5c5bc1e93ac1d5b Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Mon, 7 Oct 2024 13:36:25 -0700 Subject: [PATCH 3/6] Add COMMON and TYPOGRAPHY imports in Text.tsx --- packages/react/src/Text/Text.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/Text/Text.tsx b/packages/react/src/Text/Text.tsx index d822c24e6bb..5ad90d0d0de 100644 --- a/packages/react/src/Text/Text.tsx +++ b/packages/react/src/Text/Text.tsx @@ -2,6 +2,7 @@ import {clsx} from 'clsx' import {type StyledComponent} from 'styled-components' import React, {forwardRef} from 'react' import type {SystemCommonProps, SystemTypographyProps} from '../constants' +import {COMMON, TYPOGRAPHY} from '../constants' import type {SxProp} from '../sx' import Box from '../Box' import {useRefObjectAsForwardedRef} from '../hooks' From 435fe82a96e86ee9ecc08bd265320b69183709f6 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Mon, 7 Oct 2024 13:42:58 -0700 Subject: [PATCH 4/6] Rename `StyledTextProps` to `TextProps` --- packages/react/src/Text/Text.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Text/Text.tsx b/packages/react/src/Text/Text.tsx index 5ad90d0d0de..23acbefe924 100644 --- a/packages/react/src/Text/Text.tsx +++ b/packages/react/src/Text/Text.tsx @@ -20,7 +20,7 @@ export type TextProps = { const COMMON_PROP_NAMES = new Set(Object.keys(COMMON)) const TYPOGRAPHY_PROP_NAMES = new Set(Object.keys(TYPOGRAPHY)) -const includesSystemProps = (props: StyledTextProps) => { +const includesSystemProps = (props: TextProps) => { if (props.sx) { return true } From a692e45ebd5adbf937aec5bbc085c11b72604ced Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Mon, 7 Oct 2024 21:12:53 +0000 Subject: [PATCH 5/6] Update snapshots --- .../TextInputWithTokens.test.tsx.snap | 35 ++----------------- .../__snapshots__/ThemeProvider.test.tsx.snap | 35 ++----------------- 2 files changed, 4 insertions(+), 66 deletions(-) diff --git a/packages/react/src/__tests__/__snapshots__/TextInputWithTokens.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/TextInputWithTokens.test.tsx.snap index 02e48cf533e..81f7ffb53a0 100644 --- a/packages/react/src/__tests__/__snapshots__/TextInputWithTokens.test.tsx.snap +++ b/packages/react/src/__tests__/__snapshots__/TextInputWithTokens.test.tsx.snap @@ -814,39 +814,8 @@ exports[`TextInputWithTokens renders a truncated set of tokens 1`] = ` } .c9 { - font-size: 16px; color: var(--fgColor-muted,var(--color-fg-muted,#656d76)); -} - -.c9:where([data-size='small']) { - font-size: var(--text-body-size-small,0.75rem); - line-height: var(--text-body-lineHeight-small,1.6666); -} - -.c9:where([data-size='medium']) { - font-size: var(--text-body-size-medium,0.875rem); - line-height: var(--text-body-lineHeight-medium,1.4285); -} - -.c9:where([data-size='large']) { - font-size: var(--text-body-size-large,1rem); - line-height: var(--text-body-lineHeight-large,1.5); -} - -.c9:where([data-weight='light']) { - font-weight: var(--base-text-weight-light,300); -} - -.c9:where([data-weight='normal']) { - font-weight: var(--base-text-weight-normal,400); -} - -.c9:where([data-weight='medium']) { - font-weight: var(--base-text-weight-medium,500); -} - -.c9:where([data-weight='semibold']) { - font-weight: var(--base-text-weight-semibold,600); + font-size: 16px; } .c7 { @@ -1231,7 +1200,7 @@ exports[`TextInputWithTokens renders a truncated set of tokens 1`] = ` diff --git a/packages/react/src/__tests__/__snapshots__/ThemeProvider.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/ThemeProvider.test.tsx.snap index 1790814dd63..3f9f50481d2 100644 --- a/packages/react/src/__tests__/__snapshots__/ThemeProvider.test.tsx.snap +++ b/packages/react/src/__tests__/__snapshots__/ThemeProvider.test.tsx.snap @@ -2,43 +2,12 @@ exports[`has default theme 1`] = ` .c0 { - color: var(--fgColor-default,var(--color-fg-default,#1F2328)); margin-bottom: 4px; -} - -.c0:where([data-size='small']) { - font-size: var(--text-body-size-small,0.75rem); - line-height: var(--text-body-lineHeight-small,1.6666); -} - -.c0:where([data-size='medium']) { - font-size: var(--text-body-size-medium,0.875rem); - line-height: var(--text-body-lineHeight-medium,1.4285); -} - -.c0:where([data-size='large']) { - font-size: var(--text-body-size-large,1rem); - line-height: var(--text-body-lineHeight-large,1.5); -} - -.c0:where([data-weight='light']) { - font-weight: var(--base-text-weight-light,300); -} - -.c0:where([data-weight='normal']) { - font-weight: var(--base-text-weight-normal,400); -} - -.c0:where([data-weight='medium']) { - font-weight: var(--base-text-weight-medium,500); -} - -.c0:where([data-weight='semibold']) { - font-weight: var(--base-text-weight-semibold,600); + color: var(--fgColor-default,var(--color-fg-default,#1F2328)); } Hello From 73aaa8da173757869851e04380355e6a8e00880a Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Tue, 5 Nov 2024 18:09:08 +0000 Subject: [PATCH 6/6] Totally change all the typescript --- packages/react/src/Text/Text.test.tsx | 7 ++++- packages/react/src/Text/Text.tsx | 37 +++++++++++++++++---------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/packages/react/src/Text/Text.test.tsx b/packages/react/src/Text/Text.test.tsx index 190690d0c34..b1c8a5c7954 100644 --- a/packages/react/src/Text/Text.test.tsx +++ b/packages/react/src/Text/Text.test.tsx @@ -6,7 +6,12 @@ import {render as HTMLRender} from '@testing-library/react' import axe from 'axe-core' describe('Text', () => { - behavesAsComponent({Component: Text}) + behavesAsComponent({ + Component: Text, + options: { + skipDisplayName: true, + }, + }) checkExports('Text', { default: Text, diff --git a/packages/react/src/Text/Text.tsx b/packages/react/src/Text/Text.tsx index 23acbefe924..4c2bce011eb 100644 --- a/packages/react/src/Text/Text.tsx +++ b/packages/react/src/Text/Text.tsx @@ -1,6 +1,5 @@ import {clsx} from 'clsx' -import {type StyledComponent} from 'styled-components' -import React, {forwardRef} from 'react' +import React from 'react' import type {SystemCommonProps, SystemTypographyProps} from '../constants' import {COMMON, TYPOGRAPHY} from '../constants' import type {SxProp} from '../sx' @@ -8,19 +7,19 @@ import Box from '../Box' import {useRefObjectAsForwardedRef} from '../hooks' import classes from './Text.module.css' -export type TextProps = { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - as?: React.ComponentType | keyof JSX.IntrinsicElements +export type TextProps = { + as?: As size?: 'large' | 'medium' | 'small' weight?: 'light' | 'normal' | 'medium' | 'semibold' -} & SystemTypographyProps & +} & DistributiveOmit, 'as'> & + SystemTypographyProps & SystemCommonProps & SxProp const COMMON_PROP_NAMES = new Set(Object.keys(COMMON)) const TYPOGRAPHY_PROP_NAMES = new Set(Object.keys(TYPOGRAPHY)) -const includesSystemProps = (props: TextProps) => { +function includesSystemProps(props: TextProps) { if (props.sx) { return true } @@ -30,19 +29,21 @@ const includesSystemProps = (props: TextProps) => { }) } -const Text = forwardRef(({as: Component = 'span', className, size, weight, ...props}, forwardedRef) => { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function Text(props: TextProps, forwardedRef: React.ForwardedRef) { + const {as: Component = 'span', className, size, weight, ...rest} = props const innerRef = React.useRef(null) useRefObjectAsForwardedRef(forwardedRef, innerRef) // If props includes TYPOGRAPHY or COMMON props, pass them to the Box component - if (includesSystemProps(props)) { + if (includesSystemProps(rest)) { return ( ) @@ -53,14 +54,22 @@ const Text = forwardRef(({as: Component = 'span', className, size, weight, ...pr className={clsx(className, classes.Text)} data-size={size} data-weight={weight} - {...props} + {...rest} ref={innerRef} /> ) +} + +// eslint-disable-next-line @typescript-eslint/ban-types +type FixedForwardRef = ( + render: (props: P, ref: React.Ref) => React.ReactNode, +) => (props: P & React.RefAttributes) => React.ReactNode + +const fixedForwardRef = React.forwardRef as FixedForwardRef - // eslint-disable-next-line @typescript-eslint/no-explicit-any -}) as StyledComponent<'span', any, TextProps, never> +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type DistributiveOmit = T extends any ? Omit : never Text.displayName = 'Text' -export default Text +export default fixedForwardRef(Text)