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

Bug fix - do not drop previous constraint on click #4089

Merged
merged 3 commits into from
Jul 31, 2019
Merged

Bug fix - do not drop previous constraint on click #4089

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
80383f9
fixes phx 235 - dont drop selection on click - make sure drag is call…
archmoj Jul 30, 2019
fe7e02c
add parcoords jasmine tests to lock click issue reported in phx 235
archmoj Jul 30, 2019
30876b2
stash dragging on object instead of parcoords trace module
archmoj Jul 31, 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
257 changes: 138 additions & 119 deletions src/traces/parcoords/axisbrush.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,137 +206,156 @@ function getInterval(d, y) {
return out;
}

function dragstart(lThis, d) {
d3.event.sourceEvent.stopPropagation();
var y = d.height - d3.mouse(lThis)[1] - 2 * c.verticalPadding;
var unitLocation = d.unitToPaddedPx.invert(y);
var b = d.brush;
var interval = getInterval(d, y);
var unitRange = interval.interval;
var s = b.svgBrush;
s.wasDragged = false; // we start assuming there won't be a drag - useful for reset
s.grabbingBar = interval.region === 'ns';
if(s.grabbingBar) {
var pixelRange = unitRange.map(d.unitToPaddedPx);
s.grabPoint = y - pixelRange[0] - c.verticalPadding;
s.barLength = pixelRange[1] - pixelRange[0];
}
s.clickableOrdinalRange = interval.clickableOrdinalRange;
s.stayingIntervals = (d.multiselect && b.filterSpecified) ? b.filter.getConsolidated() : [];
if(unitRange) {
s.stayingIntervals = s.stayingIntervals.filter(function(int2) {
return int2[0] !== unitRange[0] && int2[1] !== unitRange[1];
});
}
s.startExtent = interval.region ? unitRange[interval.region === 's' ? 1 : 0] : unitLocation;
d.parent.inBrushDrag = true;
s.brushStartCallback();
}

function drag(lThis, d) {
d3.event.sourceEvent.stopPropagation();
var y = d.height - d3.mouse(lThis)[1] - 2 * c.verticalPadding;
var s = d.brush.svgBrush;
s.wasDragged = true;
s._dragging = true;

if(s.grabbingBar) { // moving the bar
s.newExtent = [y - s.grabPoint, y + s.barLength - s.grabPoint].map(d.unitToPaddedPx.invert);
} else { // south/north drag or new bar creation
s.newExtent = [s.startExtent, d.unitToPaddedPx.invert(y)].sort(sortAsc);
}

d.brush.filterSpecified = true;
s.extent = s.stayingIntervals.concat([s.newExtent]);
s.brushCallback(d);
renderHighlight(lThis.parentNode);
}

function dragend(lThis, d) {
var brush = d.brush;
var filter = brush.filter;
var s = brush.svgBrush;

if(!s._dragging) { // i.e. click
// mock zero drag
mousemove(lThis, d);
drag(lThis, d);
// remember it is a click not a drag
d.brush.svgBrush.wasDragged = false;
}
s._dragging = false;

var e = d3.event;
e.sourceEvent.stopPropagation();
var grabbingBar = s.grabbingBar;
s.grabbingBar = false;
s.grabLocation = undefined;
d.parent.inBrushDrag = false;
clearCursor(); // instead of clearing, a nicer thing would be to set it according to current location
if(!s.wasDragged) { // a click+release on the same spot (ie. w/o dragging) means a bar or full reset
s.wasDragged = undefined; // logic-wise unneeded, just shows `wasDragged` has no longer a meaning
if(s.clickableOrdinalRange) {
if(brush.filterSpecified && d.multiselect) {
s.extent.push(s.clickableOrdinalRange);
} else {
s.extent = [s.clickableOrdinalRange];
brush.filterSpecified = true;
}
} else if(grabbingBar) {
s.extent = s.stayingIntervals;
if(s.extent.length === 0) {
brushClear(brush);
}
} else {
brushClear(brush);
}
s.brushCallback(d);
renderHighlight(lThis.parentNode);
s.brushEndCallback(brush.filterSpecified ? filter.getConsolidated() : []);
return; // no need to fuse intervals or snap to ordinals, so we can bail early
}

var mergeIntervals = function() {
// Key piece of logic: once the button is released, possibly overlapping intervals will be fused:
// Here it's done immediately on click release while on ordinal snap transition it's done at the end
filter.set(filter.getConsolidated());
};

if(d.ordinal) {
var a = d.unitTickvals;
if(a[a.length - 1] < a[0]) a.reverse();
s.newExtent = [
ordinalScaleSnap(0, a, s.newExtent[0], s.stayingIntervals),
ordinalScaleSnap(1, a, s.newExtent[1], s.stayingIntervals)
];
var hasNewExtent = s.newExtent[1] > s.newExtent[0];
s.extent = s.stayingIntervals.concat(hasNewExtent ? [s.newExtent] : []);
if(!s.extent.length) {
brushClear(brush);
}
s.brushCallback(d);
if(hasNewExtent) {
// merging intervals post the snap tween
renderHighlight(lThis.parentNode, mergeIntervals);
} else {
// if no new interval, don't animate, just redraw the highlight immediately
mergeIntervals();
renderHighlight(lThis.parentNode);
}
} else {
mergeIntervals(); // merging intervals immediately
}
s.brushEndCallback(brush.filterSpecified ? filter.getConsolidated() : []);
}

