Fixes PointerBox caret not styling correctly using sx props

.changeset/thick-parents-wonder.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Fixes bug in PointerBox component where caret doesn't inherit correct styling. Backwards compataible with previous API.

docs/content/PointerBox.md

@@ -1,19 +1,21 @@
 ---
-componentId: pointer_box
 title: PointerBox
+description: A customisable, bordered Box with a caret pointer
+componentId: pointer_box
 status: Alpha
+source: https://github.com/primer/react/blob/main/src/PointerBox.tsx
 ---
 
-PointerBox is a [BorderBox](./BorderBox) component with a caret added to it.
-
-## Default example
+## Examples
 
 ```jsx live
-<PointerBox m={4} p={2} minHeight={100} bg="success.subtle" borderColor="success.emphasis">
+<PointerBox minHeight={100} sx={{m: 4, p: 2, bg: 'success.subtle', borderColor: 'success.emphasis'}}>
   PointerBox
 </PointerBox>
 ```
 
+### Caret position
+
 ```javascript live noinline
 function PointerBoxDemo(props) {
   const [pos, setPos] = React.useState('top')
@@ -24,10 +26,13 @@ function PointerBoxDemo(props) {
         Caret Position
       </Heading>
       <CaretSelector current={pos} onChange={setPos} />
-      <Box position="relative" pt={4}>
-        <PointerBox m={4} p={2} minHeight={100} bg="success.subtle" borderColor="success.emphasis" caret={pos}>
-          {' '}
-          Content{' '}
+      <Box position="relative">
+        <PointerBox
+          minHeight={100}
+          caret={pos}
+          sx={{m: 4, p: 2, bg: 'success.subtle', borderColor: 'success.emphasis'}}
+        >
+          Content
         </PointerBox>
       </Box>
     </Box>
@@ -49,9 +54,8 @@ function CaretSelector(props) {
     'top-right',
     'bottom-right'
   ].map(dir => (
-    <label>
+    <label key={dir}>
       <input
-        key={dir}
         type="radio"
         name="caret"
         value={dir}
@@ -72,12 +76,48 @@ function CaretSelector(props) {
 render(<PointerBoxDemo />)
 ```
 
-## System props
+## Props
+
+<PropsTable>
+  <PropsTableRow
+    name="caret"
+    type={`| 'top'
+| 'top-left'
+| 'top-right'
+| 'right'
+| 'right-top'
+| 'right-bottom'
+| 'bottom'
+| 'bottom-left'
+| 'bottom-right'
+| 'left'
+| 'left-top'
+| 'left-bottom'`}
+    defaultValue="'bottom'"
+    description="Sets the location of the caret. The format is [edge]-[position on edge]. For example, right-top will position the caret on the top of the right edge of the box. Use top"
+  />
+</PropsTable>
+
+## Status
 
-PointerBox components get `COMMON`, `LAYOUT`, `BORDER`, and `FLEX` system props. Read our [System Props](/system-props) doc page for a full list of available props.
+<ComponentChecklist
+items={{
+    propsDocumented: true,
+    noUnnecessaryDeps: true,
+    adaptsToThemes: true,
+    adaptsToScreenSizes: true,
+    fullTestCoverage: true,
+    usedInProduction: false,
+    usageExamplesDocumented: false,
+    designReviewed: false,
+    a11yReviewed: false,
+    stableApi: false,
+    addressedApiFeedback: false,
+    hasDesignGuidelines: false,
+    hasFigmaComponent: false
+  }}
+/>
 
-## Component props
+## Related components
 
-| Name  | Type   | Default | Description                                                                                                                                                                                                                                                  |
-| :---- | :----- | :-----: | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| caret | String | bottom  | Sets the location of the caret. The format is `[edge]-[position on edge]`. For example, `right-top` will position the caret on the top of the right edge of the box. Use `top`, `right`, `bottom`, or `left` to position a caret in the center of that edge. |
+- [Popover](/Popover)

