From f0da80531f706482da93e7be313de5c4fa72a778 Mon Sep 17 00:00:00 2001 From: beefchimi Date: Mon, 12 Jul 2021 13:07:18 -0400 Subject: [PATCH 1/5] :sparkles: [Popover] New mutationObserveConfig prop --- src/components/Popover/Popover.tsx | 4 ++++ .../PopoverOverlay/PopoverOverlay.tsx | 3 +++ .../tests/PopoverOverlay.test.tsx | 21 +++++++++++++++++++ src/components/Popover/tests/Popover.test.tsx | 19 +++++++++++++++++ .../PositionedOverlay/PositionedOverlay.tsx | 13 +++++++++--- .../tests/PositionedOverlay.test.tsx | 7 +++++++ 6 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/components/Popover/Popover.tsx b/src/components/Popover/Popover.tsx index c3106304ccc..31283f0d46b 100644 --- a/src/components/Popover/Popover.tsx +++ b/src/components/Popover/Popover.tsx @@ -44,6 +44,8 @@ export interface PopoverProps { * @default true */ preferInputActivator?: PopoverOverlayProps['preferInputActivator']; + /** Customize the MutationObserveInit object for more precise Popover re-rendering on `children` changes */ + mutationObserveConfig?: PopoverOverlayProps['mutationObserveConfig']; /** * The element type to wrap the activator with * @default 'div' @@ -96,6 +98,7 @@ export const Popover: React.FunctionComponent & { fixed, ariaHaspopup, preferInputActivator = true, + mutationObserveConfig, colorScheme, zIndexOverride, ...rest @@ -181,6 +184,7 @@ export const Popover: React.FunctionComponent & { id={id} activator={activatorNode} preferInputActivator={preferInputActivator} + mutationObserveConfig={mutationObserveConfig} onClose={handleClose} active={active} fixed={fixed} diff --git a/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx b/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx index f97e9539476..36690e27b0f 100644 --- a/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx +++ b/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx @@ -47,6 +47,7 @@ export interface PopoverOverlayProps { zIndexOverride?: number; activator: HTMLElement; preferInputActivator?: PositionedOverlayProps['preferInputActivator']; + mutationObserveConfig?: PositionedOverlayProps['mutationObserveConfig']; sectioned?: boolean; fixed?: boolean; hideOnPrint?: boolean; @@ -120,6 +121,7 @@ export class PopoverOverlay extends PureComponent { preferInputActivator = true, fixed, zIndexOverride, + mutationObserveConfig, } = this.props; const {transitionStatus} = this.state; if (transitionStatus === TransitionStatus.Exited && !active) return null; @@ -148,6 +150,7 @@ export class PopoverOverlay extends PureComponent { onScrollOut={this.handleScrollOut} classNames={className} zIndexOverride={zIndexOverride} + mutationObserveConfig={mutationObserveConfig} /> ); } diff --git a/src/components/Popover/components/PopoverOverlay/tests/PopoverOverlay.test.tsx b/src/components/Popover/components/PopoverOverlay/tests/PopoverOverlay.test.tsx index 34255fe5b3e..eb38d069cd1 100644 --- a/src/components/Popover/components/PopoverOverlay/tests/PopoverOverlay.test.tsx +++ b/src/components/Popover/components/PopoverOverlay/tests/PopoverOverlay.test.tsx @@ -188,6 +188,27 @@ describe('', () => { }); }); + it('passes mutationObserveConfig to PositionedOverlay', () => { + const mockMutationObserveConfig = { + attributes: true, + }; + const popoverOverlay = mountWithApp( + + {children} + , + ); + + expect(popoverOverlay).toContainReactComponent(PositionedOverlay, { + mutationObserveConfig: mockMutationObserveConfig, + }); + }); + it("doesn't include a tabindex prop when autofocusTarget is 'none'", () => { const popoverOverlay = mountWithAppProvider( ', () => { }); }); + it('passes mutationObserveConfig to PopoverOverlay', () => { + const mockMutationObserveConfig = { + attributes: true, + }; + const popover = mountWithApp( + Activator} + onClose={spy} + mutationObserveConfig={mockMutationObserveConfig} + />, + ); + + expect(popover).toContainReactComponent(PopoverOverlay, { + mutationObserveConfig: mockMutationObserveConfig, + }); + }); + it('has a div as activatorWrapper by default', () => { const popover = mountWithApp( { if (!this.overlay) return; - this.observer.observe(this.overlay, OBSERVER_CONFIG); - this.observer.observe(activator, OBSERVER_CONFIG); + + this.observer.observe(this.overlay, { + ...DEFAULT_OBSERVER_CONFIG, + ...mutationObserveConfig, + }); + + this.observer.observe(activator, DEFAULT_OBSERVER_CONFIG); }, ); }, diff --git a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx index 1bd293a64b3..dc7f08eeb13 100644 --- a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx +++ b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx @@ -275,6 +275,13 @@ describe('', () => { ).toBe(false); }); }); + + describe('mutationObserveConfig', () => { + // Find the `overlay` and compare against DEFAULT_OBSERVER_CONFIG + it.todo('passes default config'); + // Find the `overlay` and compare against mockMutationObserveConfig + it.todo('merges prop with default config'); + }); }); function mockRender() { From 803b61f713cc14516dd114473baf59b8ec43e456 Mon Sep 17 00:00:00 2001 From: beefchimi Date: Mon, 19 Jul 2021 13:50:27 -0400 Subject: [PATCH 2/5] :alembic: [PositionedOverlay] Attempt mutation observer tests --- .../PositionedOverlay/PositionedOverlay.tsx | 2 +- .../tests/PositionedOverlay.test.tsx | 48 +++++++++++++++++-- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/components/PositionedOverlay/PositionedOverlay.tsx b/src/components/PositionedOverlay/PositionedOverlay.tsx index 0b288286838..59a638096ae 100644 --- a/src/components/PositionedOverlay/PositionedOverlay.tsx +++ b/src/components/PositionedOverlay/PositionedOverlay.tsx @@ -58,7 +58,7 @@ interface State { lockPosition: boolean; } -const DEFAULT_OBSERVER_CONFIG: PositionedOverlayProps['mutationObserveConfig'] = { +export const DEFAULT_OBSERVER_CONFIG: PositionedOverlayProps['mutationObserveConfig'] = { childList: true, subtree: true, characterData: true, diff --git a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx index dc7f08eeb13..9fd73122b4c 100644 --- a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx +++ b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx @@ -3,7 +3,7 @@ import React from 'react'; import {mountWithAppProvider} from 'test-utilities/legacy'; import {EventListener} from '../../EventListener'; -import {PositionedOverlay} from '../PositionedOverlay'; +import {PositionedOverlay, DEFAULT_OBSERVER_CONFIG} from '../PositionedOverlay'; import * as mathModule from '../utilities/math'; import * as geometry from '../../../utilities/geometry'; import styles from '../PositionedOverlay.scss'; @@ -277,10 +277,48 @@ describe('', () => { }); describe('mutationObserveConfig', () => { - // Find the `overlay` and compare against DEFAULT_OBSERVER_CONFIG - it.todo('passes default config'); - // Find the `overlay` and compare against mockMutationObserveConfig - it.todo('merges prop with default config'); + let mutationObserverObserveSpy: jest.SpyInstance; + + beforeEach(() => { + mutationObserverObserveSpy = jest.spyOn( + MutationObserver.prototype, + 'observe', + ); + }); + + afterEach(() => { + mutationObserverObserveSpy.mockRestore(); + }); + + it('passes default config', () => { + mountWithAppProvider(); + + expect(mutationObserverObserveSpy).toHaveBeenCalledWith( + expect.any(Function), + DEFAULT_OBSERVER_CONFIG, + ); + }); + + it('merges prop with default config', () => { + const mockMutationObserveConfig = { + subtree: false, + attributes: true, + }; + mountWithAppProvider( + , + ); + + expect(mutationObserverObserveSpy).toHaveBeenCalledWith( + expect.any(Function), + { + ...DEFAULT_OBSERVER_CONFIG, + ...mockMutationObserveConfig, + }, + ); + }); }); }); From 5b75caa0342cf6ab88a5de4cf865ceace07375d8 Mon Sep 17 00:00:00 2001 From: beefchimi Date: Mon, 19 Jul 2021 14:47:42 -0400 Subject: [PATCH 3/5] :construction_worker: [PositionedOverlay] Fix tests --- .../tests/PositionedOverlay.test.tsx | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx index 9fd73122b4c..c4985fc7376 100644 --- a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx +++ b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx @@ -291,10 +291,13 @@ describe('', () => { }); it('passes default config', () => { - mountWithAppProvider(); + const positionedOverlay = mountWithAppProvider( + , + ); + const element = positionedOverlay.find('div').getDOMNode(); expect(mutationObserverObserveSpy).toHaveBeenCalledWith( - expect.any(Function), + element, DEFAULT_OBSERVER_CONFIG, ); }); @@ -304,20 +307,18 @@ describe('', () => { subtree: false, attributes: true, }; - mountWithAppProvider( + const positionedOverlay = mountWithAppProvider( , ); + const element = positionedOverlay.find('div').getDOMNode(); - expect(mutationObserverObserveSpy).toHaveBeenCalledWith( - expect.any(Function), - { - ...DEFAULT_OBSERVER_CONFIG, - ...mockMutationObserveConfig, - }, - ); + expect(mutationObserverObserveSpy).toHaveBeenCalledWith(element, { + ...DEFAULT_OBSERVER_CONFIG, + ...mockMutationObserveConfig, + }); }); }); }); From 66aff58947402b6434f0dc6e42d789e5f05dbdae Mon Sep 17 00:00:00 2001 From: beefchimi Date: Mon, 12 Jul 2021 14:15:29 -0400 Subject: [PATCH 4/5] :pencil2: [UNRELEASED] Add notes --- UNRELEASED.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index 8dae4c28da3..445ef58f6dd 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -12,6 +12,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f - Added `ariaLabelledBy` props to `Navigation` component to allow a hidden label for accessibility ([#4343](https://github.com/Shopify/polaris-react/pull/4343)) - Add `lastColumnSticky` prop to `IndexTable` to create a sticky last cell and optional sticky last heading on viewports larger than small ([#4150](https://github.com/Shopify/polaris-react/pull/4150)) - Allow promoted actions to be rendered as a menu on the `BulkAction` component ([#4266](https://github.com/Shopify/polaris-react/pull/4266)) +- Added `mutationObserveConfig` prop to `Popover` and `PositionedOverlay` ([#4303](https://github.com/Shopify/polaris-react/pull/4303)) ### Bug fixes From 29339947e73ede190765c0eae428a9f5ec8c675c Mon Sep 17 00:00:00 2001 From: beefchimi Date: Thu, 5 Aug 2021 08:41:56 -0400 Subject: [PATCH 5/5] :recycle: [Test] Update test after rebase --- .../tests/PositionedOverlay.test.tsx | 106 ++++++++---------- 1 file changed, 47 insertions(+), 59 deletions(-) diff --git a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx index c4985fc7376..4ef9529d52a 100644 --- a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx +++ b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx @@ -38,20 +38,54 @@ describe('', () => { mutationObserverObserveSpy.mockRestore(); }); - it('observers the activator', () => { - const activator = document.createElement('button'); - mountWithAppProvider( - , - ); + describe('overlay', () => { + it('passes default config to', () => { + const positionedOverlay = mountWithAppProvider( + , + ); + const element = positionedOverlay.find('div').getDOMNode(); + + expect(mutationObserverObserveSpy).toHaveBeenCalledWith( + element, + DEFAULT_OBSERVER_CONFIG, + ); + }); + + it('merges prop with default config', () => { + const mockMutationObserveConfig = { + subtree: false, + attributes: true, + }; + const positionedOverlay = mountWithAppProvider( + , + ); + const element = positionedOverlay.find('div').getDOMNode(); + + expect(mutationObserverObserveSpy).toHaveBeenCalledWith(element, { + ...DEFAULT_OBSERVER_CONFIG, + ...mockMutationObserveConfig, + }); + }); + }); - expect(mutationObserverObserveSpy).toHaveBeenCalledWith(activator, { - characterData: true, - childList: true, - subtree: true, + describe('activator', () => { + it('observers the activator', () => { + const activator = document.createElement('button'); + + mountWithAppProvider( + , + ); + + expect(mutationObserverObserveSpy).toHaveBeenCalledWith(activator, { + ...DEFAULT_OBSERVER_CONFIG, + }); }); }); }); @@ -275,52 +309,6 @@ describe('', () => { ).toBe(false); }); }); - - describe('mutationObserveConfig', () => { - let mutationObserverObserveSpy: jest.SpyInstance; - - beforeEach(() => { - mutationObserverObserveSpy = jest.spyOn( - MutationObserver.prototype, - 'observe', - ); - }); - - afterEach(() => { - mutationObserverObserveSpy.mockRestore(); - }); - - it('passes default config', () => { - const positionedOverlay = mountWithAppProvider( - , - ); - const element = positionedOverlay.find('div').getDOMNode(); - - expect(mutationObserverObserveSpy).toHaveBeenCalledWith( - element, - DEFAULT_OBSERVER_CONFIG, - ); - }); - - it('merges prop with default config', () => { - const mockMutationObserveConfig = { - subtree: false, - attributes: true, - }; - const positionedOverlay = mountWithAppProvider( - , - ); - const element = positionedOverlay.find('div').getDOMNode(); - - expect(mutationObserverObserveSpy).toHaveBeenCalledWith(element, { - ...DEFAULT_OBSERVER_CONFIG, - ...mockMutationObserveConfig, - }); - }); - }); }); function mockRender() {