Skip to content

Commit f1e6a79

Browse files
fix(popover-headless): set popover="auto" as a default (#36090)
Co-authored-by: Dmytro Kirpa <dmytrokirpa@microsoft.com>
1 parent b0cb4c7 commit f1e6a79

4 files changed

Lines changed: 56 additions & 16 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "fix: set popover=auto on surface via jsx defaults",
4+
"packageName": "@fluentui/react-headless-components-preview",
5+
"email": "vgenaev@gmail.com",
6+
"dependentChangeType": "patch"
7+
}

packages/react-components/react-headless-components-preview/library/src/components/Popover/PopoverSurface/usePopoverSurface.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export const usePopoverSurface = (props: PopoverSurfaceProps, ref: React.Ref<HTM
3434
role: 'group',
3535
...props,
3636
id: surfaceId,
37+
popover: 'auto',
3738
'data-popover-surface': '',
3839
'data-open': stringifyDataAttribute(open),
3940
},

packages/react-components/react-headless-components-preview/library/src/components/Popover/usePopover.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,6 @@ export const usePopover = (props: PopoverProps): PopoverState => {
9898
return;
9999
}
100100

101-
if (!surface.hasAttribute('popover') || surface.getAttribute('popover') !== 'auto') {
102-
surface.setAttribute('popover', 'auto');
103-
}
104-
105101
if (!(SUPPORTS_POPOVER_OPEN_SELECTOR && surface.matches(':popover-open'))) {
106102
surface.showPopover();
107103
}

packages/react-components/react-headless-components-preview/library/src/hooks/usePositioning/usePositioning.test.tsx

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ describe('usePositioning', () => {
3838
it('containerRef writes position-anchor and position-area matching the props', () => {
3939
const result = mountHook({ position: 'below', align: 'start' });
4040
const node = document.createElement('div');
41-
result.current.containerRef(node);
41+
42+
act(() => {
43+
result.current.containerRef(node);
44+
});
4245

4346
expect(node.style.getPropertyValue('position-anchor')).toMatch(/^--popover-anchor-/);
4447
expect(node).toHaveStyle({ positionArea: 'block-end span-inline-end' });
@@ -47,39 +50,54 @@ describe('usePositioning', () => {
4750
it('containerRef writes position: absolute by default and clears the UA inset/margin defaults', () => {
4851
const result = mountHook();
4952
const node = document.createElement('div');
50-
result.current.containerRef(node);
53+
54+
act(() => {
55+
result.current.containerRef(node);
56+
});
5157

5258
expect(node).toHaveStyle({ position: 'absolute', inset: 'auto', margin: '0px' });
5359
});
5460

5561
it('containerRef honors strategy: "fixed"', () => {
5662
const result = mountHook({ strategy: 'fixed' });
5763
const node = document.createElement('div');
58-
result.current.containerRef(node);
64+
65+
act(() => {
66+
result.current.containerRef(node);
67+
});
5968

6069
expect(node).toHaveStyle({ position: 'fixed' });
6170
});
6271

6372
it('containerRef writes data-placement matching (position, align)', () => {
6473
const result = mountHook({ position: 'below', align: 'start' });
6574
const node = document.createElement('div');
66-
result.current.containerRef(node);
75+
76+
act(() => {
77+
result.current.containerRef(node);
78+
});
6779

6880
expect(node).toHaveAttribute('data-placement', 'below-start');
6981
});
7082

7183
it('containerRef sets position-try-fallbacks to the three-try flip chain by default', () => {
7284
const result = mountHook();
7385
const node = document.createElement('div');
74-
result.current.containerRef(node);
86+
87+
act(() => {
88+
result.current.containerRef(node);
89+
});
7590

7691
expect(node).toHaveStyle({ positionTryFallbacks: 'flip-block, flip-inline, flip-block flip-inline' });
7792
});
7893

7994
it('containerRef uses custom fallbackPositions verbatim when provided', () => {
8095
const result = mountHook({ fallbackPositions: ['below-start', 'after'] });
8196
const node = document.createElement('div');
82-
result.current.containerRef(node);
97+
98+
act(() => {
99+
result.current.containerRef(node);
100+
});
83101

84102
expect(node).toHaveStyle({ positionTryFallbacks: 'block-end span-inline-end, inline-end' });
85103
});
@@ -88,31 +106,43 @@ describe('usePositioning', () => {
88106
const result = mountHook({ pinned: true });
89107
const node = document.createElement('div');
90108
node.style.setProperty('position-try-fallbacks', 'flip-block');
91-
result.current.containerRef(node);
109+
110+
act(() => {
111+
result.current.containerRef(node);
112+
});
92113

93114
expect(node.style.getPropertyValue('position-try-fallbacks')).toBe('');
94115
});
95116

96117
it('containerRef writes cover self-alignment when coverTarget is true', () => {
97118
const result = mountHook({ coverTarget: true, position: 'above', align: 'start' });
98119
const node = document.createElement('div');
99-
result.current.containerRef(node);
120+
121+
act(() => {
122+
result.current.containerRef(node);
123+
});
100124

101125
expect(node).toHaveStyle({ positionArea: 'center', alignSelf: 'end', justifySelf: 'start' });
102126
});
103127

104128
it('containerRef writes place-self: anchor-center for center alignment (crbug 438334710 workaround)', () => {
105129
const result = mountHook({ position: 'above', align: 'center' });
106130
const node = document.createElement('div');
107-
result.current.containerRef(node);
131+
132+
act(() => {
133+
result.current.containerRef(node);
134+
});
108135

109136
expect(node).toHaveStyle({ placeSelf: 'anchor-center' });
110137
});
111138

112139
it('containerRef does not write place-self for non-center alignments', () => {
113140
const result = mountHook({ position: 'above', align: 'start' });
114141
const node = document.createElement('div');
115-
result.current.containerRef(node);
142+
143+
act(() => {
144+
result.current.containerRef(node);
145+
});
116146

117147
expect(node.style.getPropertyValue('place-self')).toBe('');
118148
expect(node.style.getPropertyValue('justify-self')).toBe('');
@@ -122,15 +152,21 @@ describe('usePositioning', () => {
122152
it('containerRef writes matchTargetSize width via anchor-size()', () => {
123153
const result = mountHook({ matchTargetSize: 'width' });
124154
const node = document.createElement('div');
125-
result.current.containerRef(node);
155+
156+
act(() => {
157+
result.current.containerRef(node);
158+
});
126159

127160
expect(node).toHaveStyle({ width: 'anchor-size(width)' });
128161
});
129162

130163
it('containerRef applies offset as logical margins', () => {
131164
const result = mountHook({ position: 'below', offset: { mainAxis: 8, crossAxis: 4 } });
132165
const node = document.createElement('div');
133-
result.current.containerRef(node);
166+
167+
act(() => {
168+
result.current.containerRef(node);
169+
});
134170

135171
expect(node).toHaveStyle({ marginBlockStart: '8px', marginInlineStart: '4px' });
136172
});

0 commit comments

Comments
 (0)