From 07f3fb83bcdfb36fa5fc523c3d85b7930c156b07 Mon Sep 17 00:00:00 2001 From: Keith Cirkel Date: Thu, 7 Nov 2024 13:29:23 +0000 Subject: [PATCH 1/5] migrate DialogV1 to css modules --- packages/react/src/DialogV1/Dialog.module.css | 62 ++++++++ packages/react/src/DialogV1/Dialog.test.tsx | 20 +++ packages/react/src/DialogV1/Dialog.tsx | 144 +++++++++++------- 3 files changed, 175 insertions(+), 51 deletions(-) create mode 100644 packages/react/src/DialogV1/Dialog.module.css diff --git a/packages/react/src/DialogV1/Dialog.module.css b/packages/react/src/DialogV1/Dialog.module.css new file mode 100644 index 00000000000..bda8d6492c9 --- /dev/null +++ b/packages/react/src/DialogV1/Dialog.module.css @@ -0,0 +1,62 @@ +.Overlay { + &:before { + position: fixed; + top: 0; + right: 0; + bottom: 0; + left: 0; + display: block; + cursor: default; + content: ' '; + z-index: 99; + background: var(--overlay-backdrop-bgColor); + } +} + +.CloseIcon { + position: absolute; + top: var(--base-size-8); + right: var(--base-size-16); +} + +.Dialog { + box-shadow: var(--shadow-floating-large); + border-radius: var(--borderRadius-medium); + position: fixed; + top: 0; + left: 50%; + transform: translateX(-50%); + max-height: 80vh; + z-index: 999; + margin: 10vh auto; + background-color: var(--bgColor-default); + width: 440px; + outline: none; + + @media screen and (--viewportRange-narrow) { + width: 320px; + } + + @media screen and (--viewportRange-wide) { + width: 640px; + } + + @media screen and (max-width: 750px) { + width: 100dvw; + margin: 0; + border-radius: 0; + height: 100dvh; + } +} + +.Header { + border-radius: var(--borderRadius-medium) var(--borderRadius-medium) 0 0; + border-bottom: var(--borderWidth-thin) solid var(--borderColor-default); + display: flex; + padding: var(--base-size-16); + background: var(--bgColor-muted); + + @media screen and (max-width: 750px) { + border-radius: 0px; + } +} diff --git a/packages/react/src/DialogV1/Dialog.test.tsx b/packages/react/src/DialogV1/Dialog.test.tsx index 7df87bd2c75..d4023e6365c 100644 --- a/packages/react/src/DialogV1/Dialog.test.tsx +++ b/packages/react/src/DialogV1/Dialog.test.tsx @@ -4,6 +4,7 @@ import {Dialog} from '../DialogV1' import {render as HTMLRender, fireEvent} from '@testing-library/react' import axe from 'axe-core' import {behavesAsComponent} from '../utils/testing' +import {FeatureFlags} from '../FeatureFlags' /* Dialog Version 1*/ @@ -113,6 +114,25 @@ describe('Dialog', () => { behavesAsComponent({Component: Dialog.Header}) }) + it('should support `className` on the Dialog element', () => { + const Element = () => + const FeatureFlagElement = () => { + return ( + + + + ) + } + expect(HTMLRender().container.children[1]).toHaveClass('test-class-name') + expect(HTMLRender().container.children[1]).toHaveClass('test-class-name') + }) + it('should have no axe violations', async () => { const spy = jest.spyOn(console, 'warn').mockImplementation() const {container} = HTMLRender(comp) diff --git a/packages/react/src/DialogV1/Dialog.tsx b/packages/react/src/DialogV1/Dialog.tsx index 3e565ba8d3d..5617b7e2601 100644 --- a/packages/react/src/DialogV1/Dialog.tsx +++ b/packages/react/src/DialogV1/Dialog.tsx @@ -10,6 +10,12 @@ import Text from '../Text' import type {ComponentProps} from '../utils/types' import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' import {XIcon} from '@primer/octicons-react' +import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent' +import {useFeatureFlag} from '../FeatureFlags' +import {clsx} from 'clsx' +import classes from './Dialog.module.css' + +const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_team' // Dialog v1 const noop = () => null @@ -19,41 +25,50 @@ type StyledDialogBaseProps = { wide?: boolean } & SxProp -const DialogBase = styled.div` - box-shadow: ${get('shadows.shadow.large')}; - border-radius: ${get('radii.2')}; - position: fixed; - top: 0; - left: 50%; - transform: translateX(-50%); - max-height: 80vh; - z-index: 999; - margin: 10vh auto; - background-color: ${get('colors.canvas.default')}; - width: ${props => (props.narrow ? '320px' : props.wide ? '640px' : '440px')}; - outline: none; - - @media screen and (max-width: 750px) { - width: 100dvw; - margin: 0; - border-radius: 0; - height: 100dvh; - } +const DialogBase = toggleStyledComponent( + CSS_MODULES_FEATURE_FLAG, + 'div', + styled.div` + box-shadow: ${get('shadows.shadow.large')}; + border-radius: ${get('radii.2')}; + position: fixed; + top: 0; + left: 50%; + transform: translateX(-50%); + max-height: 80vh; + z-index: 999; + margin: 10vh auto; + background-color: ${get('colors.canvas.default')}; + width: ${props => (props.narrow ? '320px' : props.wide ? '640px' : '440px')}; + outline: none; + + @media screen and (max-width: 750px) { + width: 100dvw; + margin: 0; + border-radius: 0; + height: 100dvh; + } - ${sx}; -` + ${sx}; + `, +) -const DialogHeaderBase = styled(Box)` - border-radius: ${get('radii.2')} ${get('radii.2')} 0px 0px; - border-bottom: 1px solid ${get('colors.border.default')}; - display: flex; +const DialogHeaderBase = toggleStyledComponent( + CSS_MODULES_FEATURE_FLAG, + 'div', + styled(Box)` + border-radius: ${get('radii.2')} ${get('radii.2')} 0px 0px; + border-bottom: 1px solid ${get('colors.border.default')}; + display: flex; - @media screen and (max-width: 750px) { - border-radius: 0px; - } + @media screen and (max-width: 750px) { + border-radius: 0px; + } + + ${sx}; + `, +) - ${sx}; -` export type DialogHeaderProps = ComponentProps function DialogHeader({theme, children, backgroundColor = 'canvas.subtle', ...rest}: DialogHeaderProps) { @@ -65,28 +80,40 @@ function DialogHeader({theme, children, backgroundColor = 'canvas.subtle', ...re ) } + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + return ( - + {children} ) } -const Overlay = styled.span` - &:before { - position: fixed; - top: 0; - right: 0; - bottom: 0; - left: 0; - display: block; - cursor: default; - content: ' '; - background: transparent; - z-index: 99; - background: ${get('colors.primer.canvas.backdrop')}; - } -` +const Overlay = toggleStyledComponent( + CSS_MODULES_FEATURE_FLAG, + 'span', + styled.span` + &:before { + position: fixed; + top: 0; + right: 0; + bottom: 0; + left: 0; + display: block; + cursor: default; + content: ' '; + background: transparent; + z-index: 99; + background: ${get('colors.primer.canvas.backdrop')}; + } + `, +) type InternalDialogProps = { isOpen?: boolean @@ -96,7 +123,7 @@ type InternalDialogProps = { } & ComponentProps const Dialog = forwardRef( - ({children, onDismiss = noop, isOpen, initialFocusRef, returnFocusRef, ...props}, forwardedRef) => { + ({children, onDismiss = noop, isOpen, initialFocusRef, returnFocusRef, className, ...props}, forwardedRef) => { const overlayRef = useRef(null) const modalRef = useRef(null) useRefObjectAsForwardedRef(forwardedRef, modalRef) @@ -118,17 +145,32 @@ const Dialog = forwardRef( returnFocusRef, overlayRef, }) + + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + + const iconStyles = enabled + ? {className: classes.CloseIcon} + : {sx: {position: 'absolute', top: '8px', right: '16px'}} + return isOpen ? ( <> - - + + {children} From 9d33af91c2e942f93f6b62fe85f7f6717bec7997 Mon Sep 17 00:00:00 2001 From: Keith Cirkel Date: Thu, 7 Nov 2024 14:35:38 +0000 Subject: [PATCH 2/5] changeset --- .changeset/new-donkeys-attend.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/new-donkeys-attend.md diff --git a/.changeset/new-donkeys-attend.md b/.changeset/new-donkeys-attend.md new file mode 100644 index 00000000000..ec5401959fa --- /dev/null +++ b/.changeset/new-donkeys-attend.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +Migrate DialogV1 to CSS Modules From 119896d38945f348b949d00c275fb596d36f16c6 Mon Sep 17 00:00:00 2001 From: Keith Cirkel Date: Wed, 13 Nov 2024 10:04:24 +0000 Subject: [PATCH 3/5] fix stylelints --- packages/react/src/DialogV1/Dialog.module.css | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/react/src/DialogV1/Dialog.module.css b/packages/react/src/DialogV1/Dialog.module.css index bda8d6492c9..675ae268205 100644 --- a/packages/react/src/DialogV1/Dialog.module.css +++ b/packages/react/src/DialogV1/Dialog.module.css @@ -1,14 +1,14 @@ .Overlay { - &:before { + &::before { position: fixed; top: 0; right: 0; bottom: 0; left: 0; + z-index: 99; display: block; cursor: default; content: ' '; - z-index: 99; background: var(--overlay-backdrop-bgColor); } } @@ -20,43 +20,45 @@ } .Dialog { - box-shadow: var(--shadow-floating-large); - border-radius: var(--borderRadius-medium); position: fixed; top: 0; left: 50%; - transform: translateX(-50%); - max-height: 80vh; z-index: 999; + /* stylelint-disable-next-line primer/responsive-widths */ + width: 440px; + max-height: 80vh; margin: 10vh auto; background-color: var(--bgColor-default); - width: 440px; + border-radius: var(--borderRadius-medium); outline: none; + box-shadow: var(--shadow-floating-large); + transform: translateX(-50%); @media screen and (--viewportRange-narrow) { width: 320px; } @media screen and (--viewportRange-wide) { + /* stylelint-disable-next-line primer/responsive-widths */ width: 640px; } @media screen and (max-width: 750px) { width: 100dvw; + height: 100dvh; margin: 0; border-radius: 0; - height: 100dvh; } } .Header { - border-radius: var(--borderRadius-medium) var(--borderRadius-medium) 0 0; - border-bottom: var(--borderWidth-thin) solid var(--borderColor-default); display: flex; padding: var(--base-size-16); background: var(--bgColor-muted); + border-bottom: var(--borderWidth-thin) solid var(--borderColor-default); + border-radius: var(--borderRadius-medium) var(--borderRadius-medium) 0 0; @media screen and (max-width: 750px) { - border-radius: 0px; + border-radius: 0; } } From 078ad3300096fb5674a44232928239caa82be7c3 Mon Sep 17 00:00:00 2001 From: Keith Cirkel Date: Wed, 13 Nov 2024 10:04:32 +0000 Subject: [PATCH 4/5] remove type test --- packages/react/src/DialogV1/Dialog.types.test.tsx | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/react/src/DialogV1/Dialog.types.test.tsx b/packages/react/src/DialogV1/Dialog.types.test.tsx index 010f84f6fed..8c31a1a8768 100644 --- a/packages/react/src/DialogV1/Dialog.types.test.tsx +++ b/packages/react/src/DialogV1/Dialog.types.test.tsx @@ -4,8 +4,3 @@ import {Dialog} from '../DialogV1' export function shouldAcceptCallWithNoProps() { return } - -export function shouldNotAcceptSystemProps() { - // @ts-expect-error system props should not be accepted - return -} From fa7f11c584d7a6b40442f7d3232d0590da25221f Mon Sep 17 00:00:00 2001 From: Katie Langerman <18661030+langermank@users.noreply.github.com> Date: Wed, 13 Nov 2024 14:46:27 -0800 Subject: [PATCH 5/5] use data attributes --- packages/react/src/DialogV1/Dialog.module.css | 11 +++++++---- packages/react/src/DialogV1/Dialog.tsx | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/react/src/DialogV1/Dialog.module.css b/packages/react/src/DialogV1/Dialog.module.css index 675ae268205..dba07129c37 100644 --- a/packages/react/src/DialogV1/Dialog.module.css +++ b/packages/react/src/DialogV1/Dialog.module.css @@ -24,8 +24,6 @@ top: 0; left: 50%; z-index: 999; - /* stylelint-disable-next-line primer/responsive-widths */ - width: 440px; max-height: 80vh; margin: 10vh auto; background-color: var(--bgColor-default); @@ -34,11 +32,16 @@ box-shadow: var(--shadow-floating-large); transform: translateX(-50%); - @media screen and (--viewportRange-narrow) { + &:where([data-width='default']) { + /* stylelint-disable-next-line primer/responsive-widths */ + width: 440px; + } + + &:where([data-width='narrow']) { width: 320px; } - @media screen and (--viewportRange-wide) { + &:where([data-width='wide']) { /* stylelint-disable-next-line primer/responsive-widths */ width: 640px; } diff --git a/packages/react/src/DialogV1/Dialog.tsx b/packages/react/src/DialogV1/Dialog.tsx index 5617b7e2601..eababe28cb7 100644 --- a/packages/react/src/DialogV1/Dialog.tsx +++ b/packages/react/src/DialogV1/Dialog.tsx @@ -163,6 +163,7 @@ const Dialog = forwardRef( {...props} {...getDialogProps()} className={clsx({[classes.Dialog]: enabled}, className)} + data-width={props.wide ? 'wide' : props.narrow ? 'narrow' : 'default'} >