diff --git a/UNRELEASED.md b/UNRELEASED.md index 31ccfe53e02..88e23af8fcb 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -7,6 +7,7 @@ - Added high contrast outline to `ActionList` ([#2713](https://github.com/Shopify/polaris-react/pull/2713)) - Added high contrast border to `Button` ([#2712](https://github.com/Shopify/polaris-react/pull/2712)) - Added styled placeholder image to `Avatar` when initials are blank ([#2693](https://github.com/Shopify/polaris-react/pull/2693)) +- Added a `preferInputActivator` prop to `Popover` to allow better positioning of the overlay ([#2754](https://github.com/Shopify/polaris-react/pull/2754)) ### Bug fixes diff --git a/src/components/Card/tests/Card.test.tsx b/src/components/Card/tests/Card.test.tsx index c98fa120e49..a2cf4112582 100644 --- a/src/components/Card/tests/Card.test.tsx +++ b/src/components/Card/tests/Card.test.tsx @@ -1,10 +1,6 @@ import React from 'react'; // eslint-disable-next-line no-restricted-imports -import { - mountWithAppProvider, - trigger, - findByTestID, -} from 'test-utilities/legacy'; +import {mountWithAppProvider} from 'test-utilities/legacy'; import {mountWithApp} from 'test-utilities'; import {Card, Badge, Button, Popover, ActionList} from 'components'; import {WithinContentContext} from '../../../utilities/within-content-context'; @@ -177,35 +173,28 @@ describe('', () => { {content: 'Most important action'}, {content: 'Second most important action'}, ]; - const card = mountWithAppProvider( + const card = mountWithApp(

Some card content.

, ); - const disclosureButton = card.find(Button).first(); - expect(disclosureButton).toHaveLength(1); - expect(disclosureButton.text()).toBe('More'); - - const popover = card.find(Popover).first(); - expect(popover).toHaveLength(1); - expect(popover.prop('active')).toBe(false); + const disclosureButton = card.findAll(Button)[0]; + expect(disclosureButton).toContainReactText('More'); - trigger(disclosureButton, 'onClick'); + expect(card).toContainReactComponent(Popover, { + active: false, + }); - expect( - card - .find(Popover) - .first() - .prop('active'), - ).toBe(true); + disclosureButton.trigger('onClick'); - const overlay = findByTestID(card, 'popoverOverlay'); - expect(overlay).toHaveLength(1); + expect(card).toContainReactComponent(Popover, { + active: true, + }); - const actionList = overlay.find(ActionList).first(); - expect(actionList).toHaveLength(1); - expect(actionList.prop('items')).toBe(footerActions); + expect(card).toContainReactComponent(ActionList, { + items: footerActions, + }); }); it('sets the disclosure button content to the value set on secondaryFooterActionsDisclosureText', () => { diff --git a/src/components/Popover/Popover.tsx b/src/components/Popover/Popover.tsx index 047a79caf1b..33c0a587e55 100644 --- a/src/components/Popover/Popover.tsx +++ b/src/components/Popover/Popover.tsx @@ -32,6 +32,11 @@ export interface PopoverProps { active: boolean; /** The element to activate the Popover */ activator: React.ReactElement; + /** + * Use the activator's input element to calculate the Popover position + * @default true + */ + preferInputActivator?: PopoverOverlayProps['preferInputActivator']; /** * The element type to wrap the activator with * @default 'div' @@ -71,6 +76,7 @@ export const Popover: React.FunctionComponent & { active, fixed, ariaHaspopup, + preferInputActivator = true, ...rest }: PopoverProps) { const [activatorNode, setActivatorNode] = useState(); @@ -131,11 +137,11 @@ export const Popover: React.FunctionComponent & { }, [activatorNode, setAccessibilityAttributes]); const portal = activatorNode ? ( - + ', () => { ).toBe('center'); }); + it('passes preferInputActivator to PositionedOverlay when false', () => { + const popoverOverlay = mountWithAppProvider( + + {children} + , + ); + + expect( + popoverOverlay.find(PositionedOverlay).prop('preferInputActivator'), + ).toBe(false); + }); + it('calls the onClose callback when the escape key is pressed', () => { const spy = jest.fn(); diff --git a/src/components/Popover/tests/Popover.test.tsx b/src/components/Popover/tests/Popover.test.tsx index 1fb859cdbd4..8ea0d0e146e 100644 --- a/src/components/Popover/tests/Popover.test.tsx +++ b/src/components/Popover/tests/Popover.test.tsx @@ -1,7 +1,7 @@ import React, {useState, useCallback} from 'react'; -// eslint-disable-next-line no-restricted-imports -import {mountWithAppProvider, findByTestID} from 'test-utilities/legacy'; import {mountWithApp} from 'test-utilities'; +import {PositionedOverlay} from 'components/PositionedOverlay'; +import {Portal} from 'components'; import {Popover} from '../Popover'; import {PopoverOverlay} from '../components'; import * as setActivatorAttributes from '../set-activator-attributes'; @@ -22,7 +22,7 @@ describe('', () => { }); it('invokes setActivatorAttributes with active, ariaHasPopup and id', () => { - mountWithAppProvider( + mountWithApp( Activator} onClose={spy} />, ); @@ -33,43 +33,39 @@ describe('', () => { }); it('renders a portal', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( Activator} onClose={spy} />, ); - const portal = findByTestID(popover, 'portal'); - expect(portal.exists()).toBeTruthy(); + expect(popover).toContainReactComponent(Portal); }); it('renders an activator', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( Activator} onClose={spy} />, ); - const activator = findByTestID(popover, 'activator'); - expect(activator.exists()).toBeTruthy(); + expect(popover).toContainReactComponent('div', {testID: 'activator'}); }); it('renders a positionedOverlay when active is true', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( Activator} onClose={spy} />, ); - const positionedOverlay = findByTestID(popover, 'positionedOverlay'); - expect(positionedOverlay.exists()).toBeTruthy(); + expect(popover).toContainReactComponent(PositionedOverlay); }); it('doesn’t render a popover when active is false', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( Activator} onClose={spy} />, ); - const positionedOverlay = findByTestID(popover, 'positionedOverlay'); - expect(positionedOverlay.exists()).toBeFalsy(); + expect(popover).not.toContainReactComponent(PositionedOverlay); }); it("passes 'preferredPosition' to PopoverOverlay", () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( ', () => { onClose={spy} />, ); - const popoverOverlay = findByTestID(popover, 'popoverOverlay'); - expect(popoverOverlay.prop('preferredPosition')).toBe('above'); + + expect(popover).toContainReactComponent(PopoverOverlay, { + preferredPosition: 'above', + }); }); it("passes 'preferredAlignment' to PopoverOverlay", () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( ', () => { preferredAlignment="left" />, ); - const popoverOverlay = findByTestID(popover, 'popoverOverlay'); - expect(popoverOverlay.prop('preferredAlignment')).toBe('left'); + + expect(popover).toContainReactComponent(PopoverOverlay, { + preferredAlignment: 'left', + }); + }); + + it("passes 'preferInputActivator' to PopoverOverlay", () => { + const popover = mountWithApp( + Activator} + onClose={spy} + preferInputActivator={false} + />, + ); + + expect(popover).toContainReactComponent(PopoverOverlay, { + preferInputActivator: false, + }); }); it('has a div as activatorWrapper by default', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( ', () => { onClose={spy} />, ); - expect(popover.childAt(0).type()).toBe('div'); + expect(popover.children[0].type).toBe('div'); }); it('has a span as activatorWrapper when activatorWrapper prop is set to span', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( ', () => { onClose={spy} />, ); - expect(popover.childAt(0).type()).toBe('span'); + expect(popover.children[0].type).toBe('span'); }); it('passes preventAutofocus to PopoverOverlay', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( ', () => { onClose={spy} />, ); - const popoverOverlay = findByTestID(popover, 'popoverOverlay'); - expect(popoverOverlay.prop('preventAutofocus')).toBe(true); + + expect(popover).toContainReactComponent(PopoverOverlay, { + preventAutofocus: true, + }); }); it('passes sectioned to PopoverOverlay', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( ', () => { onClose={spy} />, ); - const popoverOverlay = findByTestID(popover, 'popoverOverlay'); - expect(popoverOverlay.prop('sectioned')).toBe(true); + + expect(popover).toContainReactComponent(PopoverOverlay, { + sectioned: true, + }); }); it('passes fullWidth to PopoverOverlay', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( ', () => { onClose={spy} />, ); - const popoverOverlay = findByTestID(popover, 'popoverOverlay'); - expect(popoverOverlay.prop('fullWidth')).toBe(true); + + expect(popover).toContainReactComponent(PopoverOverlay, { + fullWidth: true, + }); }); it('passes fluidContent to PopoverOverlay', () => { - const popover = mountWithAppProvider( + const popover = mountWithApp( ', () => { onClose={spy} />, ); - const popoverOverlay = findByTestID(popover, 'popoverOverlay'); - expect(popoverOverlay.prop('fluidContent')).toBe(true); + expect(popover).toContainReactComponent(PopoverOverlay, { + fluidContent: true, + }); }); it('calls onClose when you click outside the Popover', () => { - mountWithAppProvider( + mountWithApp( ', () => { const onCloseSpy = jest.fn(); - const popoverWithDisconnectedActivator = mountWithAppProvider( + const popoverWithDisconnectedActivator = mountWithApp( , ); - popoverWithDisconnectedActivator.find('button').simulate('click'); + popoverWithDisconnectedActivator.find('button')!.trigger('onClick'); const evt = new CustomEvent('click'); window.dispatchEvent(evt); diff --git a/src/components/PositionedOverlay/PositionedOverlay.tsx b/src/components/PositionedOverlay/PositionedOverlay.tsx index 38e1a79d2d4..00451af0abb 100644 --- a/src/components/PositionedOverlay/PositionedOverlay.tsx +++ b/src/components/PositionedOverlay/PositionedOverlay.tsx @@ -33,6 +33,7 @@ interface OverlayDetails { export interface PositionedOverlayProps { active: boolean; activator: HTMLElement; + preferInputActivator?: boolean; preferredPosition?: PreferredPosition; preferredAlignment?: PreferredAlignment; fullWidth?: boolean; @@ -195,14 +196,14 @@ export class PositionedOverlay extends React.PureComponent< onScrollOut, fullWidth, fixed, + preferInputActivator = true, } = this.props; - const textFieldActivator = activator.querySelector('input'); + const preferredActivator = preferInputActivator + ? activator.querySelector('input') || activator + : activator; - const activatorRect = - textFieldActivator != null - ? getRectForNode(textFieldActivator) - : getRectForNode(activator); + const activatorRect = getRectForNode(preferredActivator); const currentOverlayRect = getRectForNode(this.overlay); const scrollableElement = isDocument(this.scrollableContainer) diff --git a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx index 150b559ea9a..1c13d13aa37 100644 --- a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx +++ b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; // eslint-disable-next-line no-restricted-imports import {mountWithAppProvider} from 'test-utilities/legacy'; +import * as geometry from '@shopify/javascript-utilities/geometry'; import {EventListener} from '../../EventListener'; import {PositionedOverlay} from '../PositionedOverlay'; import * as mathModule from '../utilities/math'; @@ -131,6 +132,51 @@ describe('', () => { ); }); }); + + describe('preferInputActivator', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('uses the input to calculate its dimensions when true', () => { + const getRectForNodeSpy = jest.spyOn(geometry, 'getRectForNode'); + + const activator = document.createElement('div'); + const input = document.createElement('input'); + activator.appendChild(input); + + mountWithAppProvider( + , + ); + + expect( + getRectForNodeSpy.mock.calls.some(([node]) => node === input), + ).toBe(true); + }); + + it('does not use the input to calculate its dimensions when false', () => { + const getRectForNodeSpy = jest.spyOn(geometry, 'getRectForNode'); + const activator = document.createElement('div'); + const input = document.createElement('input'); + activator.appendChild(input); + + mountWithAppProvider( + , + ); + + expect( + getRectForNodeSpy.mock.calls.some(([node]) => node === input), + ).toBe(false); + }); + }); }); function mockRender() {