Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: Manually dispatch scroll event #940

Merged
merged 3 commits into from
Feb 25, 2019
Merged

Fix: Manually dispatch scroll event #940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa5a08e
Fix: Manually dispatch scroll event
Feb 22, 2019
101771c
Merge branch 'master' into scroll-event
Feb 22, 2019
e88f75d
Merge branch 'master' into scroll-event
Feb 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/lib/VirtualScroller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,10 @@ class VirtualScroller {
// will get rendered.
const topPosition = (this.itemHeight + this.margin) * rowIndex;
this.scrollingEl.scrollTop = topPosition;
// Some browsers don't fire the scroll event when setting scrollTop
// (IE11 & Firefox) so we need to manually dispatch the event
// in order to trigger `onScrollHandler` to render the items
this.scrollingEl.dispatchEvent(new Event('scroll'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any issues with the event firing twice on browsers that automatically trigger it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see it firing twice, but I'll double check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right it fires twice, but the scrollTop is the same so the logic to render items won't get triggered the second invocation

}
}

Expand Down
14 changes: 10 additions & 4 deletions src/lib/__tests__/VirtualScroller-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ describe('VirtualScroller', () => {
describe('init()', () => {
beforeEach(() => {
stubs.validateRequiredConfig = sandbox.stub(virtualScroller, 'validateRequiredConfig');
stubs.renderItems = sandbox.stub(virtualScroller, 'renderItems');
});

it('should parse the config object', () => {
stubs.renderItemFn = sandbox.stub();
stubs.renderItems = sandbox.stub(virtualScroller, 'renderItems');
stubs.bindDOMListeners = sandbox.stub(virtualScroller, 'bindDOMListeners');

virtualScroller.init({
Expand Down Expand Up @@ -100,7 +100,6 @@ describe('VirtualScroller', () => {

it('should call renderItems with the provided initialRowIndex', () => {
stubs.renderItemFn = sandbox.stub();
stubs.renderItems = sandbox.stub(virtualScroller, 'renderItems');

virtualScroller.init({
totalItems: 10,
Expand All @@ -115,7 +114,6 @@ describe('VirtualScroller', () => {

it('should call renderItems with 0 if initialRowIndex falls within first window', () => {
stubs.renderItemFn = sandbox.stub();
stubs.renderItems = sandbox.stub(virtualScroller, 'renderItems');

virtualScroller.init({
totalItems: 10,
Expand Down Expand Up @@ -462,7 +460,8 @@ describe('VirtualScroller', () => {
let listEl;

beforeEach(() => {
scrollingEl = { remove: () => {} };
stubs.dispatchEvent = sandbox.stub();
scrollingEl = { remove: () => {}, dispatchEvent: stubs.dispatchEvent };

virtualScroller.totalItems = 10;
virtualScroller.itemHeight = 10;
Expand All @@ -488,27 +487,31 @@ describe('VirtualScroller', () => {

expect(stubs.isVisible).not.to.be.called;
expect(scrollingEl.scrollTop).to.be.undefined;
expect(stubs.dispatchEvent).not.to.be.called;
});

it('should do nothing if rowIndex is < 0', () => {
virtualScroller.scrollIntoView(-1);

expect(stubs.isVisible).not.to.be.called;
expect(scrollingEl.scrollTop).to.be.undefined;
expect(stubs.dispatchEvent).not.to.be.called;
});

it('should do nothing if rowIndex is = totalItems', () => {
virtualScroller.scrollIntoView(10);

expect(stubs.isVisible).not.to.be.called;
expect(scrollingEl.scrollTop).to.be.undefined;
expect(stubs.dispatchEvent).not.to.be.called;
});

it('should do nothing if rowIndex is > totalItems', () => {
virtualScroller.scrollIntoView(11);

expect(stubs.isVisible).not.to.be.called;
expect(scrollingEl.scrollTop).to.be.undefined;
expect(stubs.dispatchEvent).not.to.be.called;
});

it('should set the scroll top if item is not found', () => {
Expand All @@ -519,6 +522,7 @@ describe('VirtualScroller', () => {
expect(stubs.isVisible).not.to.be.called;
expect(stubs.scrollIntoView).not.to.be.called;
expect(scrollingEl.scrollTop).not.to.be.undefined;
expect(stubs.dispatchEvent).to.be.called;
});

it('should scroll item into view if found but not visible', () => {
Expand All @@ -530,6 +534,7 @@ describe('VirtualScroller', () => {
expect(stubs.isVisible).to.be.called;
expect(stubs.scrollIntoView).to.be.called;
expect(scrollingEl.scrollTop).to.be.undefined;
expect(stubs.dispatchEvent).not.to.be.called;
});

it('should not scroll if item is found and visible', () => {
Expand All @@ -541,6 +546,7 @@ describe('VirtualScroller', () => {
expect(stubs.isVisible).to.be.called;
expect(stubs.scrollIntoView).not.to.be.called;
expect(scrollingEl.scrollTop).to.be.undefined;
expect(stubs.dispatchEvent).not.to.be.called;
});
});

Expand Down