Components: Track scroll position of all parent scrollable nodes #3007
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3007 +/- ##
==========================================
- Coverage 34.38% 34.37% -0.02%
==========================================
Files 196 196
Lines 5810 5809 -1
Branches 1027 1027
==========================================
- Hits 1998 1997 -1
Misses 3222 3222
Partials 590 590
Continue to review full report at Codecov.
|
components/popover/index.js
Outdated
| return []; | ||
| } | ||
| const parentScrollables = getScrollParents( node.parentNode ); | ||
| if ( node.scrollHeight > node.clientHeight ) { |
There was a problem hiding this comment.
Thinking this doesn't solve the issue if the parent has no scrollbars yet unless we add content later.
efba698 to
47e1fd0
Compare
|
Ok simplified this, it looks like using the "capturing" flag catches all scroll events over the page. and it fixes the issue. |
|
If I know that was the issue... https://github.com/WordPress/gutenberg/pull/2858/files#diff-a7dcd2d17f454484a328ec65ca81486aR142 :) |
components/popover/index.js
Outdated
| return; | ||
| } | ||
| const { parentNode } = anchor; |
There was a problem hiding this comment.
Both anchor.parentNode and { parentNode } = anchor in one place feels a bit weird to me.
There was a problem hiding this comment.
yep we could avoid destructuring
47e1fd0 to
1ded9dc
Compare
Try to fix #2999
Not sure if there are performance concerns about this, or if it's a good solution but this tries to fix popovers position if we scroll one of their parents.
Testing instructions