Autoupdate: Pass in empty reference element instead of target #237
Conversation
✅ Deploy Preview for anchor-position-wpt canceled.
|
| const lastSuccessful = target.getAttribute( | ||
| 'data-anchor-polyfill-last-successful', | ||
| ); | ||
| if (lastSuccessful) { | ||
| target.setAttribute('data-anchor-polyfill', lastSuccessful); | ||
| } | ||
| checking = false; |
There was a problem hiding this comment.
In practice, this checking rigamarole seems to be entirely pointless, and never fires an update with checking === true 🤷
There was a problem hiding this comment.
You're right, I also couldn't get it to fire while checking = true. However, I didn't see any clear guarantee that it wouldn't be true, and couldn't find a way to throttle CPU in a polyfillable browser. I'd say it doesn't hurt to keep it?
| @@ -369,7 +370,7 @@ async function applyPositionFallbacks( | |||
| const offsetParent = await getOffsetParent(target); | |||
|
|
|||
| autoUpdate( | |||
| target, | |||
| {} as VirtualElement, | |||
There was a problem hiding this comment.
Can you add a comment about how this change works? Should we be passing in the anchor element here for some reason?
There was a problem hiding this comment.
I don't think we want to react to changes to the anchor element in this case, because we only care about whether the target is overflowing or not. This is a bit of a hacky workaround that depends on the internals of floating-ui, but I added a comment that matches my current understanding.
| const lastSuccessful = target.getAttribute( | ||
| 'data-anchor-polyfill-last-successful', | ||
| ); | ||
| if (lastSuccessful) { | ||
| target.setAttribute('data-anchor-polyfill', lastSuccessful); | ||
| } | ||
| checking = false; |
There was a problem hiding this comment.
You're right, I also couldn't get it to fire while checking = true. However, I didn't see any clear guarantee that it wouldn't be true, and couldn't find a way to throttle CPU in a polyfillable browser. I'd say it doesn't hurt to keep it?
| @@ -369,7 +370,7 @@ async function applyPositionFallbacks( | |||
| const offsetParent = await getOffsetParent(target); | |||
|
|
|||
| autoUpdate( | |||
There was a problem hiding this comment.
Should we find a way to call the cleanup function this returns? Or perhaps that's part of #91 ?
There was a problem hiding this comment.
Yes, and yes I think?
Fixes #236
I think fixes #175 also?