This repository was archived by the owner on Mar 4, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 53
chore: update to Popper v2 #2380
Open
layershifter
wants to merge
1
commit into
master
Choose a base branch
from
chore/popper-v2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,15 @@ | ||
import { useIsomorphicLayoutEffect } from '@fluentui/react-bindings' | ||
import { Ref, isRefObject } from '@fluentui/react-component-ref' | ||
import * as _ from 'lodash' | ||
import PopperJS, * as _PopperJS from 'popper.js' | ||
import { | ||
createPopper, | ||
Instance, | ||
Placement, | ||
Options, | ||
State, | ||
VirtualElement, | ||
ModifierPhases, | ||
} from '@popperjs/core' | ||
import * as React from 'react' | ||
|
||
import isBrowser from '../isBrowser' | ||
|
@@ -28,34 +36,32 @@ function useDeepMemo<TKey, TValue>(memoFn: () => TValue, key: TKey): TValue { | |
return ref.current.value | ||
} | ||
|
||
// `popper.js` has a UMD build without `.default`, it breaks CJS builds: | ||
// https://github.com/rollup/rollup/issues/1267#issuecomment-446681320 | ||
const createPopper = ( | ||
reference: Element | _PopperJS.ReferenceObject, | ||
popper: HTMLElement, | ||
options?: PopperJS.PopperOptions, | ||
): PopperJS => { | ||
const instance = new ((_PopperJS as any).default || _PopperJS)(reference, popper, { | ||
...options, | ||
eventsEnabled: false, | ||
}) | ||
|
||
const originalUpdate = instance.update | ||
instance.update = () => { | ||
// Fix Popper.js initial positioning display issue | ||
// https://github.com/popperjs/popper.js/issues/457#issuecomment-367692177 | ||
popper.style.left = '0' | ||
popper.style.top = '0' | ||
|
||
originalUpdate() | ||
} | ||
|
||
const actualWindow = popper.ownerDocument.defaultView | ||
instance.scheduleUpdate = () => actualWindow.requestAnimationFrame(instance.update) | ||
instance.enableEventListeners() | ||
|
||
return instance | ||
} | ||
// const createPopper = ( | ||
// reference: Element | VirtualElement, | ||
// popper: HTMLElement, | ||
// options?: any /* TODO */, | ||
// ): Instance => { | ||
// return createPopper(reference, popper, { | ||
// ...options, | ||
// eventsEnabled: false, | ||
// }) | ||
|
||
// const originalUpdate = instance.update | ||
// instance.update = () => { | ||
// // Fix Popper.js initial positioning display issue | ||
// // https://github.com/popperjs/popper.js/issues/457#issuecomment-367692177 | ||
// popper.style.left = '0' | ||
// popper.style.top = '0' | ||
// | ||
// originalUpdate() | ||
// } | ||
// | ||
// const actualWindow = popper.ownerDocument.defaultView | ||
// instance.scheduleUpdate = () => actualWindow.requestAnimationFrame(instance.update) | ||
// instance.enableEventListeners() | ||
|
||
// return instance | ||
// } | ||
|
||
/** | ||
* Popper relies on the 3rd party library [Popper.js](https://github.com/FezVrasta/popper.js) for positioning. | ||
|
@@ -78,13 +84,11 @@ const Popper: React.FunctionComponent<PopperProps> = props => { | |
|
||
const proposedPlacement = getPlacement({ align, position, rtl }) | ||
|
||
const popperRef = React.useRef<PopperJS>() | ||
const popperRef = React.useRef<Instance>() | ||
const contentRef = React.useRef<HTMLElement>(null) | ||
|
||
const latestPlacement = React.useRef<PopperJS.Placement>(proposedPlacement) | ||
const [computedPlacement, setComputedPlacement] = React.useState<PopperJS.Placement>( | ||
proposedPlacement, | ||
) | ||
const latestPlacement = React.useRef<Placement>(proposedPlacement) | ||
const [computedPlacement, setComputedPlacement] = React.useState<Placement>(proposedPlacement) | ||
|
||
const hasDocument = isBrowser() | ||
const hasScrollableElement = React.useMemo(() => { | ||
|
@@ -99,35 +103,38 @@ const Popper: React.FunctionComponent<PopperProps> = props => { | |
// Is a broken dependency and can cause potential bugs, we should rethink this as all other refs | ||
// in this component. | ||
|
||
const computedModifiers: PopperJS.Modifiers = useDeepMemo( | ||
const computedModifiers: Options['modifiers'] = useDeepMemo( | ||
() => | ||
_.merge( | ||
/** | ||
* This prevents blurrines in chrome, when the coordinates are odd numbers alternative | ||
* would be to use `fn` and manipulate the computed style or ask popper to fix it but | ||
* since there is presumably only handful of poppers displayed on the page, the | ||
* performance impact should be minimal. | ||
*/ | ||
{ computeStyle: { gpuAcceleration: false } }, | ||
|
||
{ flip: { padding: 0, flipVariationsByContent: true } }, | ||
{ preventOverflow: { padding: 0 } }, | ||
|
||
offset && { | ||
offset: { offset: rtl ? applyRtlToOffset(offset, position) : offset }, | ||
keepTogether: { enabled: false }, | ||
}, | ||
|
||
[ | ||
/** | ||
* This prevents blurrines in chrome, when the coordinates are odd numbers alternative | ||
* would be to use `fn` and manipulate the computed style or ask popper to fix it but | ||
* since there is presumably only handful of poppers displayed on the page, the | ||
* performance impact should be minimal. | ||
*/ | ||
{ | ||
name: 'computeStyles', | ||
options: { gpuAcceleration: false }, | ||
}, | ||
{ name: 'flip', options: { padding: 0, flipVariations: true } }, | ||
{ name: 'preventOverflow', options: { padding: 0 } }, | ||
Comment on lines
+120
to
+121
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These seem redundant as they are the same as the defaults |
||
|
||
offset && { | ||
name: 'offset', | ||
options: { offset: rtl ? applyRtlToOffset(offset, position) : offset }, | ||
}, | ||
], | ||
/** | ||
* When the popper box is placed in the context of a scrollable element, we need to set | ||
* preventOverflow.escapeWithReference to true and flip.boundariesElement to 'scrollParent' | ||
* (default is 'viewport') so that the popper box will stick with the targetRef when we | ||
* scroll targetRef out of the viewport. | ||
*/ | ||
hasScrollableElement && { | ||
preventOverflow: { escapeWithReference: true }, | ||
flip: { boundariesElement: 'scrollParent' }, | ||
}, | ||
hasScrollableElement && [ | ||
// preventOverflow: { escapeWithReference: true }, TODO | ||
{ name: 'flip', options: { boundary: 'scrollParent' } }, | ||
], | ||
|
||
userModifiers, | ||
|
||
|
@@ -137,25 +144,25 @@ const Popper: React.FunctionComponent<PopperProps> = props => { | |
* the values of `align` and `position` props, regardless of the size of the component, the | ||
* reference element or the viewport. | ||
*/ | ||
unstable_pinned && { flip: { enabled: false } }, | ||
unstable_pinned && [{ name: 'flip', enabled: false }], | ||
), | ||
[hasScrollableElement, position, offset, rtl, unstable_pinned, userModifiers], | ||
) | ||
|
||
const scheduleUpdate = React.useCallback(() => { | ||
if (popperRef.current) { | ||
popperRef.current.scheduleUpdate() | ||
popperRef.current.update() | ||
} | ||
}, []) | ||
|
||
const destroyInstance = React.useCallback(() => { | ||
if (popperRef.current) { | ||
popperRef.current.destroy() | ||
if (popperRef.current.popper) { | ||
// Popper keeps a reference to the DOM node, which needs to be cleaned up | ||
// temporarily fix it here until fixed properly in popper | ||
popperRef.current.popper = null | ||
} | ||
// if (popperRef.current.popper) { | ||
// Popper keeps a reference to the DOM node, which needs to be cleaned up | ||
// temporarily fix it here until fixed properly in popper | ||
// popperRef.current.popper = null | ||
// } | ||
popperRef.current = null | ||
} | ||
}, []) | ||
|
@@ -166,41 +173,50 @@ const Popper: React.FunctionComponent<PopperProps> = props => { | |
const reference = | ||
targetRef && isRefObject(targetRef) | ||
? (targetRef as React.RefObject<Element>).current | ||
: (targetRef as _PopperJS.ReferenceObject) | ||
: (targetRef as VirtualElement) | ||
|
||
if (!enabled || !reference || !contentRef.current) { | ||
return | ||
} | ||
|
||
const hasPointer = !!(pointerTargetRef && pointerTargetRef.current) | ||
const handleUpdate = (data: PopperJS.Data) => { | ||
const handleUpdate = ({ state }: { state: Partial<State> }) => { | ||
console.log(state) | ||
// PopperJS performs computations that might update the computed placement: auto positioning, flipping the | ||
// placement in case the popper box should be rendered at the edge of the viewport and does not fit | ||
if (data.placement !== latestPlacement.current) { | ||
latestPlacement.current = data.placement | ||
setComputedPlacement(data.placement) | ||
if (state.placement !== latestPlacement.current) { | ||
latestPlacement.current = state.placement | ||
setComputedPlacement(state.placement) | ||
} | ||
} | ||
|
||
const options: PopperJS.PopperOptions = { | ||
console.log(computedModifiers) | ||
const options: Options = { | ||
placement: proposedPlacement, | ||
positionFixed, | ||
modifiers: { | ||
strategy: positionFixed ? 'fixed' : 'absolute', | ||
modifiers: [ | ||
...computedModifiers, | ||
|
||
/** | ||
* This modifier is necessary in order to render the pointer. Refs are resolved in effects, so it can't be | ||
* placed under computed modifiers. Deep merge is not required as this modifier has only these properties. | ||
* `arrow` modifier also requires `keepTogether`. | ||
*/ | ||
keepTogether: { enabled: hasPointer }, | ||
arrow: { | ||
{ | ||
name: 'arrow', | ||
enabled: hasPointer, | ||
element: pointerTargetRef && pointerTargetRef.current, | ||
options: { | ||
element: pointerTargetRef && pointerTargetRef.current, | ||
}, | ||
}, | ||
|
||
// afterWrite | ||
{ | ||
name: 'onUpdate', | ||
enabled: true, | ||
phase: 'afterWrite' as ModifierPhases, | ||
fn: handleUpdate, | ||
}, | ||
}, | ||
onCreate: handleUpdate, | ||
onUpdate: handleUpdate, | ||
].filter(Boolean), | ||
onFirstUpdate: state => handleUpdate({ state }), | ||
} | ||
|
||
popperRef.current = createPopper(reference, contentRef.current, options) | ||
|
@@ -213,7 +229,7 @@ const Popper: React.FunctionComponent<PopperProps> = props => { | |
proposedPlacement, | ||
targetRef, | ||
unstable_pinned, | ||
]) | ||
]) // TODO: use options instead of recreate | ||
|
||
useIsomorphicLayoutEffect(() => { | ||
createInstance() | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
packages/react/test/specs/utils/positioner/positioningHelper-test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be required anymore. Popper rounds to the nearest suitable subpixel, and in addition detects the DPI and uses 2D transforms instead of 3D (to preserve subpixel anti-aliasing).