Skip to content

Commit

Permalink
Recalculate popover position when resized while open
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Jan 20, 2025
1 parent 0255544 commit 629e59c
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 3 deletions.
10 changes: 8 additions & 2 deletions src/components/feedback/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ function usePopoverPositioning(

// First of all, open popover if it's using the native API, otherwise its
// size is 0x0 and positioning calculations won't work.
const popover = popoverRef.current;
const popover = popoverRef.current!;
if (asNativePopover) {
popover?.togglePopover(true);
popover.togglePopover(true);
}

const cleanup = adjustPopoverPositioning();
Expand All @@ -151,12 +151,18 @@ function usePopoverPositioning(
capture: true,
});

// Readjust popover positioning if its resized, in case it dropped-up, and
// it needs to be moved down
const observer = new ResizeObserver(adjustPopoverPositioning);
observer.observe(popover);

return () => {
if (asNativePopover) {
popover?.togglePopover(false);
}
cleanup();
listeners.removeAll();
observer.disconnect();
};
}, [adjustPopoverPositioning, asNativePopover, popoverOpen, popoverRef]);
}
Expand Down
52 changes: 51 additions & 1 deletion src/components/feedback/test/Popover-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { mount } from '@hypothesis/frontend-testing';
import { mount, waitFor } from '@hypothesis/frontend-testing';
import { useRef, useState } from 'preact/hooks';

import Popover, { POPOVER_VIEWPORT_HORIZONTAL_GAP } from '../Popover';
Expand Down Expand Up @@ -73,6 +73,20 @@ describe('Popover', () => {
return popoverTop < buttonTop;
};

const getDistanceBetweenButtonAndPopover = wrapper => {
const appearedAbove = popoverAppearedAbove(wrapper);
const { top: popoverTop, bottom: popoverBottom } = getPopover(wrapper)
.getDOMNode()
.getBoundingClientRect();
const { top: buttonTop, bottom: buttonBottom } = getToggleButton(wrapper)
.getDOMNode()
.getBoundingClientRect();

return Math.abs(
appearedAbove ? popoverBottom - buttonTop : buttonBottom - popoverTop,
);
};

[
{
restoreFocusOnClose: undefined, // Defaults to true
Expand Down Expand Up @@ -151,6 +165,42 @@ describe('Popover', () => {
});
});

[true, false].forEach(asNativePopover => {
it('repositions popover if it is resized after being open', async () => {
const wrapper = createComponent(
{
asNativePopover,
children: (
<>
<p>one</p>
<p>two</p>
<p>three</p>
</>
),
},
{ paddingTop: 1000 }, // Ensure popover appears above
);
togglePopover(wrapper);

// This applies only when the popover appears above, so let's verify it
assert.isTrue(popoverAppearedAbove(wrapper));

// After opening the popover, its distance to the button should be the
// same, even if it's resized
const distanceBeforeResizing =
getDistanceBetweenButtonAndPopover(wrapper);
wrapper.setProps({ children: 'Just one line' });

// Repositioning happens asynchronously via ResizeObserver, so we need to
// wait for it to eventually happen.
await waitFor(
() =>
distanceBeforeResizing ===
getDistanceBetweenButtonAndPopover(wrapper),
);
});
});

context('when popover is supported', () => {
[undefined, true].forEach(asNativePopover => {
it('opens via popover API', async () => {
Expand Down

0 comments on commit 629e59c

Please sign in to comment.