feat: referenceNode support for reference objects (closes #800)

[Benjamin-Dobell]

Fix for #800

Basically popper.js was assuming popper's were always in global (client) coordinates when a reference object was provided instead of a reference element. We now detect reference object usage and calculate coordinates relative to the popper's parent node.

There is now an additional property, referenceNode, which can be provided on a "reference object". popper.js will now position relative to this node when the property is specified.

[Benjamin-Dobell]

Sigh! IE11. Will boot up a VM and take a look.

[FezVrasta]

It's been a while since last time I checked, but I think the idea was to allow the consumer to define their own object reference parent node to improve flexibility?

[Benjamin-Dobell]

@FezVrasta Yeah, I did see some old comments indicating that would be a desirable approach, however unfortunately it doesn't work that way, at least not at present:

• https://codepen.io/anon/pen/JQwGww

You'd also need to implement nodeType in your reference object, in addition to parentNode. However, that also doesn't work, because all DOM methods throw errors (TypeError: Illegal invocation) when you pass them a non-DOM object:

e.g. compareDocumentPosition ends up being called like realDomElement.compareDocumentPosition(referenceObject) which explodes: https://github.com/FezVrasta/popper.js/blob/master/packages/popper/src/utils/findCommonOffsetParent.js#L21

• https://codepen.io/anon/pen/qzLbvv

...

However, this PR was actually my second pass at resolving this issue. My first attempt is here.

I'll happily submit that as an alternate resolution, particularly if it means I don't have to touch IE11 😉

My original approach is entirely non-breaking as the alternate behavior is only triggered if a newly defined property exists. If you'd like me to submit it as a PR, just let me know.

[FezVrasta]

Thanks, your other attempt seems cleaner to me, I'd like to merge it if you decide to send a PR.

[Benjamin-Dobell]

@FezVrasta Seems as other people have already referenced this PR. and I'm still resolving the same issue, I've just pushed it here.

[Benjamin-Dobell]

Whoops, hold up. Realised there's a couple of issues - not with the functionality, but code structure and comments i.e. getReferenceNode isn't re-exported from utils. Will fix that right now...

[FezVrasta]

Don't worry about the utils re-export, that's a deprecated feature anyways

[Benjamin-Dobell]

@FezVrasta No problem. If you don't have any other feedback, then please go ahead and merge this as is.

I was going to change the docs for getReferenceNode, however in doing so I'll just be opening a whole can of worms regarding consistency e.g. There's several other functions that are documented as taking an Element parameter but are actually Element|Object.

Additionally there's the whole Node vs Element thing, for this particular piece of functionality a Node is actually acceptable, however I don't want to confuse matters.

So if it's okay with you, I'll leave those sorts of decisions to you 😄 Obviously, feel free to make any changes you desire post-merge.

[FezVrasta]

Yeah type annotations aren't the best in v1... v2 is written from the ground up with Flow so this should get fixed if I'll ever manage to complete it.

[atomiks]

Is it possible to release 1.16.0 with this?

[atomiks]

ping @FezVrasta

New release for this feat would be great

[FezVrasta]

https://github.com/FezVrasta/popper.js/releases/tag/v1.16.0