Merge f810f76 into 928bfd2

primer · Sep 20, 2024 · dd66f07 · dd66f07
2 parents 928bfd2 + f810f76
commit dd66f07
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 2,746 deletions.
diff --git a/.changeset/fuzzy-weeks-peel.md b/.changeset/fuzzy-weeks-peel.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': major
+---
+
+Popover: Remove `caret` from Popover
diff --git a/packages/react/src/Popover/Popover.docs.json b/packages/react/src/Popover/Popover.docs.json
@@ -12,12 +12,6 @@
       "defaultValue": "div",
       "description": "Sets the underlying HTML tag for the component"
     },
-    {
-      "name": "caret",
-      "type": "| 'top' | 'bottom' | 'left' | 'right' | 'bottom-left' | 'bottom-right' | 'top-left' | 'top-right' | 'left-bottom' | 'left-top' | 'right-bottom' | 'right-top' ",
-      "defaultValue": "'top'",
-      "description": "Controls the position of the caret"
-    },
     {
       "name": "open",
       "type": "boolean",

diff --git a/packages/react/src/Popover/Popover.figma.tsx b/packages/react/src/Popover/Popover.figma.tsx
@@ -15,23 +15,9 @@ figma.connect(Popover, 'https://www.figma.com/design/GCvY3Qv8czRgZgvl1dG6lp/Prim
     body: figma.string('body'),
     heading: figma.string('heading'),
     action: figma.children('Action'),
-    caret: figma.enum('caret', {
-      top: 'top',
-      'top-left': 'top-left',
-      'top-right': 'top-right',
-      bottom: 'bottom',
-      'bottom-left': 'bottom-left',
-      'bottom-right': 'bottom-right',
-      left: 'left',
-      'left-top': 'left-top',
-      'left-bottom': 'left-bottom',
-      right: 'right',
-      'right-top': 'right-top',
-      'right-bottom': 'right-bottom',
-    }),
   },
-  example: ({caret, heading, body, action}) => (
-    <Popover caret={caret}>
+  example: ({heading, body, action}) => (
+    <Popover>
       <Popover.Content>
         <Heading
           sx={{

diff --git a/packages/react/src/Popover/Popover.stories.tsx b/packages/react/src/Popover/Popover.stories.tsx
@@ -11,7 +11,7 @@ export default {
 } as Meta<typeof Popover>
 
 export const Default = () => (
-  <Popover relative open={true} caret="top">
+  <Popover relative open={true}>
     <Popover.Content sx={{marginTop: 2}}>
       <Heading sx={{fontSize: 2}}>Popover heading</Heading>
       <Text as="p">Message about popovers</Text>
@@ -31,31 +31,11 @@ export const Playground: StoryFn<typeof Popover> = args => (
 )
 
 Playground.args = {
-  caret: 'top',
   open: true,
   relative: true,
 }
 
 Playground.argTypes = {
-  caret: {
-    control: {
-      type: 'radio',
-    },
-    options: [
-      'top',
-      'bottom',
-      'left',
-      'right',
-      'bottom-left',
-      'bottom-right',
-      'top-left',
-      'top-right',
-      'left-bottom',
-      'left-top',
-      'right-bottom',
-      'right-top',
-    ],
-  },
   open: {
     control: {
       type: 'boolean',

diff --git a/packages/react/src/Popover/Popover.tsx b/packages/react/src/Popover/Popover.tsx
@@ -1,36 +1,17 @@
-import {clsx} from 'clsx'
 import styled from 'styled-components'
 import {get} from '../constants'
 import type {SxProp} from '../sx'
 import sx from '../sx'
 import type {ComponentProps} from '../utils/types'
 
-type CaretPosition =
-  | 'top'
-  | 'bottom'
-  | 'left'
-  | 'right'
-  | 'bottom-left'
-  | 'bottom-right'
-  | 'top-left'
-  | 'top-right'
-  | 'left-bottom'
-  | 'left-top'
-  | 'right-bottom'
-  | 'right-top'
-
 type StyledPopoverProps = {
-  /**
-   * @deprecated `caret` is deprecated and will be removed in v38.
-   */
-  caret?: CaretPosition
   relative?: boolean
   open?: boolean
 } & SxProp
 
-const Popover = styled.div.attrs<StyledPopoverProps>(({className, caret = 'top'}) => {
+const Popover = styled.div.attrs<StyledPopoverProps>(({className}) => {
   return {
-    className: clsx(className, `caret-pos--${caret}`),
+    className,
   }
 })<StyledPopoverProps>`
   position: ${props => (props.relative ? 'relative' : 'absolute')};
@@ -48,173 +29,6 @@ const PopoverContent = styled.div<SxProp>`
   margin-left: auto;
   padding: ${get('space.4')};
   background-color: ${get('colors.canvas.overlay')};
-
-  // Carets
-  &::before,
-  &::after {
-    position: absolute;
-    left: 50%;
-    display: inline-block;
-    content: '';
-  }
-
-  &::before {
-    top: -${get('space.3')};
-    margin-left: -9px;
-    border: ${get('space.2')} solid transparent; // TODO: solid?
-    border-bottom-color: ${get('colors.border.default')};
-  }
-
-  &::after {
-    top: -14px;
-    margin-left: -${get('space.2')};
-    border: 7px solid transparent; // todo: solid
-    border-bottom-color: ${get('colors.canvas.overlay')};
-  }
-
-  // Bottom-oriented carets
-  ${Popover}.caret-pos--bottom & ,
-  ${Popover}.caret-pos--bottom-right & ,
-  ${Popover}.caret-pos--bottom-left & {
-    &::before,
-    &::after {
-      top: auto;
-      border-bottom-color: transparent;
-    }
-
-    &::before {
-      bottom: -${get('space.3')};
-      border-top-color: ${get('colors.border.default')};
-    }
-
-    &::after {
-      bottom: -14px;
-      // stylelint-disable-next-line primer/borders
-      border-top-color: ${get('colors.canvas.overlay')};
-    }
-  }
-
-  // Top & Bottom: Right-oriented carets
-  ${Popover}.caret-pos--top-right & ,
-  ${Popover}.caret-pos--bottom-right & {
-    right: -9px;
-    margin-right: 0;
-
-    &::before,
-    &::after {
-      left: auto;
-      margin-left: 0;
-    }
-
-    &::before {
-      right: 20px;
-    }
-
-    &::after {
-      right: 21px;
-    }
-  }
-
-  // Top & Bottom: Left-oriented carets
-  ${Popover}.caret-pos--top-left & ,
-  ${Popover}.caret-pos--bottom-left & {
-    left: -9px;
-    margin-left: 0;
-
-    &::before,
-    &::after {
-      left: ${get('space.4')};
-      margin-left: 0;
-    }
-
-    &::after {
-      left: calc(${get('space.4')} + 1px);
-    }
-  }
-
-  // Right- & Left-oriented carets
-  ${Popover}.caret-pos--right & ,
-  ${Popover}.caret-pos--right-top & ,
-  ${Popover}.caret-pos--right-bottom & ,
-  ${Popover}.caret-pos--left & ,
-  ${Popover}.caret-pos--left-top & ,
-  ${Popover}.caret-pos--left-bottom & {
-    &::before,
-    &::after {
-      top: 50%;
-      left: auto;
-      margin-left: 0;
-      border-bottom-color: transparent;
-    }
-
-    &::before {
-      // stylelint-disable-next-line primer/spacing
-      margin-top: calc((${get('space.2')} + 1px) * -1);
-    }
-
-    &::after {
-      margin-top: -${get('space.2')};
-    }
-  }
-
-  // Right-oriented carets
-  ${Popover}.caret-pos--right & ,
-  ${Popover}.caret-pos--right-top & ,
-  ${Popover}.caret-pos--right-bottom & {
-    &::before {
-      right: -${get('space.3')};
-      border-left-color: ${get('colors.border.default')};
-    }
-
-    &::after {
-      right: -14px;
-      // stylelint-disable-next-line primer/borders
-      border-left-color: ${get('colors.canvas.overlay')};
-    }
-  }
-
-  // Left-oriented carets
-  ${Popover}.caret-pos--left & ,
-  ${Popover}.caret-pos--left-top & ,
-  ${Popover}.caret-pos--left-bottom & {
-    &::before {
-      left: -${get('space.3')};
-      border-right-color: ${get('colors.border.default')};
-    }
-
-    &::after {
-      left: -14px;
-      // stylelint-disable-next-line primer/borders
-      border-right-color: ${get('colors.canvas.overlay')};
-    }
-  }
-
-  // Right & Left: Top-oriented carets
-  ${Popover}.caret-pos--right-top & ,
-  ${Popover}.caret-pos--left-top & {
-    &::before,
-    &::after {
-      top: ${get('space.4')};
-    }
-  }
-
-  // Right & Left: Bottom-oriented carets
-  ${Popover}.caret-pos--right-bottom & ,
-  ${Popover}.caret-pos--left-bottom & {
-    &::before,
-    &::after {
-      top: auto;
-    }
-
-    &::before {
-      bottom: ${get('space.3')};
-    }
-
-    &::after {
-      bottom: calc(${get('space.3')} + 1px);
-    }
-  }
-
   ${sx};
 `
 

diff --git a/packages/react/src/__tests__/Popover.test.tsx b/packages/react/src/__tests__/Popover.test.tsx
@@ -1,12 +1,11 @@
 import React from 'react'
-import type {PopoverProps} from '../Popover'
 import Popover from '../Popover'
 import {render, behavesAsComponent, checkExports} from '../utils/testing'
 import {render as HTMLRender} from '@testing-library/react'
 import axe from 'axe-core'
 
 const comp = (
-  <Popover caret="top" open>
+  <Popover open>
     <Popover.Content>Hello!</Popover.Content>
   </Popover>
 )
@@ -24,41 +23,14 @@ describe('Popover', () => {
 
   it('should have no axe violations', async () => {
     const {container} = HTMLRender(
-      <Popover caret="top" open>
+      <Popover open>
         <Popover.Content>Hello!</Popover.Content>
       </Popover>,
     )
     const results = await axe.run(container)
     expect(results).toHaveNoViolations()
   })
 
-  const CARET_POSITIONS: PopoverProps['caret'][] = [
-    'top',
-    'bottom',
-    'left',
-    'right',
-    'bottom-left',
-    'bottom-right',
-    'top-left',
-    'top-right',
-    'left-bottom',
-    'left-top',
-    'right-bottom',
-    'right-top',
-  ]
-
-  for (const pos of CARET_POSITIONS) {
-    it(`renders correctly for a caret position of ${pos}`, () => {
-      const element = (
-        <Popover caret={pos} open>
-          <Popover.Content>Hello!</Popover.Content>
-        </Popover>
-      )
-
-      expect(render(element)).toMatchSnapshot()
-    })
-  }
-
   it('renders both elements as a <div>', () => {
     expect(render(<Popover />).type).toEqual('div')
     expect(render(<Popover.Content />).type).toEqual('div')