Skip to content

Commit 90cd8ca

Browse files
ling1726ling1726
authored
feat(Popover): Support nesting (#18148)
* nested popover story * feat(Popover): Support nesting Introduces `@fluentui/react-virtual-parents` as a utility library to set virtual parents. Integrates it as an optional feature in `usePopper`. The virtual parent feature will allow `Popovers` to be nested so that: * click outside works for the entire `Popover` stack * clicking in parent `Popovers` will close nested `Popovers` * Change files * move virtual parent code to portal * remove package, refactor vparent * Change files * Add docs and spec for portal * fix api extractor * PR suggestions * Add E2E tests for nested popovers * turn off as conformance test * nested popover story * feat(Popover): Support nesting Introduces `@fluentui/react-virtual-parents` as a utility library to set virtual parents. Integrates it as an optional feature in `usePopper`. The virtual parent feature will allow `Popovers` to be nested so that: * click outside works for the entire `Popover` stack * clicking in parent `Popovers` will close nested `Popovers` * Change files * move virtual parent code to portal * remove package, refactor vparent * Change files * Add docs and spec for portal * fix api extractor * PR suggestions * Add E2E tests for nested popovers * turn off as conformance test * update tooltip snapshots due to portal virtual parent * Change files * PR suggestions
1 parent 644df65 commit 90cd8ca

27 files changed

+615
-21
lines changed

‎change/@fluentui-react-portal-7e967006-0a2d-4cbf-a040-9c510134c3f3.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "Render portals with virtual parent",
4+
"packageName": "@fluentui/react-portal",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-tooltip-44f53e3c-eea5-4747-b250-9b8bdc2efbd8.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "none",
3+
"comment": "Update snapshots referencing months (#10023)",
4+
"packageName": "@fluentui/react-tooltip",
5+
"email": "[email protected]",
6+
"dependentChangeType": "none"
7+
}

‎change/@fluentui-react-utilities-d865e9d6-c439-410e-bc3c-aa754d0b206e.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "Support custom `contains` check",
4+
"packageName": "@fluentui/react-utilities",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎packages/react-popover/e2e/Popover.e2e.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,55 @@ describe('With custom trigger', () => {
4343
.should('not.exist');
4444
});
4545
});
46+
47+
describe('Nested', () => {
48+
beforeEach(() => {
49+
// Open the whole stack of popovers
50+
cy.visitStory('Popover', 'NestedPopovers')
51+
.contains('Root')
52+
.click()
53+
.get('body')
54+
.contains('First')
55+
.click()
56+
.get('body')
57+
.contains('Second')
58+
.first()
59+
.click();
60+
});
61+
62+
it('should dismiss all nested popovers on outside click', () => {
63+
cy.get('body')
64+
.click('bottomRight')
65+
.get(popoverContentSelector)
66+
.should('not.exist');
67+
});
68+
69+
it('should not dismiss when clicking on nested content', () => {
70+
cy.contains('Second nested button')
71+
.click()
72+
.get(popoverContentSelector)
73+
.should('have.length', 3);
74+
});
75+
76+
it('should dismiss child popovers when clicking on parents', () => {
77+
cy.contains('First nested button')
78+
.click()
79+
.get(popoverContentSelector)
80+
.should('have.length', 2)
81+
.contains('Root button')
82+
.click()
83+
.get(popoverContentSelector)
84+
.should('have.length', 1);
85+
});
86+
87+
it('should when opening a sibling popover, should dismiss other sibling popover', () => {
88+
cy.contains('Sibling nested trigger')
89+
.click()
90+
.get(popoverContentSelector)
91+
.should('have.length', 3)
92+
.contains('Second nested trigger')
93+
.click()
94+
.get(popoverContentSelector)
95+
.should('have.length', 3);
96+
});
97+
});

‎packages/react-popover/src/Popover.stories.tsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,51 @@ export const WithCustomTrigger = () => {
103103
);
104104
};
105105

