From 94d3ed7af36db5f0d23d70a162226082dca7edda Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Sat, 8 Dec 2018 22:02:43 +0100 Subject: [PATCH 1/4] Fix `setPopperNode` only to update popper instance when popperNode is HTMLElement --- .size-snapshot.json | 22 ++++++++++---------- src/Popper.js | 27 ++++++++++++------------ src/Popper.test.js | 2 +- src/__mocks__/popper.js.js | 30 ++++++++++++++++++++++++++- src/__snapshots__/Popper.test.js.snap | 5 +---- yarn.lock | 3 ++- 6 files changed, 57 insertions(+), 32 deletions(-) diff --git a/.size-snapshot.json b/.size-snapshot.json index 6d76dee..de3262d 100644 --- a/.size-snapshot.json +++ b/.size-snapshot.json @@ -1,25 +1,25 @@ { "dist/index.umd.js": { - "bundled": 49703, - "minified": 17814, - "gzipped": 5717 + "bundled": 49757, + "minified": 17740, + "gzipped": 5565 }, "dist/index.umd.min.js": { - "bundled": 25362, - "minified": 10181, - "gzipped": 3420 + "bundled": 24384, + "minified": 10009, + "gzipped": 3336 }, "dist/index.esm.js": { - "bundled": 9794, - "minified": 5639, - "gzipped": 1769, + "bundled": 10112, + "minified": 5788, + "gzipped": 1827, "treeshaked": { "rollup": { - "code": 4637, + "code": 4593, "import_statements": 137 }, "webpack": { - "code": 5371 + "code": 5714 } } } diff --git a/src/Popper.js b/src/Popper.js index 7f59160..1946fc1 100644 --- a/src/Popper.js +++ b/src/Popper.js @@ -11,7 +11,7 @@ import type { Style } from 'typed-styles'; import { ManagerContext } from './Manager'; import { safeInvoke, unwrapArray } from './utils'; -type getRefFn = (?HTMLElement) => void; +type getRefFn = (HTMLElement | null) => void; type ReferenceElement = ReferenceObject | HTMLElement | null; type StyleOffsets = { top: number, left: number }; type StylePosition = { position: 'absolute' | 'fixed' }; @@ -59,7 +59,7 @@ export class InnerPopper extends React.Component { static defaultProps = { placement: 'bottom', eventsEnabled: true, - referenceElement: undefined, + referenceElement: null, positionFixed: false, }; @@ -68,13 +68,13 @@ export class InnerPopper extends React.Component { placement: undefined, }; - popperInstance: ?Instance; + popperInstance: Instance | null; - popperNode: ?HTMLElement = null; - arrowNode: ?HTMLElement = null; + popperNode: HTMLElement | null = null; + arrowNode: HTMLElement | null = null; - setPopperNode = (popperNode: ?HTMLElement) => { - if (this.popperNode === popperNode) return; + setPopperNode = (popperNode: HTMLElement | null) => { + if (!popperNode || this.popperNode === popperNode) return; safeInvoke(this.props.innerRef, popperNode); this.popperNode = popperNode; @@ -82,8 +82,8 @@ export class InnerPopper extends React.Component { this.updatePopperInstance(); }; - setArrowNode = (arrowNode: ?HTMLElement) => { - if (this.arrowNode === arrowNode) return; + setArrowNode = (arrowNode: HTMLElement | null) => { + if (!arrowNode || this.arrowNode === arrowNode) return; this.arrowNode = arrowNode; if (!this.popperInstance) this.updatePopperInstance(); @@ -94,10 +94,7 @@ export class InnerPopper extends React.Component { order: 900, fn: (data: Object) => { const { placement } = data; - this.setState( - { data, placement }, - placement !== this.state.placement ? this.scheduleUpdate : undefined - ); + this.setState({ data, placement }); return data; }, }; @@ -218,7 +215,9 @@ export default function Popper({ referenceElement, ...props }: PopperProps) { {({ referenceNode }) => ( )} diff --git a/src/Popper.test.js b/src/Popper.test.js index 81e52f2..ee88642 100644 --- a/src/Popper.test.js +++ b/src/Popper.test.js @@ -71,7 +71,7 @@ describe('Popper component', () => { const referenceElement = document.createElement('div'); expect(() => mount( - + {({ ref, style, placement, arrowProps }) => (
ref(current)} diff --git a/src/__mocks__/popper.js.js b/src/__mocks__/popper.js.js index 48e85cc..d76e811 100644 --- a/src/__mocks__/popper.js.js +++ b/src/__mocks__/popper.js.js @@ -8,13 +8,41 @@ export default class Popper { }; constructor(reference, popper, options = {}) { + const modifiers = Object.keys(options.modifiers) + .map(name => ({ + name, + ...options.modifiers[name], + })) + .sort((a, b) => a.order - b.order); + + const update = () => { + const data = { + placement: options.placement, + arrowStyles: {}, + offsets: { + popper: { + position: 'absolute', + }, + reference: {}, + }, + }; + modifiers.forEach(m => { + if (m.enabled && m.fn) { + m.fn(data, m); + } + }); + }; + update(); + return { reference, popper, options: { ...Popper.Defaults, ...options }, state: this.state, destroy: () => (this.state.isDestroyed = true), - scheduleUpdate: () => {}, + scheduleUpdate: () => { + update(); + }, }; } } diff --git a/src/__snapshots__/Popper.test.js.snap b/src/__snapshots__/Popper.test.js.snap index af3a369..83c8f44 100644 --- a/src/__snapshots__/Popper.test.js.snap +++ b/src/__snapshots__/Popper.test.js.snap @@ -8,13 +8,10 @@ exports[`Popper component renders the expected markup 1`] = ` referenceElement={
} >
diff --git a/yarn.lock b/yarn.lock index 71651fd..fdbd442 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2048,9 +2048,10 @@ create-hmac@^1.1.0, create-hmac@^1.1.2, create-hmac@^1.1.4: safe-buffer "^5.0.1" sha.js "^2.4.8" -create-react-context@^0.2.1, create-react-context@^0.2.2: +create-react-context@<=0.2.2, create-react-context@^0.2.2: version "0.2.2" resolved "https://registry.yarnpkg.com/create-react-context/-/create-react-context-0.2.2.tgz#9836542f9aaa22868cd7d4a6f82667df38019dca" + integrity sha512-KkpaLARMhsTsgp0d2NA/R94F/eDLbhXERdIq3LvX2biCAXcDvHYoOqHfWCHf1+OLj+HKBotLG3KqaOOf+C1C+A== dependencies: fbjs "^0.8.0" gud "^1.0.0" From ac3664d96560043e7fd3bc5f1af3fc9fbb186f56 Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Sun, 9 Dec 2018 10:57:40 +0100 Subject: [PATCH 2/4] Revert type changes --- .size-snapshot.json | 22 +++++++++++----------- src/Popper.js | 19 +++++++++++-------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/.size-snapshot.json b/.size-snapshot.json index de3262d..496c69b 100644 --- a/.size-snapshot.json +++ b/.size-snapshot.json @@ -1,25 +1,25 @@ { "dist/index.umd.js": { - "bundled": 49757, - "minified": 17740, - "gzipped": 5565 + "bundled": 49834, + "minified": 17788, + "gzipped": 5577 }, "dist/index.umd.min.js": { - "bundled": 24384, - "minified": 10009, - "gzipped": 3336 + "bundled": 24461, + "minified": 10057, + "gzipped": 3347 }, "dist/index.esm.js": { - "bundled": 10112, - "minified": 5788, - "gzipped": 1827, + "bundled": 10189, + "minified": 5836, + "gzipped": 1838, "treeshaked": { "rollup": { - "code": 4593, + "code": 4641, "import_statements": 137 }, "webpack": { - "code": 5714 + "code": 5762 } } } diff --git a/src/Popper.js b/src/Popper.js index 1946fc1..4f391f8 100644 --- a/src/Popper.js +++ b/src/Popper.js @@ -11,7 +11,7 @@ import type { Style } from 'typed-styles'; import { ManagerContext } from './Manager'; import { safeInvoke, unwrapArray } from './utils'; -type getRefFn = (HTMLElement | null) => void; +type getRefFn = (?HTMLElement) => void; type ReferenceElement = ReferenceObject | HTMLElement | null; type StyleOffsets = { top: number, left: number }; type StylePosition = { position: 'absolute' | 'fixed' }; @@ -59,7 +59,7 @@ export class InnerPopper extends React.Component { static defaultProps = { placement: 'bottom', eventsEnabled: true, - referenceElement: null, + referenceElement: undefined, positionFixed: false, }; @@ -68,12 +68,12 @@ export class InnerPopper extends React.Component { placement: undefined, }; - popperInstance: Instance | null; + popperInstance: ?Instance; - popperNode: HTMLElement | null = null; - arrowNode: HTMLElement | null = null; + popperNode: ?HTMLElement = null; + arrowNode: ?HTMLElement = null; - setPopperNode = (popperNode: HTMLElement | null) => { + setPopperNode = (popperNode: ?HTMLElement) => { if (!popperNode || this.popperNode === popperNode) return; safeInvoke(this.props.innerRef, popperNode); @@ -82,7 +82,7 @@ export class InnerPopper extends React.Component { this.updatePopperInstance(); }; - setArrowNode = (arrowNode: HTMLElement | null) => { + setArrowNode = (arrowNode: ?HTMLElement) => { if (!arrowNode || this.arrowNode === arrowNode) return; this.arrowNode = arrowNode; @@ -94,7 +94,10 @@ export class InnerPopper extends React.Component { order: 900, fn: (data: Object) => { const { placement } = data; - this.setState({ data, placement }); + this.setState( + { data, placement }, + placement !== this.state.placement ? this.scheduleUpdate : undefined + ); return data; }, }; From 0f311560705fa78659a7b5f65f657b8bc7b93ffa Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Sun, 9 Dec 2018 11:00:22 +0100 Subject: [PATCH 3/4] Simplify setArrowNode --- .size-snapshot.json | 22 +++++++++++----------- src/Popper.js | 3 --- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/.size-snapshot.json b/.size-snapshot.json index 496c69b..39baa86 100644 --- a/.size-snapshot.json +++ b/.size-snapshot.json @@ -1,25 +1,25 @@ { "dist/index.umd.js": { - "bundled": 49834, - "minified": 17788, - "gzipped": 5577 + "bundled": 49705, + "minified": 17723, + "gzipped": 5569 }, "dist/index.umd.min.js": { - "bundled": 24461, - "minified": 10057, - "gzipped": 3347 + "bundled": 24332, + "minified": 9992, + "gzipped": 3338 }, "dist/index.esm.js": { - "bundled": 10189, - "minified": 5836, - "gzipped": 1838, + "bundled": 10062, + "minified": 5771, + "gzipped": 1829, "treeshaked": { "rollup": { - "code": 4641, + "code": 4576, "import_statements": 137 }, "webpack": { - "code": 5762 + "code": 5697 } } } diff --git a/src/Popper.js b/src/Popper.js index 4f391f8..599a429 100644 --- a/src/Popper.js +++ b/src/Popper.js @@ -83,10 +83,7 @@ export class InnerPopper extends React.Component { }; setArrowNode = (arrowNode: ?HTMLElement) => { - if (!arrowNode || this.arrowNode === arrowNode) return; this.arrowNode = arrowNode; - - if (!this.popperInstance) this.updatePopperInstance(); }; updateStateModifier = { From e75aeff5a46f46507b208b765f8c2471ae5757b0 Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Sun, 9 Dec 2018 12:06:27 +0100 Subject: [PATCH 4/4] Clear popperNode ref on unmount when innerRef is used --- .size-snapshot.json | 22 +++++++++++----------- src/Popper.js | 1 + 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/.size-snapshot.json b/.size-snapshot.json index 39baa86..2cd38d6 100644 --- a/.size-snapshot.json +++ b/.size-snapshot.json @@ -1,25 +1,25 @@ { "dist/index.umd.js": { - "bundled": 49705, - "minified": 17723, - "gzipped": 5569 + "bundled": 49749, + "minified": 17751, + "gzipped": 5578 }, "dist/index.umd.min.js": { - "bundled": 24332, - "minified": 9992, - "gzipped": 3338 + "bundled": 24376, + "minified": 10020, + "gzipped": 3347 }, "dist/index.esm.js": { - "bundled": 10062, - "minified": 5771, - "gzipped": 1829, + "bundled": 10105, + "minified": 5808, + "gzipped": 1838, "treeshaked": { "rollup": { - "code": 4576, + "code": 4604, "import_statements": 137 }, "webpack": { - "code": 5697 + "code": 5725 } } } diff --git a/src/Popper.js b/src/Popper.js index 599a429..72fc56e 100644 --- a/src/Popper.js +++ b/src/Popper.js @@ -189,6 +189,7 @@ export class InnerPopper extends React.Component { } componentWillUnmount() { + safeInvoke(this.props.innerRef, null); this.destroyPopperInstance(); }