Skip to content

Commit 6d174ca

Browse files
committed
stop eating scroll events that the table didn't use - non-static case
1 parent 07d73df commit 6d174ca

File tree

3 files changed

+88
-8
lines changed

3 files changed

+88
-8
lines changed

src/traces/table/plot.js

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,11 @@ module.exports = function plot(gd, wrappedTraceHolders) {
5959
.on('mousewheel', function(d) {
6060
if(d.scrollbarState.wheeling) return;
6161
d.scrollbarState.wheeling = true;
62-
d3.event.stopPropagation();
63-
d3.event.preventDefault();
64-
makeDragRow(gd, tableControlView, null, d.scrollY + d3.event.deltaY)(d);
62+
var noChange = makeDragRow(gd, tableControlView, null, d.scrollY + d3.event.deltaY)(d);
63+
if(!noChange) {
64+
d3.event.stopPropagation();
65+
d3.event.preventDefault();
66+
}
6567
d.scrollbarState.wheeling = false;
6668
})
6769
.call(renderScrollbarKit, gd, true);
@@ -711,13 +713,20 @@ function updateBlockYPosition(gd, cellsColumnBlock, tableControlView) {
711713

712714
function makeDragRow(gd, allTableControlView, optionalMultiplier, optionalPosition) {
713715
return function dragRow(eventD) {
714-
// may come from whicever DOM event target: drag, wheel, bar... eventD corresponds to event target
716+
// may come from whichever DOM event target: drag, wheel, bar... eventD corresponds to event target
715717
var d = eventD.calcdata ? eventD.calcdata : eventD;
716718
var tableControlView = allTableControlView.filter(function(dd) {return d.key === dd.key;});
717719
var multiplier = optionalMultiplier || d.scrollbarState.dragMultiplier;
720+
721+
var initialScrollY = d.scrollY;
722+
718723
d.scrollY = optionalPosition === void(0) ? d.scrollY + multiplier * d3.event.dy : optionalPosition;
719724
var cellsColumnBlock = tableControlView.selectAll('.' + c.cn.yColumn).selectAll('.' + c.cn.columnBlock).filter(cellsBlock);
720725
updateBlockYPosition(gd, cellsColumnBlock, tableControlView);
726+
727+
// return false if we've "used" the scroll, ie it did something,
728+
// so the event shouldn't bubble (if appropriate)
729+
return d.scrollY === initialScrollY;
721730
};
722731
}
723732

test/jasmine/assets/mouse_event.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,11 @@ module.exports = function(type, x, y, opts) {
3636
var el = (opts && opts.element) || document.elementFromPoint(x, y),
3737
ev;
3838

39-
if(type === 'scroll') {
40-
ev = new window.WheelEvent('wheel', Lib.extendFlat({}, fullOpts, opts));
39+
if(type === 'scroll' || type === 'mousewheel') {
40+
// somehow table needs this to be mouswheel but others need wheel.
41+
// yet they all work the same in the browser?
42+
type = (type === 'scroll') ? 'wheel' : 'mousewheel';
43+
ev = new window.WheelEvent(type, Lib.extendFlat({}, fullOpts, opts));
4144
} else {
4245
ev = new window.MouseEvent(type, fullOpts);
4346
}

test/jasmine/tests/table_test.js

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ var createGraphDiv = require('../assets/create_graph_div');
99
var destroyGraphDiv = require('../assets/destroy_graph_div');
1010
var failTest = require('../assets/fail_test');
1111
var supplyAllDefaults = require('../assets/supply_defaults');
12+
var mouseEvent = require('../assets/mouse_event');
1213

1314
var mockMulti = require('@mocks/table_latex_multitrace_scatter.json');
1415

@@ -385,8 +386,7 @@ describe('table', function() {
385386
});
386387

387388
describe('more restyling tests with scenegraph queries', function() {
388-
var mockCopy,
389-
gd;
389+
var mockCopy, gd;
390390

391391
beforeEach(function(done) {
392392
mockCopy = Lib.extendDeep({}, mock2);
@@ -425,4 +425,72 @@ describe('table', function() {
425425
.then(done);
426426
});
427427
});
428+
429+
describe('scroll effects', function() {
430+
var mockCopy, gd, gdWheelEventCount;
431+
432+
beforeEach(function(done) {
433+
mockCopy = Lib.extendDeep({}, mockMulti);
434+
gd = createGraphDiv();
435+
gdWheelEventCount = 0;
436+
Plotly.plot(gd, mockCopy.data, mockCopy.layout)
437+
.then(function() {
438+
gd.addEventListener('mousewheel', function(evt) {
439+
gdWheelEventCount++;
440+
// make sure we don't *really* scroll the page.
441+
// that would be annoying!
442+
evt.stopPropagation();
443+
evt.preventDefault();
444+
});
445+
})
446+
.then(done);
447+
});
448+
449+
function assertBubbledEvents(cnt) {
450+
expect(gdWheelEventCount).toBe(cnt);
451+
gdWheelEventCount = 0;
452+
}
453+
454+
function getCenter(el) {
455+
var bBox = el.getBoundingClientRect();
456+
// actually above center, since these bboxes are bigger than the
457+
// visible table area
458+
return {x: bBox.left + bBox.width / 2, y: bBox.top + bBox.height / 4};
459+
}
460+
461+
function scroll(pos, dy) {
462+
mouseEvent('mousemove', pos.x, pos.y);
463+
mouseEvent('mousewheel', pos.x, pos.y, {deltaX: 0, deltaY: dy});
464+
}
465+
466+
it('bubbles scroll events iff they did not scroll a table', function() {
467+
var allTableControls = document.querySelectorAll('.' + cn.tableControlView);
468+
var smallCenter = getCenter(allTableControls[0]);
469+
var bigCenter = getCenter(allTableControls[1]);
470+
471+
// table with no scroll bars - events always bubble
472+
scroll(smallCenter, -20);
473+
assertBubbledEvents(1);
474+
scroll(smallCenter, 20);
475+
assertBubbledEvents(1);
476+
477+
// table with scrollbars - events bubble if we don't use them
478+
scroll(bigCenter, -20); // up from the top - bubbles
479+
assertBubbledEvents(1);
480+
scroll(bigCenter, 20); // down from the top - scrolled, so no bubble
481+
assertBubbledEvents(0);
482+
scroll(bigCenter, -40); // back to the top, with extra dy discarded
483+
assertBubbledEvents(0);
484+
scroll(bigCenter, -20); // now it bubbles
485+
assertBubbledEvents(1);
486+
scroll(bigCenter, 200000); // all the way to the bottom
487+
assertBubbledEvents(0);
488+
scroll(bigCenter, 20); // now it bubbles from the bottom..
489+
assertBubbledEvents(1);
490+
});
491+
492+
it('does not scroll any tables with staticPlot', function(done) {
493+
// TODO
494+
});
495+
});
428496
});

0 commit comments

Comments
 (0)