Skip to content

Commit be331af

Browse files
Luke Bowermanmdodgelooker
Luke Bowerman
and
mdodgelooker
authored
Popover now supports the preferred cloneElement style (#1103)
- Popover now supports the preferred `cloneElement` style usage in addition to the existing render prop style - Additionally the render prop style now exposes `aria-haspopup` and `aria-expanded` * Popover examples render props cleanup Co-authored-by: Meredith Dodge <[email protected]>
1 parent 9a31529 commit be331af

File tree

12 files changed

+220
-297
lines changed

12 files changed

+220
-297
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2727
- `InputText` interface simplified / narrowed
2828
- No longer supports typography or pseudo props
2929
- Switch from using defaultProps to `css` block to share common styles with other components
30+
- `Popover` now supports the preferred `cloneElement` style usage in addition to the existing render prop style
31+
- Additionally the render prop style now exposes `aria-haspopup` for use
3032
- `TextArea` interface simplified / narrowed
3133
- No longer supports border or typography props
3234
- `IconButton` improved hover/active states and no background on hover

‎packages/components/src/Popover/Popover.test.tsx‎

Lines changed: 73 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,7 @@ const PopoverGroup = () => {
4444
<>
4545
<div ref={groupRef}>
4646
<Popover content={SimpleContent} groupedPopoversRef={groupRef}>
47-
{(onClick, ref, className) => (
48-
<a onClick={onClick} ref={ref} className={className}>
49-
Instant Click
50-
</a>
51-
)}
47+
<a>Instant Click</a>
5248
</Popover>
5349

5450
<a onClick={instantClick} id="instant">
@@ -73,11 +69,57 @@ describe('Popover', () => {
7369
}
7470
})
7571

76-
test('opens and closes', () => {
72+
test('renderProps style opens and closes', () => {
7773
const { getByText, queryByText } = renderWithTheme(
7874
<Popover content={SimpleContent}>
79-
{(onClick, ref, className) => (
80-
<button ref={ref} onClick={onClick} className={className}>
75+
{(popoverProps) => <button {...popoverProps}>Test</button>}
76+
</Popover>
77+
)
78+
79+
// Verify hidden
80+
expect(queryByText('simple content')).not.toBeInTheDocument()
81+
82+
const trigger = getByText('Test')
83+
fireEvent.click(trigger)
84+
85+
// Find content
86+
expect(getByText('simple content')).toBeInTheDocument()
87+
88+
fireEvent.click(trigger)
89+
expect(queryByText('simple content')).not.toBeInTheDocument()
90+
})
91+
92+
test('cloneElement style opens and closes', () => {
93+
const { getByText, queryByText } = renderWithTheme(
94+
<Popover content={SimpleContent}>
95+
<button>Test</button>
96+
</Popover>
97+
)
98+
99+
// Verify hidden
100+
expect(queryByText('simple content')).not.toBeInTheDocument()
101+
102+
const trigger = getByText('Test')
103+
fireEvent.click(trigger)
104+
105+
// Find content
106+
expect(getByText('simple content')).toBeInTheDocument()
107+
108+
fireEvent.click(trigger)
109+
expect(queryByText('simple content')).not.toBeInTheDocument()
110+
})
111+
112+
test('renderProps style expanded opens and closes', () => {
113+
const { getByText, queryByText } = renderWithTheme(
114+
<Popover content={SimpleContent}>
115+
{({ className, onClick, ref, ...props }) => (
116+
<button
117+
aria-expanded={props['aria-expanded']}
118+
aria-haspopup={props['aria-haspopup']}
119+
className={className}
120+
onClick={onClick}
121+
ref={ref}
122+
>
81123
Test
82124
</button>
83125
)}
@@ -97,17 +139,33 @@ describe('Popover', () => {
97139
expect(queryByText('simple content')).not.toBeInTheDocument()
98140
})
99141

142+
test('cloneElement style opens and closes', () => {
143+
const { getByText, queryByText } = renderWithTheme(
144+
<Popover content={SimpleContent}>
145+
<button>Test</button>
146+
</Popover>
147+
)
148+
149+
// Verify hidden
150+
expect(queryByText('simple content')).not.toBeInTheDocument()
151+
152+
const trigger = getByText('Test')
153+
fireEvent.click(trigger)
154+
155+
// Find content
156+
expect(getByText('simple content')).toBeInTheDocument()
157+
158+
fireEvent.click(trigger)
159+
expect(queryByText('simple content')).not.toBeInTheDocument()
160+
})
161+
100162
test('stopPropagation works - event on container is not called', () => {
101163
const mockContainerOnClick = jest.fn()
102164

103165
const { getByText } = renderWithTheme(
104166
<div onClick={mockContainerOnClick}>
105167
<Popover content={SimpleContent}>
106-
{(onClick, ref, className) => (
107-
<button ref={ref} onClick={onClick} className={className}>
108-
Test
109-
</button>
110-
)}
168+
<button>Test</button>
111169
</Popover>
112170
</div>
113171
)
@@ -125,11 +183,7 @@ describe('Popover', () => {
125183
const { getByText, queryByText } = renderWithTheme(
126184
<>
127185
<Popover content={SimpleContent}>
128-
{(onClick, ref, className) => (
129-
<button onClick={onClick} ref={ref} className={className}>
130-
Instant Click
131-
</button>
132-
)}
186+
<button>Instant Click</button>
133187
</Popover>
134188
<a onClick={doThing}>Do thing...</a>
135189
</>
@@ -179,11 +233,7 @@ describe('Popover', () => {
179233
return (
180234
<div ref={hoverRef}>
181235
<Popover content={SimpleContent} hoverDisclosureRef={hoverRef}>
182-
{(onClick, ref, className) => (
183-
<Button onClick={onClick} ref={ref} className={className}>
184-
Instant Click
185-
</Button>
186-
)}
236+
<Button>Instant Click</Button>
187237
</Popover>
188238
Some text in the div
189239
</div>

‎packages/components/src/Popover/Popover.tsx‎

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ import React, {
3434
RefObject,
3535
Ref,
3636
SyntheticEvent,
37+
isValidElement,
38+
cloneElement,
3739
} from 'react'
3840
import { Box } from '../Layout'
3941
import { Portal } from '../Portal'
@@ -135,20 +137,30 @@ export interface UsePopoverProps {
135137
focusTrap?: boolean
136138
}
137139

140+
type PopoverRenderProp = (popoverProps: {
141+
onClick: (event: SyntheticEvent) => void
142+
/**
143+
* Used by popper.js to position the OverlaySurface relative to the trigger
144+
*/
145+
ref: Ref<any>
146+
className?: string
147+
'aria-expanded': boolean
148+
'aria-haspopup': boolean
149+
}) => ReactNode
150+
151+
function isRenderProp(
152+
children: ReactNode | PopoverRenderProp
153+
): children is PopoverRenderProp {
154+
return typeof children === 'function'
155+
}
156+
138157
export interface PopoverProps extends UsePopoverProps {
139158
/**
140159
* Component to wrap. The HOC will listen for mouse events on this
141160
* component, maintain the state of isOpen accordingly, and pass that state into
142161
* the modal renderProp.
143162
*/
144-
children: (
145-
onClick: (event: SyntheticEvent) => void,
146-
/**
147-
* Used by popper.js to position the OverlaySurface relative to the trigger
148-
*/
149-
ref: Ref<any>,
150-
className?: string
151-
) => JSX.Element
163+
children: ReactNode | PopoverRenderProp
152164

153165
/**
154166
* The element which hovering on/off of will show/hide the triggering element
@@ -528,16 +540,45 @@ export function Popover({
528540
hoverDisclosureRef,
529541
...props
530542
}: PopoverProps) {
531-
const { popover, open, ref, isOpen } = usePopover(props)
532-
const childrenOutput = children(open, ref, isOpen ? 'active' : '')
543+
const popoverProps = usePopover(props)
544+
545+
let target = children
546+
547+
// const popoverPropsLabeled = {
548+
// ...omit(popoverProps, ['popover', 'isOpen']),
549+
// }
550+
551+
const popoverPropsLabeled = {
552+
'aria-expanded': popoverProps.isOpen,
553+
'aria-haspopup': true,
554+
className: popoverProps.isOpen ? 'active' : '',
555+
onClick: popoverProps.open,
556+
ref: popoverProps.ref,
557+
}
558+
559+
if (isValidElement(children)) {
560+
target = cloneElement(children, {
561+
...popoverPropsLabeled,
562+
className: popoverProps.isOpen
563+
? `${children.props.className} active`
564+
: children.props.className,
565+
})
566+
} else if (isRenderProp(children)) {
567+
target = children(popoverPropsLabeled)
568+
} else {
569+
// eslint-disable-next-line no-console
570+
console.warn(
571+
`Element "${typeof target}" can't be used as target for Popover`
572+
)
573+
}
533574

534575
const [isHovered] = useHovered(hoverDisclosureRef)
535-
const triggerShown = isHovered || isOpen
576+
const triggerShown = isHovered || popoverProps.isOpen
536577

537578
return (
538579
<>
539-
{popover}
540-
{triggerShown && childrenOutput}
580+
{popoverProps.popover}
581+
{triggerShown && target}
541582
</>
542583
)
543584
}

‎packages/components/src/Tooltip/Tooltip.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ export interface TooltipProps extends UseTooltipProps {
123123
*/
124124
children: ReactNode | TooltipRenderProp
125125
}
126+
126127
export function useTooltip({
127128
arrow = true,
128129
canClose,

‎storybook/src/Forms/Date.stories.tsx‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -95,22 +95,15 @@ export const Controlled: FC = () => {
9595
</PopoverContent>
9696
}
9797
>
98-
{(onClick, ref, className) => (
99-
<Button
100-
aria-haspopup="true"
101-
onClick={onClick}
102-
ref={ref}
103-
className={className}
104-
>
105-
{controlledDate ? (
106-
<>
107-
<DateFormat>{controlledDate}</DateFormat>
108-
</>
109-
) : (
110-
'Select Dates'
111-
)}
112-
</Button>
113-
)}
98+
<Button>
99+
{controlledDate ? (
100+
<>
101+
<DateFormat>{controlledDate}</DateFormat>
102+
</>
103+
) : (
104+
'Select Dates'
105+
)}
106+
</Button>
114107
</Popover>
115108
)
116109
}

‎storybook/src/Forms/DateRange.stories.tsx‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,23 +86,16 @@ export const Controlled: FC = () => {
8686
</PopoverContent>
8787
}
8888
>
89-
{(onClick, ref, className) => (
90-
<Button
91-
aria-haspopup="true"
92-
onClick={onClick}
93-
ref={ref}
94-
className={className}
95-
>
96-
{controlledDateRange ? (
97-
<>
98-
<DateFormat>{controlledDateRange.from}</DateFormat> &mdash;
99-
<DateFormat>{controlledDateRange.to}</DateFormat>
100-
</>
101-
) : (
102-
'Select Dates'
103-
)}
104-
</Button>
105-
)}
89+
<Button>
90+
{controlledDateRange ? (
91+
<>
92+
<DateFormat>{controlledDateRange.from}</DateFormat> &mdash;
93+
<DateFormat>{controlledDateRange.to}</DateFormat>
94+
</>
95+
) : (
96+
'Select Dates'
97+
)}
98+
</Button>
10699
</Popover>
107100
)
108101
}

‎storybook/src/Overlays/Popovers/ContentOverflow.tsx‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,9 @@ export const ContentOverflow: FC = ({ children }) => (
4848
</PopoverContent>
4949
}
5050
>
51-
{(onClick, ref, className) => (
52-
<ButtonOutline
53-
iconAfter="ArrowDown"
54-
m="xxlarge"
55-
className={className}
56-
onClick={onClick}
57-
ref={ref}
58-
>
59-
{children}
60-
</ButtonOutline>
61-
)}
51+
<ButtonOutline iconAfter="ArrowDown" m="xxlarge">
52+
{children}
53+
</ButtonOutline>
6254
</Popover>
6355
</Box>
6456
)

‎storybook/src/Overlays/Popovers/EdgeOverflow.tsx‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,9 @@ export const EdgeOverflow: FC<Props> = ({
4949
</PopoverContent>
5050
}
5151
>
52-
{(onClick, ref, className) => (
53-
<ButtonOutline
54-
iconAfter="ArrowDown"
55-
m="xxlarge"
56-
className={className}
57-
onClick={onClick}
58-
ref={ref}
59-
>
60-
{children}
61-
</ButtonOutline>
62-
)}
52+
<ButtonOutline iconAfter="ArrowDown" m="xxlarge">
53+
{children}
54+
</ButtonOutline>
6355
</Popover>
6456
</Box>
6557
)

0 commit comments

Comments
 (0)