Skip to content

Commit 0f0fbbe

Browse files
authored
fix(popover): replace displayName checks (#21550)
1 parent 1f1b726 commit 0f0fbbe

2 files changed

Lines changed: 35 additions & 13 deletions

File tree

packages/react/src/components/Popover/__tests__/Popover-test.js

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*/
77

88
import { render, screen } from '@testing-library/react';
9-
import React from 'react';
9+
import React, { forwardRef } from 'react';
1010
import { Popover, PopoverContent } from '../../Popover';
1111
import userEvent from '@testing-library/user-event';
1212
import { waitForPosition } from '../../ListBox/test-helpers';
@@ -17,6 +17,11 @@ import { default as Checkbox } from '../../Checkbox';
1717
const prefix = 'cds';
1818

1919
describe('Popover', () => {
20+
const TriggerWithPopoverContentDisplayName = forwardRef((props, ref) => (
21+
<button type="button" ref={ref} {...props} />
22+
));
23+
TriggerWithPopoverContentDisplayName.displayName = 'PopoverContent';
24+
2025
it('should support a ref on the outermost element', () => {
2126
const ref = jest.fn();
2227
const { container } = render(
@@ -112,6 +117,24 @@ describe('Popover', () => {
112117
expect(caretContainer).toHaveStyle({ left: '0px', top: '-7px' });
113118
});
114119

120+
it('should auto align when trigger component shares `PopoverContent` `displayName`', async () => {
121+
render(
122+
<Popover open align="bottom" data-testid="test" autoAlign>
123+
<TriggerWithPopoverContentDisplayName>
124+
Settings
125+
</TriggerWithPopoverContentDisplayName>
126+
<PopoverContent />
127+
</Popover>
128+
);
129+
130+
await waitForPosition();
131+
132+
const caretContainer =
133+
screen.getByTestId('test').lastChild.lastChild.firstChild;
134+
135+
expect(caretContainer).toHaveStyle({ left: '0px', top: '-6px' });
136+
});
137+
115138
it('should forward additional props on the outermost element', () => {
116139
const { container } = render(
117140
<PopoverContent id="test" data-testid="test">

packages/react/src/components/Popover/index.tsx

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import React, {
1616
type ElementType,
1717
} from 'react';
1818
import useIsomorphicEffect from '../../internal/useIsomorphicEffect';
19+
import { isComponentElement } from '../../internal';
1920
import { useMergedRefs } from '../../internal/useMergedRefs';
2021
import { usePrefix } from '../../internal/usePrefix';
2122
import { useWindowEvent, useEvent } from '../../internal/useEvent';
@@ -475,7 +476,13 @@ export const Popover: PopoverComponent & {
475476
const mappedChildren = React.Children.map(children, (child) => {
476477
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- https://github.com/carbon-design-system/carbon/issues/20452
477478
const item = child as any;
478-
const displayName = item?.type?.displayName;
479+
// TODO: Stop relying on `displayName` checks by moving `Toggletip`
480+
// subcomponents into their own files that avoid the `Popover` <->
481+
// `Toggletip` circular dependency. Then replace these `displayName` checks
482+
// with `isComponentElement(item, ...)` checks.
483+
const isToggletipButton = item?.type?.displayName === 'ToggletipButton';
484+
const isToggletipContent = item?.type?.displayName === 'ToggletipContent';
485+
const isPopoverContent = isComponentElement(item, PopoverContent);
479486

480487
/**
481488
* Only trigger elements (button) or trigger components (ToggletipButton) should be
@@ -485,13 +492,9 @@ export const Popover: PopoverComponent & {
485492
* is on, even if they are a trigger element.
486493
*/
487494
const isTriggerElement = item?.type === 'button';
488-
const isTriggerComponent =
489-
enableFloatingStyles &&
490-
displayName &&
491-
['ToggletipButton'].includes(displayName);
495+
const isTriggerComponent = enableFloatingStyles && isToggletipButton;
492496
const isAllowedTriggerComponent =
493-
enableFloatingStyles &&
494-
!['ToggletipContent', 'PopoverContent'].includes(displayName);
497+
enableFloatingStyles && !isToggletipContent && !isPopoverContent;
495498

496499
if (
497500
React.isValidElement(item) &&
@@ -523,11 +526,7 @@ export const Popover: PopoverComponent & {
523526
// For a toggletip there is a specific trigger component, ToggletipButton.
524527
// In either of these cases we want to set this as the reference node for floating-ui autoAlign
525528
// positioning.
526-
if (
527-
(enableFloatingStyles && item?.type !== PopoverContent) ||
528-
(enableFloatingStyles &&
529-
item?.type['displayName'] === 'ToggletipButton')
530-
) {
529+
if (enableFloatingStyles && !isPopoverContent) {
531530
// Set the reference element for floating-ui
532531
refs.setReference(node);
533532
}

0 commit comments

Comments
 (0)