[Dialog] Replace deprecated Button for current one (#3084)

* feat(Dialog): use new `Button` + change `DialogButtonProps` to match it + change `autoFocus` logic so first `button` gets focus when navigating with tabs * fix(Dialog): rollback change on `focus` functionality * fix(Dialog): add support to deprecated `normal` `buttonType` * fix(Dialog): override `ButtonProps` `children` * chore(tests): update snapshots * chore(Dialog): changeset * chore(tests): update snapshots * chore(ConfirmationDialog): update tests and snapshot * Update src/__tests__/ConfirmationDialog.test.tsx Co-authored-by: Armağan <broccolinisoup@github.com> * import ActionList and remove unused buttonRef and extra lines * fix the import * Add changed components to .changeset/cool-schools-cheer.md --------- Co-authored-by: Armağan <broccolinisoup@github.com> Co-authored-by: Mike Perrotti <mperrotti@github.com>
primer · Jul 12, 2023 · ded74d8 · ded74d8
1 parent 90a145c
commit ded74d8
Show file tree

Hide file tree

Showing 8 changed files with 263 additions and 96 deletions.
diff --git a/.changeset/cool-schools-cheer.md b/.changeset/cool-schools-cheer.md
@@ -0,0 +1,6 @@
+---
+'@primer/react': minor
+---
+
+[Dialog] Replaces deprecated button for current one
+<!-- Changed components: Dialog-->
diff --git a/src/Dialog/Dialog.tsx b/src/Dialog/Dialog.tsx
@@ -1,6 +1,6 @@
 import React, {useCallback, useEffect, useRef, useState} from 'react'
 import styled from 'styled-components'
-import Button, {ButtonPrimary, ButtonDanger, ButtonProps} from '../deprecated/Button'
+import {Button, ButtonProps} from '../Button'
 import Box from '../Box'
 import {get} from '../constants'
 import {useOnEscapePress, useProvidedRefOrCreate} from '../hooks'
@@ -20,11 +20,11 @@ const ANIMATION_DURATION = '200ms'
  * Props that characterize a button to be rendered into the footer of
  * a Dialog.
  */
-export type DialogButtonProps = ButtonProps & {
+export type DialogButtonProps = Omit<ButtonProps, 'children'> & {
   /**
    * The type of Button element to use
    */
-  buttonType?: 'normal' | 'primary' | 'danger'
+  buttonType?: 'default' | 'primary' | 'danger' | 'normal'
 
   /**
    * The Button's inner text
@@ -366,11 +366,6 @@ const Footer = styled.div<SxProp>`
   ${sx};
 `
 
-const buttonTypes = {
-  normal: Button,
-  primary: ButtonPrimary,
-  danger: ButtonDanger,
-}
 const Buttons: React.FC<React.PropsWithChildren<{buttons: DialogButtonProps[]}>> = ({buttons}) => {
   const autoFocusRef = useProvidedRefOrCreate<HTMLButtonElement>(buttons.find(button => button.autoFocus)?.ref)
   let autoFocusCount = 0
@@ -387,17 +382,16 @@ const Buttons: React.FC<React.PropsWithChildren<{buttons: DialogButtonProps[]}>>
   return (
     <>
       {buttons.map((dialogButtonProps, index) => {
-        const {content, buttonType = 'normal', autoFocus = false, ...buttonProps} = dialogButtonProps
-        const ButtonElement = buttonTypes[buttonType]
+        const {content, buttonType = 'default', autoFocus = false, ...buttonProps} = dialogButtonProps
         return (
-          <ButtonElement
+          <Button
             key={index}
             {...buttonProps}
-            variant={buttonType}
+            variant={buttonType === 'normal' ? 'default' : buttonType}
             ref={autoFocus && autoFocusCount === 0 ? (autoFocusCount++, autoFocusRef) : null}
           >
             {content}
-          </ButtonElement>
+          </Button>
         )
       })}
     </>

diff --git a/src/SelectPanel/__snapshots__/SelectPanel.test.tsx.snap b/src/SelectPanel/__snapshots__/SelectPanel.test.tsx.snap
@@ -7,6 +7,10 @@ exports[`SelectPanel renders consistently 1`] = `
   color: #1F2328;
 }
 
+.c2 {
+  margin-left: 4px;
+}
+
 .c1 {
   position: relative;
   display: inline-block;
@@ -71,10 +75,6 @@ exports[`SelectPanel renders consistently 1`] = `
   border-color: rgba(31,35,40,0.15);
 }
 
-.c2 {
-  margin-left: 4px;
-}
-
 <div
   className="c0"
   color="fg.default"

diff --git a/src/__tests__/ConfirmationDialog.test.tsx b/src/__tests__/ConfirmationDialog.test.tsx
@@ -2,10 +2,11 @@ import {render as HTMLRender, fireEvent} from '@testing-library/react'
 import {axe} from 'jest-axe'
 import React, {useCallback, useRef, useState} from 'react'
 
-import {ActionMenu} from '../deprecated/ActionMenu'
+import {ActionMenu} from '../ActionMenu'
+import {ActionList} from '../ActionList'
 import BaseStyles from '../BaseStyles'
 import Box from '../Box'
-import Button from '../deprecated/Button/Button'
+import {Button} from '../Button'
 import {ConfirmationDialog, useConfirm} from '../Dialog/ConfirmationDialog'
 import theme from '../theme'
 import {ThemeProvider} from '../ThemeProvider'
@@ -62,10 +63,14 @@ const ShorthandHookFromActionMenu = () => {
       <SSRProvider>
         <BaseStyles>
           <Box display="flex" flexDirection="column" alignItems="flex-start">
-            <ActionMenu
-              renderAnchor={props => <Button {...props}>{text}</Button>}
-              items={[{text: 'Show dialog', onAction: onButtonClick}]}
-            />
+            <ActionMenu>
+              <ActionMenu.Button>{text}</ActionMenu.Button>
+              <ActionMenu.Overlay>
+                <ActionList>
+                  <ActionList.Item onSelect={onButtonClick}>Show dialog</ActionList.Item>
+                </ActionList>
+              </ActionMenu.Overlay>
+            </ActionMenu>
           </Box>
         </BaseStyles>
       </SSRProvider>
@@ -95,36 +100,36 @@ describe('ConfirmationDialog', () => {
   })
 
   it('focuses the primary action when opened and the confirmButtonType is not set', async () => {
-    const {getByText} = HTMLRender(<Basic />)
+    const {getByText, getByRole} = HTMLRender(<Basic />)
     fireEvent.click(getByText('Show dialog'))
-    expect(getByText('Primary')).toEqual(document.activeElement)
+    expect(getByRole('button', {name: 'Primary'})).toEqual(document.activeElement)
     expect(getByText('Secondary')).not.toEqual(document.activeElement)
   })
 
   it('focuses the primary action when opened and the confirmButtonType is not danger', async () => {
-    const {getByText} = HTMLRender(<Basic confirmButtonType="primary" />)
+    const {getByText, getByRole} = HTMLRender(<Basic confirmButtonType="primary" />)
     fireEvent.click(getByText('Show dialog'))
-    expect(getByText('Primary')).toEqual(document.activeElement)
+    expect(getByRole('button', {name: 'Primary'})).toEqual(document.activeElement)
     expect(getByText('Secondary')).not.toEqual(document.activeElement)
   })
 
   it('focuses the secondary action when opened and the confirmButtonType is danger', async () => {
-    const {getByText} = HTMLRender(<Basic confirmButtonType="danger" />)
+    const {getByText, getByRole} = HTMLRender(<Basic confirmButtonType="danger" />)
     fireEvent.click(getByText('Show dialog'))
-    expect(getByText('Primary')).not.toEqual(document.activeElement)
-    expect(getByText('Secondary')).toEqual(document.activeElement)
+    expect(getByRole('button', {name: 'Primary'})).not.toEqual(document.activeElement)
+    expect(getByRole('button', {name: 'Secondary'})).toEqual(document.activeElement)
   })
 
   it('supports nested `focusTrap`s', async () => {
     const spy = jest.spyOn(console, 'error').mockImplementationOnce(() => {})
 
-    const {getByText} = HTMLRender(<ShorthandHookFromActionMenu />)
+    const {getByText, getByRole} = HTMLRender(<ShorthandHookFromActionMenu />)
 
     fireEvent.click(getByText('Show menu'))
     fireEvent.click(getByText('Show dialog'))
 
-    expect(getByText('Primary')).toEqual(document.activeElement)
-    expect(getByText('Secondary')).not.toEqual(document.activeElement)
+    expect(getByRole('button', {name: 'Primary'})).toEqual(document.activeElement)
+    expect(getByRole('button', {name: 'Secondary'})).not.toEqual(document.activeElement)
 
     // REACT_VERSION_LATEST should be treated as a constant for the test
     // environment

diff --git a/src/__tests__/__snapshots__/AnchoredOverlay.test.tsx.snap b/src/__tests__/__snapshots__/AnchoredOverlay.test.tsx.snap
@@ -100,6 +100,25 @@ exports[`AnchoredOverlay should render consistently when open 1`] = `
   color: #1F2328;
 }
 
+.c2 {
+  background-color: #ffffff;
+  box-shadow: 0 1px 3px rgba(31,35,40,0.12),0 8px 24px rgba(66,74,83,0.12);
+  position: absolute;
+  min-width: 192px;
+  max-width: 640px;
+  height: auto;
+  width: auto;
+  border-radius: 12px;
+  overflow: hidden;
+  -webkit-animation: overlay-appear 200ms cubic-bezier(0.33,1,0.68,1);
+  animation: overlay-appear 200ms cubic-bezier(0.33,1,0.68,1);
+  visibility: var(--styled-overlay-visibility);
+}
+
+.c2:focus {
+  outline: none;
+}
+
 .c1 {
   position: relative;
   display: inline-block;
@@ -164,25 +183,6 @@ exports[`AnchoredOverlay should render consistently when open 1`] = `
   border-color: rgba(31,35,40,0.15);
 }
 
-.c2 {
-  background-color: #ffffff;
-  box-shadow: 0 1px 3px rgba(31,35,40,0.12),0 8px 24px rgba(66,74,83,0.12);
-  position: absolute;
-  min-width: 192px;
-  max-width: 640px;
-  height: auto;
-  width: auto;
-  border-radius: 12px;
-  overflow: hidden;
-  -webkit-animation: overlay-appear 200ms cubic-bezier(0.33,1,0.68,1);
-  animation: overlay-appear 200ms cubic-bezier(0.33,1,0.68,1);
-  visibility: var(--styled-overlay-visibility);
-}
-
-.c2:focus {
-  outline: none;
-}
-
 @media (forced-colors:active) {
   .c2 {
     outline: solid 1px transparent;