Skip to content

Commit

Permalink
Fix fire onEnter callback anyway when parent isn't display
Browse files Browse the repository at this point in the history
Fix #328
  • Loading branch information
xyy94813 committed May 17, 2021
1 parent 2239914 commit d75ef7d
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 15 deletions.
48 changes: 33 additions & 15 deletions src/waypoint.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,7 @@ export class Waypoint extends React.PureComponent {
* as a fallback.
*/
_findScrollableAncestor() {
const {
horizontal,
scrollableAncestor,
} = this.props;
const { horizontal, scrollableAncestor } = this.props;

if (scrollableAncestor) {
return resolveScrollableAncestorProp(scrollableAncestor);
Expand All @@ -159,7 +156,11 @@ export class Waypoint extends React.PureComponent {
: style.getPropertyValue('overflow-y');
const overflow = overflowDirec || style.getPropertyValue('overflow');

if (overflow === 'auto' || overflow === 'scroll' || overflow === 'overlay') {
if (
overflow === 'auto'
|| overflow === 'scroll'
|| overflow === 'overlay'
) {
return node;
}
}
Expand All @@ -169,6 +170,25 @@ export class Waypoint extends React.PureComponent {
return window;
}

/**
* Traverses up the DOM to check element is display.
*
* @return {Boolean}
*/
_checkDisplay() {
let node = this._ref;

while (node && node instanceof Element) {
const style = window.getComputedStyle(node);
if (style.getPropertyValue('display') === 'none') {
return false;
}
node = node.parentNode;
}

return true;
}

/**
* @param {Object} event the native scroll event coming from the scrollable
* ancestor, or resize event coming from the window. Will be undefined if
Expand All @@ -181,7 +201,9 @@ export class Waypoint extends React.PureComponent {
}

const bounds = this._getBounds();
const currentPosition = getCurrentPosition(bounds);
// If waypoint or its parent is not display
const currentPosition = this._checkDisplay() ? getCurrentPosition(bounds) : INVISIBLE;

const previousPosition = this._previousPosition;
const {
debug,
Expand Down Expand Up @@ -221,10 +243,8 @@ export class Waypoint extends React.PureComponent {
onLeave.call(this, callbackArg);
}

const isRapidScrollDown = previousPosition === BELOW
&& currentPosition === ABOVE;
const isRapidScrollUp = previousPosition === ABOVE
&& currentPosition === BELOW;
const isRapidScrollDown = previousPosition === BELOW && currentPosition === ABOVE;
const isRapidScrollUp = previousPosition === ABOVE && currentPosition === BELOW;

if (fireOnRapidScroll && (isRapidScrollDown || isRapidScrollUp)) {
// If the scroll event isn't fired often enough to occur while the
Expand Down Expand Up @@ -264,7 +284,8 @@ export class Waypoint extends React.PureComponent {
contextHeight = horizontal ? window.innerWidth : window.innerHeight;
contextScrollTop = 0;
} else {
contextHeight = horizontal ? this.scrollableAncestor.offsetWidth
contextHeight = horizontal
? this.scrollableAncestor.offsetWidth
: this.scrollableAncestor.offsetHeight;
contextScrollTop = horizontal
? this.scrollableAncestor.getBoundingClientRect().left
Expand Down Expand Up @@ -341,10 +362,7 @@ if (process.env.NODE_ENV !== 'production') {
// For instance, if you pass "-20%", and the containing element is 100px tall,
// then the waypoint will be triggered when it has been scrolled 20px beyond
// the top of the containing element.
topOffset: PropTypes.oneOfType([
PropTypes.string,
PropTypes.number,
]),
topOffset: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),

// `bottomOffset` can either be a number, in which case its a distance from the
// bottom of the container in pixels, or a string value. Valid string values are
Expand Down
35 changes: 35 additions & 0 deletions test/browser/waypoint_test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -1390,6 +1390,41 @@ describe('<Waypoint>', () => {
});
});
});

describe('When the parent element is not displayed', () => {
beforeEach(() => {
subject = () => {
parentStyle = {
height: 100,
display: 'none',
};
const el = renderAttached(
<div style={parentStyle}>
<div style={{ display: 'inline-block' }} />
<Waypoint {...props} />
<div style={{ display: 'inline-block' }} />
</div>,
);

jasmine.clock().tick(1);
return el;
};
});

afterEach(() => { });

it('does not call the onEnter handler', () => {
expect(props.onEnter).not.toHaveBeenCalled();
});

it('does not call the onLeave handler', () => {
expect(props.onLeave).not.toHaveBeenCalled();
});

it('does not call the onPositionChange handler', () => {
expect(props.onPositionChange).not.toHaveBeenCalled();
});
});
});

// smoke tests for horizontal scrolling
Expand Down

0 comments on commit d75ef7d

Please sign in to comment.