Skip to content

Commit 191540f

Browse files
committed
refactor: improve RiPopover API with trigger and className props for better extensibility
Refactor RiPopover to add optional trigger and className props while maintaining backwards compatibility. React elements in trigger render directly without span wrapper for styled-components support.
1 parent 3e1990d commit 191540f

File tree

2 files changed

+67
-26
lines changed

2 files changed

+67
-26
lines changed

redisinsight/ui/src/components/base/popover/RiPopover.tsx

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,34 +11,68 @@ export const RiPopover = ({
1111
children,
1212
ownFocus,
1313
button,
14+
trigger,
1415
anchorPosition,
1516
panelPaddingSize,
1617
anchorClassName,
1718
panelClassName,
19+
className,
1820
maxWidth = '100%',
1921
...props
20-
}: RiPopoverProps) => (
21-
<Popover
22-
{...props}
23-
open={isOpen}
24-
onClickOutside={closePopover}
25-
onKeyDown={(event) => {
26-
// Close on escape press
27-
if (event.key === keys.ESCAPE) {
28-
closePopover?.(event as any)
29-
}
30-
}}
31-
content={children}
32-
// Props passed to the children wrapper:
33-
className={panelClassName}
34-
maxWidth={maxWidth}
35-
style={{
36-
padding: panelPaddingSize && panelPaddingSizeMap[panelPaddingSize],
37-
}}
38-
autoFocus={ownFocus}
39-
placement={anchorPosition && anchorPositionMap[anchorPosition]?.placement}
40-
align={anchorPosition && anchorPositionMap[anchorPosition]?.align}
41-
>
42-
<span className={anchorClassName}>{button}</span>
43-
</Popover>
44-
)
22+
}: RiPopoverProps) => {
23+
// Warn if both button and trigger are provided
24+
if (button !== undefined && trigger !== undefined) {
25+
console.warn(
26+
"RiPopover: Both 'button' and 'trigger' props are provided. Using 'trigger'. Please migrate to 'trigger' prop.",
27+
)
28+
}
29+
30+
// Warn if both panelClassName and className are provided
31+
if (panelClassName !== undefined && className !== undefined) {
32+
console.warn(
33+
"RiPopover: Both 'panelClassName' and 'className' props are provided. Using 'className'. Please migrate to 'className' prop.",
34+
)
35+
}
36+
37+
// Determine which trigger to use
38+
const activeTrigger = trigger ?? button
39+
40+
// Determine which className to use
41+
const activeClassName = className ?? panelClassName
42+
43+
// Render trigger element
44+
// For new API (trigger): React elements render directly, scalars wrap in span
45+
// For old API (button): Always wrap in span (backwards compatibility)
46+
const triggerElement =
47+
trigger !== undefined && React.isValidElement(activeTrigger) ? (
48+
activeTrigger
49+
) : (
50+
<span className={anchorClassName}>{activeTrigger}</span>
51+
)
52+
53+
return (
54+
<Popover
55+
{...props}
56+
open={isOpen}
57+
onClickOutside={closePopover}
58+
onKeyDown={(event) => {
59+
// Close on escape press
60+
if (event.key === keys.ESCAPE) {
61+
closePopover?.(event as any)
62+
}
63+
}}
64+
content={children}
65+
// Props passed to the children wrapper:
66+
className={activeClassName}
67+
maxWidth={maxWidth}
68+
style={{
69+
padding: panelPaddingSize && panelPaddingSizeMap[panelPaddingSize],
70+
}}
71+
autoFocus={ownFocus}
72+
placement={anchorPosition && anchorPositionMap[anchorPosition]?.placement}
73+
align={anchorPosition && anchorPositionMap[anchorPosition]?.align}
74+
>
75+
{triggerElement}
76+
</Popover>
77+
)
78+
}

redisinsight/ui/src/components/base/popover/types.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { type PopoverProps } from '@redis-ui/components'
22

33
import { anchorPositionMap, panelPaddingSizeMap } from './config'
4+
import { ReactNode } from 'react'
45

56
type AnchorPosition = keyof typeof anchorPositionMap
67

@@ -19,10 +20,16 @@ export type RiPopoverProps = Omit<
1920
isOpen?: PopoverProps['open']
2021
closePopover?: PopoverProps['onClickOutside']
2122
ownFocus?: PopoverProps['autoFocus']
22-
button: PopoverProps['content']
23+
/* @deprecated old prop for popover trigger element, use trigger */
24+
button?: PopoverProps['content']
25+
/* preferred prop for popover trigger element (optional) */
26+
trigger?: ReactNode
2327
anchorPosition?: AnchorPosition
2428
panelPaddingSize?: PanelPaddingSize
2529
anchorClassName?: string
30+
/* @deprecated - use @see{className} - this is popover content wrapper class name */
2631
panelClassName?: string
32+
// new preferred prop for popover content wrapper class name (optional)
33+
className?: string
2734
'data-testid'?: string
2835
}

0 commit comments

Comments
 (0)