function mousemove(lThis, d) {
var y = d.height - d3.mouse(lThis)[1] - 2 * c.verticalPadding;
var interval = getInterval(d, y);

var cursor = 'crosshair';
if(interval.clickableOrdinalRange) cursor = 'pointer';
else if(interval.region) cursor = interval.region + '-resize';
d3.select(document.body)
.style('cursor', cursor);
}

function attachDragBehavior(selection) {
// There's some fiddling with pointer cursor styling so that the cursor preserves its shape while dragging a brush
// even if the cursor strays from the interacting bar, which is bound to happen as bars are thin and the user
// will inevitably leave the hotspot strip. In this regard, it does something similar to what the D3 brush would do.
selection
.on('mousemove', function(d) {
d3.event.preventDefault();
if(!d.parent.inBrushDrag) {
var y = d.height - d3.mouse(this)[1] - 2 * c.verticalPadding;
var interval = getInterval(d, y);

var cursor = 'crosshair';
if(interval.clickableOrdinalRange) cursor = 'pointer';
else if(interval.region) cursor = interval.region + '-resize';
d3.select(document.body)
.style('cursor', cursor);
}
if(!d.parent.inBrushDrag) mousemove(this, d);
})
.on('mouseleave', function(d) {
if(!d.parent.inBrushDrag) clearCursor();
})
.call(d3.behavior.drag()
.on('dragstart', function(d) {
d3.event.sourceEvent.stopPropagation();
var y = d.height - d3.mouse(this)[1] - 2 * c.verticalPadding;
var unitLocation = d.unitToPaddedPx.invert(y);
var b = d.brush;
var interval = getInterval(d, y);
var unitRange = interval.interval;
var s = b.svgBrush;
s.wasDragged = false; // we start assuming there won't be a drag - useful for reset
s.grabbingBar = interval.region === 'ns';
if(s.grabbingBar) {
var pixelRange = unitRange.map(d.unitToPaddedPx);
s.grabPoint = y - pixelRange[0] - c.verticalPadding;
s.barLength = pixelRange[1] - pixelRange[0];
}
s.clickableOrdinalRange = interval.clickableOrdinalRange;
s.stayingIntervals = (d.multiselect && b.filterSpecified) ? b.filter.getConsolidated() : [];
if(unitRange) {
s.stayingIntervals = s.stayingIntervals.filter(function(int2) {
return int2[0] !== unitRange[0] && int2[1] !== unitRange[1];
});
}
s.startExtent = interval.region ? unitRange[interval.region === 's' ? 1 : 0] : unitLocation;
d.parent.inBrushDrag = true;
s.brushStartCallback();
})
.on('drag', function(d) {
d3.event.sourceEvent.stopPropagation();
var y = d.height - d3.mouse(this)[1] - 2 * c.verticalPadding;
var s = d.brush.svgBrush;
s.wasDragged = true;

if(s.grabbingBar) { // moving the bar
s.newExtent = [y - s.grabPoint, y + s.barLength - s.grabPoint].map(d.unitToPaddedPx.invert);
} else { // south/north drag or new bar creation
s.newExtent = [s.startExtent, d.unitToPaddedPx.invert(y)].sort(sortAsc);
}

d.brush.filterSpecified = true;
s.extent = s.stayingIntervals.concat([s.newExtent]);
s.brushCallback(d);
renderHighlight(this.parentNode);
})
.on('dragend', function(d) {
var e = d3.event;
e.sourceEvent.stopPropagation();
var brush = d.brush;
var filter = brush.filter;
var s = brush.svgBrush;
var grabbingBar = s.grabbingBar;
s.grabbingBar = false;
s.grabLocation = undefined;
d.parent.inBrushDrag = false;
clearCursor(); // instead of clearing, a nicer thing would be to set it according to current location
if(!s.wasDragged) { // a click+release on the same spot (ie. w/o dragging) means a bar or full reset
s.wasDragged = undefined; // logic-wise unneeded, just shows `wasDragged` has no longer a meaning
if(s.clickableOrdinalRange) {
if(brush.filterSpecified && d.multiselect) {
s.extent.push(s.clickableOrdinalRange);
} else {
s.extent = [s.clickableOrdinalRange];
brush.filterSpecified = true;
}
} else if(grabbingBar) {
s.extent = s.stayingIntervals;
if(s.extent.length === 0) {
brushClear(brush);
}
} else {
brushClear(brush);
}
s.brushCallback(d);
renderHighlight(this.parentNode);
s.brushEndCallback(brush.filterSpecified ? filter.getConsolidated() : []);
return; // no need to fuse intervals or snap to ordinals, so we can bail early
}

var mergeIntervals = function() {
// Key piece of logic: once the button is released, possibly overlapping intervals will be fused:
// Here it's done immediately on click release while on ordinal snap transition it's done at the end
filter.set(filter.getConsolidated());
};

if(d.ordinal) {
var a = d.unitTickvals;
if(a[a.length - 1] < a[0]) a.reverse();
s.newExtent = [
ordinalScaleSnap(0, a, s.newExtent[0], s.stayingIntervals),
ordinalScaleSnap(1, a, s.newExtent[1], s.stayingIntervals)
];
var hasNewExtent = s.newExtent[1] > s.newExtent[0];
s.extent = s.stayingIntervals.concat(hasNewExtent ? [s.newExtent] : []);
if(!s.extent.length) {
brushClear(brush);
}
s.brushCallback(d);
if(hasNewExtent) {
// merging intervals post the snap tween
renderHighlight(this.parentNode, mergeIntervals);
} else {
// if no new interval, don't animate, just redraw the highlight immediately
mergeIntervals();
renderHighlight(this.parentNode);
}
} else {
mergeIntervals(); // merging intervals immediately
}
s.brushEndCallback(brush.filterSpecified ? filter.getConsolidated() : []);
})
.on('dragstart', function(d) { dragstart(this, d); })
.on('drag', function(d) { drag(this, d); })
.on('dragend', function(d) { dragend(this, d); })
);
}

