From e74cfc39dd9f781b67fd918f6066825d62ccb299 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 5 May 2025 15:05:47 -0500 Subject: [PATCH 1/3] refactor(Flash): update sx usage to CSS Modules --- .changeset/giant-bushes-matter.md | 5 ++ packages/react/src/Flash/Flash.module.css | 59 +++++++++++++++ packages/react/src/Flash/Flash.tsx | 87 +++++------------------ 3 files changed, 82 insertions(+), 69 deletions(-) create mode 100644 .changeset/giant-bushes-matter.md create mode 100644 packages/react/src/Flash/Flash.module.css diff --git a/.changeset/giant-bushes-matter.md b/.changeset/giant-bushes-matter.md new file mode 100644 index 00000000000..6d7dd23f819 --- /dev/null +++ b/.changeset/giant-bushes-matter.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Update Flash from sx to CSS Modules diff --git a/packages/react/src/Flash/Flash.module.css b/packages/react/src/Flash/Flash.module.css new file mode 100644 index 00000000000..1ce51e36c29 --- /dev/null +++ b/packages/react/src/Flash/Flash.module.css @@ -0,0 +1,59 @@ +.Flash { + position: relative; + color: var(--fgColor-default); + padding: var(--base-size-16); + border-style: solid; + border-width: 1px; + border-radius: var(--borderRadius-medium); + margin-top: 0; + + &:where([data-variant='default']) { + background-color: var(--bgColor-accent-muted); + border-color: var(--borderColor-accent-muted); + + & :where(svg) { + color: var(--fgColor-accent); + } + } + + &:where([data-variant='success']) { + background-color: var(--bgColor-success-muted); + border-color: var(--borderColor-success-muted); + + & :where(svg) { + color: var(--fgColor-success); + } + } + + &:where([data-variant='danger']) { + background-color: var(--bgColor-danger-muted); + border-color: var(--borderColor-danger-muted); + + & :where(svg) { + color: var(--fgColor-danger); + } + } + + &:where([data-variant='warning']) { + background-color: var(--bgColor-attention-muted); + border-color: var(--borderColor-attention-muted); + + & :where(svg) { + color: var(--fgColor-attention); + } + } + + &:where([data-full]) { + border-width: 1px 0px; + border-radius: 0; + margin-top: -1px; + } + + & :where(p:last-child) { + margin-bottom: 0; + } + + & :where(svg) { + margin-right: var(--base-size-8); + } +} diff --git a/packages/react/src/Flash/Flash.tsx b/packages/react/src/Flash/Flash.tsx index 8701f8ea8d9..557f0e33cce 100644 --- a/packages/react/src/Flash/Flash.tsx +++ b/packages/react/src/Flash/Flash.tsx @@ -1,80 +1,29 @@ +import {clsx} from 'clsx' import React from 'react' -import styled from 'styled-components' -import {variant} from 'styled-system' -import {get} from '../constants' import type {SxProp} from '../sx' -import sx from '../sx' -import type {ComponentProps} from '../utils/types' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' +import {BoxWithFallback} from '../internal/components/BoxWithFallback' +import classes from './Flash.module.css' -const variants = variant({ - variants: { - default: { - color: 'fg.default', - backgroundColor: 'accent.subtle', - borderColor: 'accent.muted', - svg: { - color: 'accent.fg', - }, - }, - success: { - color: 'fg.default', - backgroundColor: 'success.subtle', - borderColor: 'success.muted', - svg: { - color: 'success.fg', - }, - }, - danger: { - color: 'fg.default', - backgroundColor: 'danger.subtle', - borderColor: 'danger.muted', - svg: { - color: 'danger.fg', - }, - }, - warning: { - color: 'fg.default', - backgroundColor: 'attention.subtle', - borderColor: 'attention.muted', - svg: { - color: 'attention.fg', - }, - }, - }, -}) - -type StyledFlashProps = { +export type FlashProps = React.ComponentPropsWithoutRef<'div'> & { + className?: string variant?: 'default' | 'warning' | 'success' | 'danger' full?: boolean } & SxProp -const StyledFlash = styled.div` - position: relative; - color: ${get('colors.fg.default')}; - padding: ${get('space.3')}; - border-style: solid; - border-width: ${props => (props.full ? '1px 0px' : '1px')}; - border-radius: ${props => (props.full ? '0' : get('radii.2'))}; - margin-top: ${props => (props.full ? '-1px' : '0')}; - - p:last-child { - margin-bottom: 0; - } - - svg { - margin-right: ${get('space.2')}; - } - - ${variants}; - ${sx}; -` - -export type FlashProps = ComponentProps - -const Flash = React.forwardRef(function Flash({as, variant = 'default', ...rest}, ref) { - return -}) as PolymorphicForwardRefComponent<'div', StyledFlashProps> +const Flash = React.forwardRef(function Flash({as, className, variant = 'default', full, sx, ...rest}, ref) { + return ( + + ) +}) as PolymorphicForwardRefComponent<'div', FlashProps> if (__DEV__) { Flash.displayName = 'Flash' From b0aa48a23c5ac2ec8a745230ce3305c9c66e2e97 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 5 May 2025 16:03:03 -0500 Subject: [PATCH 2/3] test: update tests --- .../react/src/Flash/__tests__/Flash.test.tsx | 62 +++++++++---------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/packages/react/src/Flash/__tests__/Flash.test.tsx b/packages/react/src/Flash/__tests__/Flash.test.tsx index d6a118f8823..ec838d5a256 100644 --- a/packages/react/src/Flash/__tests__/Flash.test.tsx +++ b/packages/react/src/Flash/__tests__/Flash.test.tsx @@ -1,44 +1,42 @@ import React from 'react' import Flash from '..' -import {render, behavesAsComponent, checkExports} from '../../utils/testing' -import {render as HTMLRender} from '@testing-library/react' -import axe from 'axe-core' +import {render, screen} from '@testing-library/react' describe('Flash', () => { - behavesAsComponent({Component: Flash}) - - checkExports('Flash', { - default: Flash, + it('should support the `full` prop', () => { + render( + <> + + + , + ) + expect(screen.getByTestId('full')).toHaveAttribute('data-full', '') + expect(screen.getByTestId('no-full')).not.toHaveAttribute('data-full') }) - it('should have no axe violations', async () => { - const {container} = HTMLRender() - const results = await axe.run(container) - expect(results).toHaveNoViolations() + it('should support the `variant` prop', () => { + render( + <> + + + + + , + ) + + expect(screen.getByTestId('danger')).toHaveAttribute('data-variant', 'danger') + expect(screen.getByTestId('success')).toHaveAttribute('data-variant', 'success') + expect(screen.getByTestId('warning')).toHaveAttribute('data-variant', 'warning') + expect(screen.getByTestId('default')).toHaveAttribute('data-variant', 'default') }) - it('respects the "full" prop', () => { - expect(render()).toHaveStyleRule('margin-top', '-1px') - expect(render()).toHaveStyleRule('border-radius', '0') - expect(render()).toHaveStyleRule('border-width', '1px 0px') + it('should support `className` on the outermost element', () => { + const {container} = render() + expect(container.firstChild).toHaveClass('test-class') }) - it('respects the "variant" prop', () => { - expect(render()).toHaveStyleRule( - 'background-color', - 'var(--bgColor-attention-muted,var(--color-attention-subtle,#fff8c5))', - ) - expect(render()).toHaveStyleRule( - 'background-color', - 'var(--bgColor-danger-muted,var(--color-danger-subtle,#ffebe9))', - ) - expect(render()).toHaveStyleRule( - 'background-color', - 'var(--bgColor-success-muted,var(--color-success-subtle,#dafbe1))', - ) - expect(render()).toHaveStyleRule( - 'background-color', - 'var(--bgColor-accent-muted,var(--color-accent-subtle,#ddf4ff))', - ) + it('should spread props to the outermost element', () => { + const {container} = render() + expect(container.firstChild).toHaveAttribute('data-testid', 'test') }) }) From 85067e1f3cf02f4c1f20d7cfeca86062dab4d62a Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 6 May 2025 13:36:51 -0500 Subject: [PATCH 3/3] fix: stylelint errors --- packages/react/src/Flash/Flash.module.css | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/react/src/Flash/Flash.module.css b/packages/react/src/Flash/Flash.module.css index 1ce51e36c29..c16e839a4e3 100644 --- a/packages/react/src/Flash/Flash.module.css +++ b/packages/react/src/Flash/Flash.module.css @@ -1,11 +1,11 @@ .Flash { position: relative; - color: var(--fgColor-default); padding: var(--base-size-16); + margin-top: 0; + color: var(--fgColor-default); border-style: solid; - border-width: 1px; + border-width: var(--borderWidth-thin); border-radius: var(--borderRadius-medium); - margin-top: 0; &:where([data-variant='default']) { background-color: var(--bgColor-accent-muted); @@ -44,9 +44,10 @@ } &:where([data-full]) { - border-width: 1px 0px; - border-radius: 0; + /* stylelint-disable-next-line primer/spacing */ margin-top: -1px; + border-width: var(--borderWidth-thin) 0; + border-radius: 0; } & :where(p:last-child) {