Add matchAnchorWidth to popover #39
Conversation
packages/boundless-popover/index.js
Outdated
| matchAnchorWidth = () => { | ||
| if (!this.props.matchAnchorWidth) return; | ||
|
|
||
| const anchor = this.props.anchor instanceof HTMLElement |
There was a problem hiding this comment.
Just learned this recently, but findDOMNode is actually an identity function if called on a DOM node, so you can remove this ternary and always call it.
packages/boundless-popover/index.js
Outdated
| ? this.props.anchor | ||
| : findDOMNode(this.props.anchor); | ||
| const dialog = this.dialog.$dialog; | ||
| const anchorWidth = anchor.getBoundingClientRect().width; |
There was a problem hiding this comment.
We compute this in cacheViewportCartography() as called by this.align(), so if we move the invocation of this function to after the caching is complete, we can avoid doing the measurement work twice 😄
packages/boundless-popover/index.js
Outdated
| this.align(); | ||
| window.addEventListener('resize', this.matchAnchorWidth, true); |
There was a problem hiding this comment.
I think we can remove this additional event listener as well by integrating the width matching function into the alignment flow.
|
@probablyup Thanks for the comments. I will make the updates and work on adding tests this weekend. |
newyork-anthonyng
force-pushed
the
popover-match-anchor-width
branch
from
ae4f502 to
6307048
Compare
newyork-anthonyng
force-pushed
the
popover-match-anchor-width
branch
from
6307048 to
03cc5e8
Compare
|
I made the changes and added a test for this. Let me know if there's anything else needed :) |
|
Sorry for the very long delay, this looks excellent. |
This commit is a WIP.
I will add tests to this if this is the way to go.