Skip to content

better zooming behavior for errorbars, bars, and box plots #1897

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

Merged
merged 1 commit into from
Jul 19, 2017
Merged

better zooming behavior for errorbars, bars, and box plots #1897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions src/components/errorbars/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ module.exports = function plot(traces, plotinfo, transitionOpts) {

if(isNew) {
yerror = errorbar.append('path')
.style('vector-effect', 'non-scaling-stroke')
.classed('yerror', true);
} else if(hasAnimation) {
yerror = yerror
Expand Down Expand Up @@ -117,6 +118,7 @@ module.exports = function plot(traces, plotinfo, transitionOpts) {

if(isNew) {
xerror = errorbar.append('path')
.style('vector-effect', 'non-scaling-stroke')
Copy link
Contributor

Choose a reason for hiding this comment

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

Something still looks off:

peek 2017-07-19 11-15

Looks like the whiskers appear thicker during zooms.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right - because that's about the length of the whisker, not its stroke. I couldn't think of a way to handle this at first but now that you mention it, I suppose we could have the x scale of the y errorbars (and the y scale of x error bars) get inverse-scaled the same way as we do for both x and y scales of the points... I can look into this but it may take a little more work, do you think it's worth holding up this PR or shall I tackle it separately later?

Copy link
Contributor

@etpinard etpinard Jul 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

right - because that's about the length of the whisker, not its stroke

Thanks for clarification 📚

do you think it's worth holding up this PR or shall I tackle it separately later?

Not worth holding up this PR. 👍

Opening a new issue will be just fine.

.classed('xerror', true);
} else if(hasAnimation) {
xerror = xerror
Expand Down
2 changes: 1 addition & 1 deletion src/plots/cartesian/dragbox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) {
.call(Drawing.setTranslate, clipDx, clipDy)
.call(Drawing.setScale, xScaleFactor2, yScaleFactor2);

var scatterPoints = subplot.plot.select('.scatterlayer').selectAll('.points');
var scatterPoints = subplot.plot.selectAll('.scatterlayer .points, .boxlayer .points');
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice touch 👌


subplot.plot
.call(Drawing.setTranslate, plotDx, plotDy)
Expand Down
6 changes: 4 additions & 2 deletions src/traces/bar/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,10 @@ module.exports = function plot(gd, plotinfo, cdbar) {
// append bar path and text
var bar = d3.select(this);

bar.append('path').attr('d',
'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z');
bar.append('path')
Copy link
Contributor

Choose a reason for hiding this comment

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

Non- ⛔ comment

What about bar text labels?

peek 2017-07-19 11-39

they probably would benefit from the same tricks scatter text nodes get on scroll

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are also tricky - as sometimes they will scale up, sometimes they won't, and sometimes they'll move or rotate after the zoom. Also I played around with including them and the element structure isn't quite set up the same way as scatter text points so it's nontrivial to make it work. I'll add this to the list of possible future improvements.

.style('vector-effect', 'non-scaling-stroke')
.attr('d',
'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z');

appendBarText(gd, bar, d, i, x0, x1, y0, y1);
});
Expand Down
7 changes: 6 additions & 1 deletion src/traces/box/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ module.exports = function plot(gd, plotinfo, cdbox) {
d3.select(this).selectAll('path.box')
.data(Lib.identity)
.enter().append('path')
.style('vector-effect', 'non-scaling-stroke')
.attr('class', 'box')
.each(function(d) {
var posc = posAxis.c2p(d.pos + bPos, true),
Expand Down Expand Up @@ -204,6 +205,7 @@ module.exports = function plot(gd, plotinfo, cdbox) {
});
})
.enter().append('path')
.classed('point', true)
.call(Drawing.translatePoints, xa, ya);
}
// draw mean (and stdev diamond) if desired
Expand All @@ -212,7 +214,10 @@ module.exports = function plot(gd, plotinfo, cdbox) {
.data(Lib.identity)
.enter().append('path')
.attr('class', 'mean')
.style('fill', 'none')
.style({
fill: 'none',
'vector-effect': 'non-scaling-stroke'
})
.each(function(d) {
var posc = posAxis.c2p(d.pos + bPos, true),
pos0 = posAxis.c2p(d.pos + bPos - bdPos, true),
Expand Down