106+
export const NestedPopovers = () => {
107+
return (
108+
<Popover>
109+
<PopoverTrigger>
110+
<button>Root trigger</button>
111+
</PopoverTrigger>
112+
113+
<PopoverContent>
114+
<ExampleContent />
115+
<button>Root button</button>
116+
<Popover>
117+
<PopoverTrigger>
118+
<button style={{ marginLeft: 100 }}>First nested trigger</button>
119+
</PopoverTrigger>
120+
121+
<PopoverContent>
122+
<ExampleContent />
123+
<button>First nested button</button>
124+
<Popover>
125+
<PopoverTrigger>
126+
<button>Second nested trigger</button>
127+
</PopoverTrigger>
128+
129+
<PopoverContent>
130+
<ExampleContent />
131+
<button>Second nested button</button>
132+
</PopoverContent>
133+
</Popover>
134+
<Popover>
135+
<PopoverTrigger>
136+
<button style={{ marginLeft: 100 }}>Sibling nested trigger</button>
137+
</PopoverTrigger>
138+
139+
<PopoverContent>
140+
<ExampleContent />
141+
<button>Second nested button</button>
142+
</PopoverContent>
143+
</Popover>
144+
</PopoverContent>
145+
</Popover>
146+
</PopoverContent>
147+
</Popover>
148+
);
149+
};
150+
106151
export default {
107152
title: 'Components/Popover',
108153
component: Popover,

‎packages/react-popover/src/components/Popover/usePopover.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import * as React from 'react';
2-
import { makeMergeProps, useControllableValue, useOnClickOutside, useEventCallback } from '@fluentui/react-utilities';
2+
import { makeMergeProps, useControllableValue, useEventCallback, useOnClickOutside } from '@fluentui/react-utilities';
33
import { useFluent } from '@fluentui/react-shared-contexts';
44
import { usePopper } from '@fluentui/react-positioning';
5+
import { elementContains } from '@fluentui/react-portal';
56
import { PopoverProps, PopoverState } from './Popover.types';
67

78
const mergeProps = makeMergeProps<PopoverState>({});
@@ -35,6 +36,7 @@ export const usePopover = (props: PopoverProps, defaultProps?: PopoverProps): Po
3536

3637
const { targetDocument } = useFluent();
3738
useOnClickOutside({
39+
contains: elementContains,
3840
element: targetDocument,
3941
callback: ev => state.setOpen(ev, false),
4042
refs: [state.triggerRef, state.contentRef],

‎packages/react-popover/src/components/PopoverContent/PopoverContent.test.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ jest.mock('../../popoverContext');
1010

1111
describe('PopoverContent', () => {
1212
isConformant({
13+
// as render test will pass a span tag which is also considered one of the skipped helperComponents
14+
disabledTests: ['as-renders-html'],
1315
Component: PopoverContent,
1416
displayName: 'PopoverContent',
1517
requiredProps: { open: true },
16-
helperComponents: [Portal],
18+
helperComponents: [Portal, 'span'],
1719
});
1820

1921
let wrapper: ReactWrapper | undefined;

‎packages/react-portal/README.md

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,117 @@ const node = document.getElementById('customNode');
3131
### Styling
3232

3333
`Portal` renders React children directly to the default/configured DOM node. Therefore styling should be applied to the `children` by users directly.
34+
35+
### Virtual parents
36+
37+
Out of order DOM elements can be problematic when using 'click outside' event listeners since you cannot rely on `element.contains(event.target)` because the `Portal` elements are out of DOM order.
38+
39+
```tsx
40+
41+
const outerButtonRef = React.useRef();
42+
const innerButtonRef = React.useRef();
43+
44+
45+
<Portal>
46+
<div>
47+
<button ref={outerButtonRef}> Outer button </button>
48+
<Portal>
49+
<div>
50+
<button ref={innerButtonRef}> Inner button </button>
51+
</div>
52+
</Portal>
53+
</div>
54+
</Portal>
55+
56+
// DOM output
57+
<div>
58+
<button>Outer button</button>
59+
</div>
60+
61+
<div>
62+
<button>Inner button</button>
63+
</div>
64+
65+
// Let's add an event listener to 'dismss' the outer portal when clicked outside
66+
// ⚠⚠⚠ This will always be called when clicking on the inner button
67+
document.addEventListener((event) => {
68+
if (outerButtonRef.current.contains(event.target)) {
69+
dismissOuterPortal();
70+
}
71+
})
72+
```
73+
74+
When the above case is not required, using `element.contains` is perfectly fine. But nested cases should still be handled appropriately. We do this using the concept of `virtual parents`
75+
76+
`Portal` will make public 2 utilities that will only be used in cases where the user needs to know if an out of order DOM element will need to be used or not.
77+
78+
- `setVirtualParent` - sets virtual parent. Portal uses this already internally.
79+
- `elementContains` - similar to `element.contains` but uses the virtual hierarchy as reference
80+
81+
Below shows what a virtual parent is
82+
83+
```tsx
84+
// Setting a virtual parent
85+
86+
const parent document.getElementById('parent')
87+
const child document.getElement.ById('child');
88+
89+
child._virtual.parent = parent;
90+
```
91+
92+
`Portals` will render a hidden span that will be the virtual parent, by nesting portals virtual parens will also be nested so that `elementContains` will work predictably.
93+
94+
```tsx
95+
<FluentProvider>
96+
<Portal id="portal-1" />
97+
<Portal id="portal-2" />
98+
</FluentProvider>
99+
```
100+
101+
DOM output:
102+
103+
```tsx
104+
<body>
105+
<div>
106+
{/* Virtual parent for portal*/}
107+
<span aria-hidden />
108+
{/* Virtual parent for portal*/}
109+
<span aria-hidden />
110+
</div>
111+
112+
<div id="portal-1" class="theme-provider-0">
113+
{children}
114+
</div>
115+
<div id="portal-2" class="theme-provider-0">
116+
{children}
117+
</div>
118+
</body>
119+
```
120+
121+
```tsx
122+
<FluentProvider>
123+
<Portal id="portal-1">
124+
<Portal id="portal-2" />
125+
</Portal>
126+
</FluentProvider>
127+
```
128+
129+
DOM output:
130+
131+
```tsx
132+
<body>
133+
<div>
134+
{/* Virtual parent for outer portal*/}
135+
<span aria-hidden></span>
136+
</div>
137+
138+
<div id="portal-1" class="theme-provider-0">
139+
{/* Virtual parent for inner portal*/}
140+
<span aria-hidden />
141+
{children}
142+
</div>
143+
<div id="portal-2" class="theme-provider-0">
144+
{children}
145+
</div>
146+
</body>
147+
```

0 commit comments

Comments
 (0)