Expand Down
72 changes: 72 additions & 0 deletions test/jasmine/tests/parcoords_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ function mouseTo(x, y) {
mouseEvent('mouseover', x, y);
}

function mouseClick(x, y) {
mouseTo(x, y);
mouseEvent('mousedown', x, y);
mouseEvent('mouseup', x, y);
}

function mostOfDrag(x1, y1, x2, y2) {
mouseTo(x1, y1);
mouseEvent('mousedown', x1, y1);
Expand Down Expand Up @@ -1625,3 +1631,69 @@ describe('parcoords constraint interactions - with defined axis ranges', functio
.then(done);
});
});

describe('parcoords constraint click interactions - with pre-defined constraint ranges', function() {
function initialFigure() {
return {
data: [{
type: 'parcoords',
dimensions: [{
values: [0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1],
}, {
values: [0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6],
tickvals: [0, 1, 2, 3, 4, 5, 6],
ticktext: ['a', 'b', 'c', 'd', 'e', 'f', 'g'],
constraintrange: [1, 2]
}]
}],
layout: {
width: 400,
height: 400,
margin: {t: 100, b: 100, l: 100, r: 100}
}
};
}

var gd;
var initialSnapDuration;
var shortenedSnapDuration = 20;
var snapDelay = 100;
beforeAll(function() {
initialSnapDuration = PC.bar.snapDuration;
PC.bar.snapDuration = shortenedSnapDuration;
});

afterAll(function() {
purgeGraphDiv();
PC.bar.snapDuration = initialSnapDuration;
});

beforeEach(function(done) {
var hasGD = !!gd;
if(!hasGD) gd = createGraphDiv();

Plotly.react(gd, initialFigure())
.catch(failTest)
.then(done);
});

it('@noCI @gl should not drop constraintrange on click', function(done) {
expect(gd._fullData[0].dimensions[1].constraintrange).toBeCloseToArray([0.75, 2.25]);

// click to add a new item to the selection
mouseClick(295, 200);
delay(snapDelay)()
.then(function() {
expect(gd._fullData[0].dimensions[1].constraintrange).toBeCloseToArray([[0.75, 2.25], [2.75, 3.25]]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice test.

Before this patch gd._fullData[0].dimensions[1].constraintrange evaluated to [2.75, 3.25]


// click to deselect all
mouseClick(295, 205);
})
.then(delay(snapDelay)())
.then(function() {
expect(gd._fullData[0].dimensions[1].constraintrange).toEqual(undefined);
})
.catch(failTest)
.then(done);
});
});