src/PointerBox.tsx

@@ -1,30 +1,45 @@
 import React from 'react'
-import BorderBox, {BorderBoxProps} from './BorderBox'
+import Box, {BoxProps} from './Box'
 import Caret, {CaretProps} from './Caret'
+import {SxProp} from './sx'
+
+// FIXME: Make this work with BetterStyledSystem types
+type MutatedSxProps = {
+  sx?: {
+    bg?: string
+    backgroundColor?: string
+    borderColor?: string
+  }
+} & SxProp
 
 export type PointerBoxProps = {
   caret?: CaretProps['location']
   bg?: CaretProps['bg']
   borderColor?: CaretProps['borderColor']
   border?: CaretProps['borderWidth']
-} & BorderBoxProps
+} & BoxProps &
+  MutatedSxProps
 
 function PointerBox(props: PointerBoxProps) {
   // don't destructure these, just grab them
-  const {bg, border, borderColor, theme} = props
+  const {bg, border, borderColor, theme, sx} = props
   const {caret, children, ...boxProps} = props
+
   const caretProps = {
-    bg,
-    borderColor,
+    bg: bg || sx?.bg || sx?.backgroundColor,
+    borderColor: borderColor || sx?.borderColor,
     borderWidth: border,
     location: caret,
     theme
   }
+
+  const defaultBoxProps = {borderWidth: '1px', borderStyle: 'solid', borderColor: 'border.default', borderRadius: 2}
+
   return (
-    <BorderBox sx={{position: 'relative'}} {...boxProps}>
+    <Box {...defaultBoxProps} {...boxProps} sx={{...sx, position: 'relative'}}>
       {children}
       <Caret {...caretProps} />
-    </BorderBox>
+    </Box>
   )
 }
 

src/__tests__/PointerBox.test.tsx

@@ -1,6 +1,6 @@
 import React from 'react'
 import {PointerBox} from '..'
-import {render, behavesAsComponent, checkExports} from '../utils/testing'
+import {render, behavesAsComponent, checkExports, renderStyles} from '../utils/testing'
 import {render as HTMLRender, cleanup} from '@testing-library/react'
 import {axe, toHaveNoViolations} from 'jest-axe'
 import 'babel-polyfill'
@@ -13,7 +13,7 @@ describe('PointerBox', () => {
     default: PointerBox
   })
 
