From 5536c3b2c4633ac3cf7c7c1056719da2f9b8576a Mon Sep 17 00:00:00 2001 From: Andrew Musgrave Date: Tue, 17 Dec 2019 12:43:15 -0500 Subject: [PATCH 1/5] Fixed document being referenced --- src/components/DropZone/DropZone.tsx | 36 ++++++++++++++++++++++++---- src/utilities/use-event-listener.ts | 8 ++++--- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/components/DropZone/DropZone.tsx b/src/components/DropZone/DropZone.tsx index 441062930d4..4e8021d8f9a 100755 --- a/src/components/DropZone/DropZone.tsx +++ b/src/components/DropZone/DropZone.tsx @@ -180,7 +180,9 @@ export const DropZone: React.FunctionComponent & { const [measuring, setMeasuring] = useState(true); const i18n = useI18n(); - const dropNode = dropOnPage ? document : node; + const dropNode = dropOnPage + ? (typeof document !== undefined && document) || null + : node; const getValidatedFiles = useCallback( (files: File[] | DataTransferItem[]) => { @@ -297,10 +299,34 @@ export const DropZone: React.FunctionComponent & { [allowMultiple, disabled, dropNode, numFiles, onDragLeave], ); - useEventListener(dropNode, 'drop', handleDrop); - useEventListener(dropNode, 'dragover', handleDragOver); - useEventListener(dropNode, 'dragenter', handleDragEnter); - useEventListener(dropNode, 'dragleave', handleDragLeave); + useEventListener( + dropNode, + 'drop', + handleDrop, + {}, + {defaultToDocument: dropOnPage}, + ); + useEventListener( + dropNode, + 'dragover', + handleDragOver, + {}, + {defaultToDocument: dropOnPage}, + ); + useEventListener( + dropNode, + 'dragenter', + handleDragEnter, + {}, + {defaultToDocument: dropOnPage}, + ); + useEventListener( + dropNode, + 'dragleave', + handleDragLeave, + {}, + {defaultToDocument: dropOnPage}, + ); useEventListener(null, 'resize', adjustSize, {}, {defaultToWindow: true}); useComponentDidMount(() => { diff --git a/src/utilities/use-event-listener.ts b/src/utilities/use-event-listener.ts index a84048e072d..7b516292e09 100644 --- a/src/utilities/use-event-listener.ts +++ b/src/utilities/use-event-listener.ts @@ -1,7 +1,8 @@ import {useEffect, RefObject} from 'react'; interface Options { - defaultToWindow: boolean; + defaultToWindow?: boolean; + defaultToDocument?: boolean; } /** @@ -32,8 +33,9 @@ export function useEventListener( ) { useEffect(() => { let eventTarget = target && 'current' in target ? target.current : target; - if (!eventTarget && options && options.defaultToWindow) { - eventTarget = window; + if (!eventTarget && options) { + if (options.defaultToWindow) eventTarget = window; + else if (options.defaultToDocument) eventTarget = document; } if (!eventTarget) return; From ea5b1ee3446fa496a80d4182a2eb6d907f799f72 Mon Sep 17 00:00:00 2001 From: Andrew Musgrave Date: Tue, 17 Dec 2019 12:46:14 -0500 Subject: [PATCH 2/5] changelog --- UNRELEASED.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index 477d7d9d910..d06d29acd6a 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -8,6 +8,7 @@ - Fixed `TextField` to no longer render `aria-invalid="false"` ([#2282](https://github.com/Shopify/polaris-react/pull/2282)) - Fixed `TextField` to only render `min` ,`max` and `step` attributes when explicitly passed ([#2282](https://github.com/Shopify/polaris-react/pull/2282)) +- Removed reference to `document` in `DropZone` ([#2560](https://github.com/Shopify/polaris-react/pull/2560)) ### Documentation From e2cf1dc5e8f19242540b47d15df9fc51fe8d8c51 Mon Sep 17 00:00:00 2001 From: Andrew Musgrave Date: Tue, 17 Dec 2019 13:25:05 -0500 Subject: [PATCH 3/5] Removed usage of useEventListener --- src/components/DropZone/DropZone.tsx | 63 ++++++++++++---------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/src/components/DropZone/DropZone.tsx b/src/components/DropZone/DropZone.tsx index 4e8021d8f9a..9ad45b1bc57 100755 --- a/src/components/DropZone/DropZone.tsx +++ b/src/components/DropZone/DropZone.tsx @@ -7,6 +7,10 @@ import React, { useEffect, } from 'react'; import debounce from 'lodash/debounce'; +import { + addEventListener, + removeEventListener, +} from '@shopify/javascript-utilities/events'; import { DragDropMajorMonotone, CircleAlertMajorMonotone, @@ -21,7 +25,7 @@ import {DisplayText} from '../DisplayText'; import {VisuallyHidden} from '../VisuallyHidden'; import {Labelled, Action} from '../Labelled'; import {useI18n} from '../../utilities/i18n'; -import {useEventListener} from '../../utilities/use-event-listener'; +import {isServer} from '../../utilities/target'; import {useUniqueId} from '../../utilities/unique-id'; import {useComponentDidMount} from '../../utilities/use-component-did-mount'; import {useToggle} from '../../utilities/use-toggle'; @@ -180,9 +184,6 @@ export const DropZone: React.FunctionComponent & { const [measuring, setMeasuring] = useState(true); const i18n = useI18n(); - const dropNode = dropOnPage - ? (typeof document !== undefined && document) || null - : node; const getValidatedFiles = useCallback( (files: File[] | DataTransferItem[]) => { @@ -284,8 +285,8 @@ export const DropZone: React.FunctionComponent & { if (disabled || (!allowMultiple && numFiles > 0)) return; dragTargets.current = dragTargets.current.filter((el: Node) => { - const compareNode = - dropNode && 'current' in dropNode ? dropNode.current : document; + const compareNode = dropOnPage && !isServer ? document : node.current; + return el !== event.target && compareNode && compareNode.contains(el); }); @@ -296,38 +297,28 @@ export const DropZone: React.FunctionComponent & { onDragLeave && onDragLeave(); }, - [allowMultiple, disabled, dropNode, numFiles, onDragLeave], + [allowMultiple, disabled, numFiles, onDragLeave], ); - useEventListener( - dropNode, - 'drop', - handleDrop, - {}, - {defaultToDocument: dropOnPage}, - ); - useEventListener( - dropNode, - 'dragover', - handleDragOver, - {}, - {defaultToDocument: dropOnPage}, - ); - useEventListener( - dropNode, - 'dragenter', - handleDragEnter, - {}, - {defaultToDocument: dropOnPage}, - ); - useEventListener( - dropNode, - 'dragleave', - handleDragLeave, - {}, - {defaultToDocument: dropOnPage}, - ); - useEventListener(null, 'resize', adjustSize, {}, {defaultToWindow: true}); + useEffect(() => { + const dropNode = dropOnPage ? document : node.current; + + if (!dropNode) return; + + addEventListener(dropNode, 'drop', handleDrop); + addEventListener(dropNode, 'dragover', handleDragOver); + addEventListener(dropNode, 'dragenter', handleDragEnter); + addEventListener(dropNode, 'dragleave', handleDragLeave); + addEventListener(window, 'resize', adjustSize); + + return () => { + removeEventListener(dropNode, 'drop', handleDrop); + removeEventListener(dropNode, 'dragover', handleDragOver); + removeEventListener(dropNode, 'dragenter', handleDragEnter); + removeEventListener(dropNode, 'dragleave', handleDragLeave); + removeEventListener(window, 'resize', adjustSize); + }; + }); useComponentDidMount(() => { adjustSize(); From 139b91a2eca677a4cd14eaca6da7a8792491bf37 Mon Sep 17 00:00:00 2001 From: Andrew Musgrave Date: Tue, 17 Dec 2019 13:35:34 -0500 Subject: [PATCH 4/5] Removed useEventListener --- src/components/DropZone/DropZone.tsx | 11 ++- .../tests/use-event-listener.test.tsx | 93 ------------------- src/utilities/use-event-listener.ts | 49 ---------- 3 files changed, 9 insertions(+), 144 deletions(-) delete mode 100644 src/utilities/tests/use-event-listener.test.tsx delete mode 100644 src/utilities/use-event-listener.ts diff --git a/src/components/DropZone/DropZone.tsx b/src/components/DropZone/DropZone.tsx index 9ad45b1bc57..a5d9c73b94b 100755 --- a/src/components/DropZone/DropZone.tsx +++ b/src/components/DropZone/DropZone.tsx @@ -297,7 +297,7 @@ export const DropZone: React.FunctionComponent & { onDragLeave && onDragLeave(); }, - [allowMultiple, disabled, numFiles, onDragLeave], + [allowMultiple, dropOnPage, disabled, numFiles, onDragLeave], ); useEffect(() => { @@ -318,7 +318,14 @@ export const DropZone: React.FunctionComponent & { removeEventListener(dropNode, 'dragleave', handleDragLeave); removeEventListener(window, 'resize', adjustSize); }; - }); + }, [ + dropOnPage, + handleDrop, + handleDragOver, + handleDragEnter, + handleDragLeave, + adjustSize, + ]); useComponentDidMount(() => { adjustSize(); diff --git a/src/utilities/tests/use-event-listener.test.tsx b/src/utilities/tests/use-event-listener.test.tsx deleted file mode 100644 index b3be49505f5..00000000000 --- a/src/utilities/tests/use-event-listener.test.tsx +++ /dev/null @@ -1,93 +0,0 @@ -import React, {useRef, useEffect} from 'react'; -import {mount} from 'test-utilities'; -import {useEventListener} from '../use-event-listener'; - -describe('useEventListener', () => { - let addEventListenerSpy: jest.SpyInstance; - let removeEventListenerSpy: jest.SpyInstance; - - beforeEach(() => { - addEventListenerSpy = jest.spyOn(document, 'addEventListener'); - removeEventListenerSpy = jest.spyOn(document, 'removeEventListener'); - }); - - afterEach(() => { - addEventListenerSpy.mockRestore(); - removeEventListenerSpy.mockRestore(); - }); - - it('adds event to react refs', () => { - const spy = jest.fn(); - function Component() { - const div = useRef(null); - - useEventListener(div, 'blur', spy); - - useEffect(() => { - div.current!.dispatchEvent(new Event('blur')); - }); - - return
; - } - - mount(); - expect(spy).toHaveBeenCalledTimes(1); - }); - - it('invokes the provided handler when the type of event is triggered', () => { - const spy = jest.fn(); - function Component() { - useEventListener(document, 'reset', spy); - - return null; - } - - mount(); - document.dispatchEvent(new Event('reset')); - expect(spy).toHaveBeenCalledTimes(1); - }); - - it('passes type, handler and options to the listener', () => { - const options = {passive: true}; - const noop = () => {}; - function Component() { - useEventListener(document, 'reset', noop, options); - - return null; - } - - mount(); - expect(addEventListenerSpy).toHaveBeenCalledWith('reset', noop, options); - }); - - it('removes the event listener when unmounting', () => { - function Component() { - useEventListener(document, 'reset', () => {}); - - return null; - } - - const component = mount(); - expect(removeEventListenerSpy).toHaveBeenCalledTimes(0); - component.unmount(); - expect(removeEventListenerSpy).toHaveBeenCalledTimes(1); - }); - - it('defaults to window when the option is set', () => { - const spy = jest.fn(); - function Component() { - const div = useRef(null); - - useEventListener(div, 'blur', spy, {}, {defaultToWindow: true}); - - useEffect(() => { - window.dispatchEvent(new Event('blur')); - }); - - return
; - } - - mount(); - expect(spy).toHaveBeenCalledTimes(1); - }); -}); diff --git a/src/utilities/use-event-listener.ts b/src/utilities/use-event-listener.ts deleted file mode 100644 index 7b516292e09..00000000000 --- a/src/utilities/use-event-listener.ts +++ /dev/null @@ -1,49 +0,0 @@ -import {useEffect, RefObject} from 'react'; - -interface Options { - defaultToWindow?: boolean; - defaultToDocument?: boolean; -} - -/** - * Attaches and removes event listeners from the target - * @param target Defines a target for the listener to be placed on. Defaults to window. - * @param type Defines the type of event, i.e blur or focus - * @param handler Defines a callback to be invoked when the event type occurs - * @param listenerOptions Object that specifies event properties - * @param options Object that defines properties used in the hook - * interface Options { - * // Uses window as a back up event target when the current - * // target is falsy - * defaultToWindow: boolean; - * } - * @example - * function Playground() { - * useEventListener(window, 'resize', () => console.log('resize')); - * - * return null; - * } - */ -export function useEventListener( - target: RefObject | Window | Document | null, - type: K, - handler: (ev: WindowEventMap[K]) => any, - listenerOptions?: boolean | AddEventListenerOptions, - options?: Options, -) { - useEffect(() => { - let eventTarget = target && 'current' in target ? target.current : target; - if (!eventTarget && options) { - if (options.defaultToWindow) eventTarget = window; - else if (options.defaultToDocument) eventTarget = document; - } - - if (!eventTarget) return; - - eventTarget.addEventListener(type, handler, listenerOptions); - return () => { - eventTarget && - eventTarget.removeEventListener(type, handler, listenerOptions); - }; - }, [handler, listenerOptions, options, target, type]); -} From 0e03cdd1a5c01caefddc66cac978b541c92f2ad6 Mon Sep 17 00:00:00 2001 From: Andrew Musgrave Date: Tue, 17 Dec 2019 14:13:53 -0500 Subject: [PATCH 5/5] Re-run stalling percy job