Skip to content

Commit

Permalink
Fix window scrolling
Browse files Browse the repository at this point in the history
We introduced a bug in d5eb92b which caused Waypoints that listen
to scroll events on the `window` object to fail. This was because we
switched from parentNode to parentElement when traversing up the DOM
tree. This in turn meant that on the way to `window`, we would now pass
the `document` node. This node has no styles, and if you try calling
`window.getComputedStyles` on it, it will return `null`.

I've added a guard for this scenario that will bring back the desired
behavior.

While adding a spec, I noticed that a previous, seemingly unrelated,
spec was suddenly failing. It was meant to test the fact that we
explicitly throw an error if the offsetParent for the Waypoint node does
not have positioning. This is done to help prevent unexpected behavior
when using the Waypoint component. The spec for this however, wasn't
catching the right error thrown. Instead it falsely caught the error for
when the getComputedStyle was null. I looked for a way in Jasmine to
make this spec better specify what error it wanted to catch, but I
couldn't find anything obvious. Ideas for how to improve that spec are
welcome.
  • Loading branch information
Henric Trotzig committed Jan 30, 2015
1 parent f1f3656 commit a3dc459
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
28 changes: 27 additions & 1 deletion spec/waypoint_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,33 @@ describe('Waypoint', function() {
});

it('throws an error', function() {
expect(this.subject).toThrow();
expect(function() { this.subject(); }.bind(this)).toThrow();
});
});

describe('when the window is the scrollable parent', function() {
beforeEach(function() {
// Make the normal parent non-scrollable
this.parentStyle = {};

// Make the spacers large enough to make the Waypoint render off-screen
this.topSpacerHeight = 4000;
this.bottomSpacerHeight = 4000;
});

it('does not fire the onEnter handler on mount', function() {
expect(this.props.onEnter).not.toHaveBeenCalled();
});

describe('when the Waypoint is in view', function() {
beforeEach(function() {
this.subject();
scrollNodeTo(window, 3900);
});

it('fires the onEnter handler', function() {
expect(this.props.onEnter).toHaveBeenCalled();
});
});
});
});
5 changes: 5 additions & 0 deletions src/waypoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ var Waypoint = React.createClass({
node = node.parentNode;

var style = window.getComputedStyle(node);
if (!style) {
// Some nodes will return null for `getComputedStyle`. An example of
// that is the `document` node.
continue;
}
var overflowY = style.getPropertyValue('overflow-y') ||
style.getPropertyValue('overflow');

Expand Down

0 comments on commit a3dc459

Please sign in to comment.