-  it('renders a <Caret> in <BorderBox> with relative positioning', () => {
+  it('renders a <Caret> in <Box> with relative positioning', () => {
     expect(render(<PointerBox />)).toMatchSnapshot()
   })
 
@@ -24,11 +24,27 @@ describe('PointerBox', () => {
     cleanup()
   })
 
-  it('passes the "borderColor" prop to both <BorderBox> and <Caret>', () => {
+  it('applies the border color via "borderColor" prop for backwards compatibility', () => {
     expect(render(<PointerBox borderColor="danger.emphasis" />)).toMatchSnapshot()
   })
 
-  it('passes the "bg" prop to both <BorderBox> and <Caret>', () => {
-    expect(render(<PointerBox bg="danger.subtle" />)).toMatchSnapshot()
+  it('applies the border color via sx prop', () => {
+    expect(render(<PointerBox sx={{borderColor: 'danger.emphasis'}} />)).toMatchSnapshot()
+  })
+
+  it('applies the background color via "bg" prop for backwards compatibility', () => {
+    expect(render(<PointerBox bg="danger.emphasis" />)).toMatchSnapshot()
+  })
+
+  it('applies the background color via sx prop', () => {
+    expect(render(<PointerBox sx={{bg: 'danger.emphasis'}} />)).toMatchSnapshot()
+  })
+
+  it('ensures that background-color set via bg prop and sx output the same for backwards compatibility', () => {
+    const mockBg = 'red'
+    const viaStyledSystem = renderStyles(<PointerBox bg={mockBg} />)
+    const viaSxProp = renderStyles(<PointerBox sx={{bg: mockBg}} />)
+    expect(viaStyledSystem).toEqual(expect.objectContaining({'background-color': mockBg}))
+    expect(viaSxProp).toEqual(expect.objectContaining({'background-color': mockBg}))
   })
 })

src/__tests__/__snapshots__/PointerBox.test.tsx.snap

@@ -1,8 +1,8 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`PointerBox passes the "bg" prop to both <BorderBox> and <Caret> 1`] = `
+exports[`PointerBox applies the background color via "bg" prop for backwards compatibility 1`] = `
 .c0 {
-  background-color: #FFEBE9;
+  background-color: #cf222e;
   border-width: 1px;
   border-style: solid;
   border-color: #d0d7de;
@@ -31,7 +31,7 @@ exports[`PointerBox passes the "bg" prop to both <BorderBox> and <Caret> 1`] = `
     >
       <path
         d="M-8,0L0,8L8,0L-8,0Z"
-        fill="#FFEBE9"
+        fill="#cf222e"
       />
       <path
         d="M-8,0L0,8L8,0"
@@ -44,12 +44,100 @@ exports[`PointerBox passes the "bg" prop to both <BorderBox> and <Caret> 1`] = `
 </div>
 `;
 
-exports[`PointerBox passes the "borderColor" prop to both <BorderBox> and <Caret> 1`] = `
+exports[`PointerBox applies the background color via sx prop 1`] = `
 .c0 {
+  border-width: 1px;
+  border-style: solid;
+  border-color: #d0d7de;
+  border-radius: 6px;
+  background-color: #cf222e;
+  position: relative;
+}
+
+<div
+  className="c0"
+>
+  <svg
+    height={16}
+    style={
+      Object {
+        "left": "50%",
+        "marginLeft": -8,
+        "pointerEvents": "none",
+        "position": "absolute",
+        "top": "100%",
+      }
+    }
+    width={16}
+  >
+    <g
+      transform="translate(8,0)"
+    >
+      <path
+        d="M-8,0L0,8L8,0L-8,0Z"
+        fill="#cf222e"
+      />
+      <path
+        d="M-8,0L0,8L8,0"
+        fill="none"
+        stroke="#d0d7de"
+        strokeWidth="1px"
+      />
+    </g>
+  </svg>
+</div>
+`;
+
+exports[`PointerBox applies the border color via "borderColor" prop for backwards compatibility 1`] = `
+.c0 {
+  border-width: 1px;
+  border-style: solid;
   border-color: #cf222e;
+  border-radius: 6px;
+  position: relative;
+}
+
+<div
+  className="c0"
+>
+  <svg
+    height={16}
+    style={
+      Object {
+        "left": "50%",
+        "marginLeft": -8,
+        "pointerEvents": "none",
+        "position": "absolute",
+        "top": "100%",
+      }
+    }
+    width={16}
+  >
+    <g
+      transform="translate(8,0)"
+    >
+      <path
+        d="M-8,0L0,8L8,0L-8,0Z"
+        fill="#ffffff"
+      />
+      <path
+        d="M-8,0L0,8L8,0"
+        fill="none"
+        stroke="#cf222e"
+        strokeWidth="1px"
+      />
+    </g>
+  </svg>
+</div>
+`;
+
+exports[`PointerBox applies the border color via sx prop 1`] = `
+.c0 {
   border-width: 1px;
   border-style: solid;
+  border-color: #d0d7de;
   border-radius: 6px;
+  border-color: #cf222e;
   position: relative;
 }
 
@@ -87,7 +175,7 @@ exports[`PointerBox passes the "borderColor" prop to both <BorderBox> and <Caret
 </div>
 `;
 
-exports[`PointerBox renders a <Caret> in <BorderBox> with relative positioning 1`] = `
+exports[`PointerBox renders a <Caret> in <Box> with relative positioning 1`] = `
 .c0 {
   border-width: 1px;
   border-style: solid;