Skip to content

Commit

Permalink
Merge pull request #172 from joinhandshake/skovy/safely-navigate-null…
Browse files Browse the repository at this point in the history
…-scroll-component

Safely navigate a null scroll component (offsetHeight of null)
  • Loading branch information
danbovey committed Sep 14, 2018
2 parents 0b1a5b4 + ac51e07 commit 4d9f594
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 10 deletions.
25 changes: 20 additions & 5 deletions dist/InfiniteScroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ var InfiniteScroll = (function(_Component) {
{
key: 'getParentElement',
value: function getParentElement(el) {
return el.parentNode;
return el && el.parentNode;
},
},
{
Expand Down Expand Up @@ -241,9 +241,7 @@ var InfiniteScroll = (function(_Component) {
if (this.props.isReverse) {
offset = scrollTop;
} else {
offset =
this.calculateTopPosition(el) +
(el.offsetHeight - scrollTop - window.innerHeight);
offset = this.calculateOffset(el, scrollTop);
}
} else if (this.props.isReverse) {
offset = parentNode.scrollTop;
Expand All @@ -253,7 +251,11 @@ var InfiniteScroll = (function(_Component) {
}

// Here we make sure the element is visible as well as checking the offset
if (offset < Number(this.props.threshold) && el.offsetParent !== null) {
if (
offset < Number(this.props.threshold) &&
el &&
el.offsetParent !== null
) {
this.detachScrollListener();
// Call loadMore after detachScrollListener to allow for non-async loadMore functions
if (typeof this.props.loadMore === 'function') {
Expand All @@ -262,6 +264,19 @@ var InfiniteScroll = (function(_Component) {
}
},
},
{
key: 'calculateOffset',
value: function calculateOffset(el, scrollTop) {
if (!el) {
return 0;
}

return (
this.calculateTopPosition(el) +
(el.offsetHeight - scrollTop - window.innerHeight)
);
},
},
{
key: 'calculateTopPosition',
value: function calculateTopPosition(el) {
Expand Down
22 changes: 17 additions & 5 deletions src/InfiniteScroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export default class InfiniteScroll extends Component {
}

getParentElement(el) {
return el.parentNode;
return el && el.parentNode;
}

filterProps(props) {
Expand Down Expand Up @@ -149,9 +149,7 @@ export default class InfiniteScroll extends Component {
if (this.props.isReverse) {
offset = scrollTop;
} else {
offset =
this.calculateTopPosition(el) +
(el.offsetHeight - scrollTop - window.innerHeight);
offset = this.calculateOffset(el, scrollTop);
}
} else if (this.props.isReverse) {
offset = parentNode.scrollTop;
Expand All @@ -160,7 +158,10 @@ export default class InfiniteScroll extends Component {
}

// Here we make sure the element is visible as well as checking the offset
if (offset < Number(this.props.threshold) && el.offsetParent !== null) {
if (
offset < Number(this.props.threshold) &&
(el && el.offsetParent !== null)
) {
this.detachScrollListener();
// Call loadMore after detachScrollListener to allow for non-async loadMore functions
if (typeof this.props.loadMore === 'function') {
Expand All @@ -169,6 +170,17 @@ export default class InfiniteScroll extends Component {
}
}

calculateOffset(el, scrollTop) {
if (!el) {
return 0;
}

return (
this.calculateTopPosition(el) +
(el.offsetHeight - scrollTop - window.innerHeight)
);
}

calculateTopPosition(el) {
if (!el) {
return 0;
Expand Down
24 changes: 24 additions & 0 deletions test/infiniteScroll_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,28 @@ describe('InfiniteScroll component', () => {
InfiniteScroll.prototype.attachScrollListener.restore();
InfiniteScroll.prototype.scrollListener.restore();
});

it('should handle when the scrollElement is removed from the DOM', () => {
const loadMore = stub();

const wrapper = mount(
<div>
<InfiniteScroll pageStart={0} loadMore={loadMore} hasMore={false}>
<div className="child-component">Child Text</div>
</InfiniteScroll>
</div>,
);

const component = wrapper.find(InfiniteScroll);

// The component has now mounted, but the scrollComponent is null
component.instance().scrollComponent = null;

// Invoke the scroll listener which depends on the scrollComponent to
// verify it executes properly, and safely navigates when the
// scrollComponent is null.
component.instance().scrollListener();

expect(wrapper.text()).to.contain('Child Text');
});
});

0 comments on commit 4d9f594

Please sign in to comment.