Skip to content

Fix zoom overlay positioning (Fixes #359) #395

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

Closed
wants to merge 7 commits into from
Closed

Fix zoom overlay positioning (Fixes #359) #395

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
df69584
Split method shapes.draw into smaller functions
n-riesco Apr 1, 2016
e342a9e
Remove unnecessary step from shapes.drawAll()
n-riesco Apr 4, 2016
bfc8e0a
Move select and zoombox elements into shapelayer
n-riesco Apr 5, 2016
1d23673
Shapes: Remove unnecessary call to String()
n-riesco Apr 5, 2016
e3bfea7
graph_interact: Definitions before declarations
n-riesco Apr 5, 2016
9b8d3c5
Add more tests to select_test.js
n-riesco Apr 5, 2016
4fbf087
Add zoom tests
n-riesco Apr 5, 2016
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
146 changes: 89 additions & 57 deletions src/components/shapes/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ function linearToData(ax) { return ax.type === 'category' ? ax.l2c : ax.l2d; }

shapes.drawAll = function(gd) {
var fullLayout = gd._fullLayout;
fullLayout._shapelayer.selectAll('path').remove();
for(var i = 0; i < fullLayout.shapes.length; i++) {
shapes.draw(gd, i);
}
Expand All @@ -115,86 +114,114 @@ shapes.add = function(gd) {
// if opt is blank, val can be 'add' or a full options object to add a new
// annotation at that point in the array, or 'remove' to delete this one
shapes.draw = function(gd, index, opt, value) {
var layout = gd.layout,
fullLayout = gd._fullLayout,
i;

// TODO: abstract out these drawAll, add, and remove blocks for shapes and annotations
if(!isNumeric(index) || index===-1) {
// no index provided - we're operating on ALL shapes
if(!index && Array.isArray(value)) {
// a whole annotation array is passed in
// (as in, redo of delete all)
layout.shapes = value;
shapes.supplyLayoutDefaults(layout, fullLayout);
shapes.drawAll(gd);
replaceAllShapes(gd, value);
return;
}
else if(value==='remove') {
// delete all
delete layout.shapes;
fullLayout.shapes = [];
shapes.drawAll(gd);
deleteAllShapes(gd);
return;
}
else if(opt && value!=='add') {
// make the same change to all shapes
for(i = 0; i < fullLayout.shapes.length; i++) {
shapes.draw(gd, i, opt, value);
}
updateAllShapes(gd, opt, value);
return;
}
else {
// add a new empty annotation
index = fullLayout.shapes.length;
fullLayout.shapes.push({});
index = gd._fullLayout.shapes.length;
gd._fullLayout.shapes.push({});
}
}

if(!opt && value) {
if(value==='remove') {
fullLayout._shapelayer.selectAll('[data-index="'+index+'"]')
.remove();
fullLayout.shapes.splice(index,1);
layout.shapes.splice(index,1);
for(i=index; i<fullLayout.shapes.length; i++) {
fullLayout._shapelayer
.selectAll('[data-index="'+(i+1)+'"]')
.attr('data-index',String(i));

// redraw all shapes past the removed one,
// so they bind to the right events
shapes.draw(gd,i);
}
deleteShape(gd, index);
return;
}
else if(value==='add' || Plotly.Lib.isPlainObject(value)) {
fullLayout.shapes.splice(index,0,{});
insertShape(gd, index, value);
}
}

var rule = Plotly.Lib.isPlainObject(value) ?
Plotly.Lib.extendFlat({}, value) :
{text: 'New text'};
updateShape(gd, index, opt, value);
return;
};

if(layout.shapes) {
layout.shapes.splice(index, 0, rule);
} else {
layout.shapes = [rule];
}
function replaceAllShapes(gd, newShapes) {
gd.layout.shapes = newShapes;
shapes.supplyLayoutDefaults(gd.layout, gd._fullLayout);
shapes.drawAll(gd);
return;
}

for(i=fullLayout.shapes.length-1; i>index; i--) {
fullLayout._shapelayer
.selectAll('[data-index="'+(i-1)+'"]')
.attr('data-index',String(i));
shapes.draw(gd,i);
}
}
function deleteAllShapes(gd) {
delete gd.layout.shapes;
gd._fullLayout.shapes = [];
shapes.drawAll(gd);
return;
}

function updateAllShapes(gd, opt, value) {
for(var i = 0; i < gd._fullLayout.shapes.length; i++) {
shapes.draw(gd, i, opt, value);
}
return;
}

function deleteShape(gd, index) {
gd._fullLayout._shapelayer.selectAll('[data-index="' + index + '"]')
.remove();

gd._fullLayout.shapes.splice(index, 1);

gd.layout.shapes.splice(index, 1);

for(var i = index; i < gd._fullLayout.shapes.length; i++) {
// redraw all shapes past the removed one,
// so they bind to the right events
gd._fullLayout._shapelayer
.selectAll('[data-index="' + (i+1) + '"]')
.attr('data-index', String(i));
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI String(i) is unnecessary - i will be coerced to a string by .attr.

shapes.draw(gd, i);
}

return;
}

function insertShape(gd, index, newShape) {
gd._fullLayout.shapes.splice(index, 0, {});

var rule = Plotly.Lib.isPlainObject(newShape) ?
Plotly.Lib.extendFlat({}, newShape) :
{text: 'New text'};

if(gd.layout.shapes) {
gd.layout.shapes.splice(index, 0, rule);
} else {
gd.layout.shapes = [rule];
}

for(var i = gd._fullLayout.shapes.length - 1; i > index; i--) {
gd._fullLayout._shapelayer
.selectAll('[data-index="' + (i - 1) + '"]')
.attr('data-index', String(i));
shapes.draw(gd, i);
}

return;
}

function updateShape(gd, index, opt, value) {
var i;

// remove the existing shape if there is one
fullLayout._shapelayer.selectAll('[data-index="'+index+'"]').remove();
gd._fullLayout._shapelayer.selectAll('[data-index="' + index + '"]')
.remove();

// remember a few things about what was already there,
var optionsIn = layout.shapes[index];
var optionsIn = gd.layout.shapes[index];

// (from annos...) not sure how we're getting here... but C12 is seeing a bug
// where we fail here when they add/remove annotations
Expand Down Expand Up @@ -261,8 +288,8 @@ shapes.draw = function(gd, index, opt, value) {
optionsIn[posAttr] = position;
}

var options = handleShapeDefaults(optionsIn, fullLayout);
fullLayout.shapes[index] = options;
var options = handleShapeDefaults(optionsIn, gd._fullLayout);
gd._fullLayout.shapes[index] = options;

var attrs = {
'data-index': String(index),
Expand All @@ -273,15 +300,20 @@ shapes.draw = function(gd, index, opt, value) {

var lineColor = options.line.width ? options.line.color : 'rgba(0,0,0,0)';

var path = fullLayout._shapelayer.append('path')
var path = gd._fullLayout._shapelayer.append('path')
.attr(attrs)
.style('opacity', options.opacity)
.call(Plotly.Color.stroke, lineColor)
.call(Plotly.Color.fill, options.fillcolor)
.call(Plotly.Drawing.dashLine, options.line.dash, options.line.width);

if(clipAxes) path.call(Plotly.Drawing.setClipUrl, 'clip' + fullLayout._uid + clipAxes);
};
if(clipAxes) {
path.call(Plotly.Drawing.setClipUrl,
'clip' + gd._fullLayout._uid + clipAxes);
}

return;
}

function decodeDate(convertToPx) {
return function(v) { return convertToPx(v.replace('_', ' ')); };
Expand Down
11 changes: 8 additions & 3 deletions src/plots/cartesian/graph_interact.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1433,6 +1433,9 @@ function dragBox(gd, plotinfo, x, y, w, h, ns, ew) {

var x0,
y0,
xs = plotinfo.x()._offset,
Copy link
Contributor

Choose a reason for hiding this comment

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

🐄 make the definitions before, and declarations after.

e.g.

var xs = plotinfo.x()._offset,
    ys = plotinfo.y(),_offset,
    x0,
    y0,
    // etc.

ys = plotinfo.y()._offset,
zoomlayer = gd._fullLayout._shapelayer,
box,
lum,
path0,
Expand All @@ -1453,22 +1456,24 @@ function dragBox(gd, plotinfo, x, y, w, h, ns, ew) {
dimmed = false;
zoomMode = 'xy';

zb = plotinfo.plot.append('path')
zb = zoomlayer.append('path')
.attr('class', 'zoombox')
.style({
'fill': lum>0.2 ? 'rgba(0,0,0,0)' : 'rgba(255,255,255,0)',
'stroke-width': 0
})
.attr('transform','translate(' + xs + ' ' + ys + ')')
.attr('d', path0 + 'Z');

corners = plotinfo.plot.append('path')
corners = zoomlayer.append('path')
.attr('class', 'zoombox-corners')
.style({
fill: Plotly.Color.background,
stroke: Plotly.Color.defaultLine,
'stroke-width': 1,
opacity: 0
})
.attr('transform','translate(' + xs + ' ' + ys + ')')
.attr('d','M0,0Z');

clearSelect();
Expand All @@ -1479,7 +1484,7 @@ function dragBox(gd, plotinfo, x, y, w, h, ns, ew) {
// until we get around to persistent selections, remove the outline
// here. The selection itself will be removed when the plot redraws
// at the end.
plotinfo.plot.selectAll('.select-outline').remove();
zoomlayer.selectAll('.select-outline').remove();
}

function zoomMove(dx0, dy0) {
Expand Down
6 changes: 5 additions & 1 deletion src/plots/cartesian/select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ var MINSELECT = constants.MINSELECT;
function getAxId(ax) { return ax._id; }

module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
var plot = dragOptions.plotinfo.plot,
var plot = dragOptions.gd._fullLayout._shapelayer,
dragBBox = dragOptions.element.getBoundingClientRect(),
xs = dragOptions.plotinfo.x()._offset,
ys = dragOptions.plotinfo.y()._offset,
x0 = startX - dragBBox.left,
y0 = startY - dragBBox.top,
x1 = x0,
Expand All @@ -45,6 +47,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
outlines.enter()
.append('path')
.attr('class', function(d) { return 'select-outline select-outline-' + d; })
.attr('transform','translate(' + xs + ' ' + ys + ')')
.attr('d', path0 + 'Z');

var corners = plot.append('path')
Expand All @@ -54,6 +57,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) {
stroke: color.defaultLine,
'stroke-width': 1
})
.attr('transform','translate(' + xs + ' ' + ys + ')')
.attr('d','M0,0Z');


Expand Down