Skip to content

Add polar.hole #2977

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 13 commits into from
Sep 10, 2018
Merged

Add polar.hole #2977

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
39b2ee3
place <g.barlayer> below <g.scatterlayer> in polar subplot
etpinard Sep 6, 2018
ea7fd2c
add polar.hole
etpinard Sep 6, 2018
f547692
add a few TODO comments related to polar.hole
etpinard Sep 6, 2018
35199c6
make mock cartesian scale computation marginally simpler
etpinard Sep 7, 2018
8e5833d
cover a few more cases in polar_hole.json
etpinard Sep 7, 2018
ea877ee
add hover tests with polar.hole>0
etpinard Sep 7, 2018
a859872
add info about polar.hole in setConvert jsdoc
etpinard Sep 7, 2018
776e292
modif polar_hole mock to "see" axis line/tick ordering
etpinard Sep 7, 2018
9c3dcef
draw polar axis ticks above axis lines
etpinard Sep 7, 2018
1122907
fix and :lock: maindrag when polar.hole>0
etpinard Sep 7, 2018
d466146
add 'inner' radial dragbox when polar.hole>0
etpinard Sep 7, 2018
5a998a2
Merge branch 'master' into polar-hole
etpinard Sep 10, 2018
092006a
add 'inner' radial dragbox to all polar subplots
etpinard Sep 10, 2018
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
3 changes: 3 additions & 0 deletions src/plots/polar/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ module.exports = {
'frontplot',
'angular-axis',
'radial-axis',
// TODO hmm, putting angular-line above radial-axis
// can sometimes hide radial ticks and label
// this happens especially for the 'inner' angular axis
'angular-line',
'radial-line'
],
Expand Down
5 changes: 5 additions & 0 deletions src/plots/polar/polar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,9 @@ proto.updateAngularAxis = function(fullLayout, polarLayout) {
}
_this.vangles = vangles;

// TODO maybe two arcs is better here?
// maybe split style attributes between inner and outer angular axes?

updateElement(layers['angular-line'].select('path'), angularLayout.showline, {
d: _this.pathSubplot(),
transform: strTranslate(cx, cy)
Expand Down Expand Up @@ -937,6 +940,8 @@ proto.updateRadialDrag = function(fullLayout) {
var tx = cx + (radius + bl2) * Math.cos(angle0);
var ty = cy - (radius + bl2) * Math.sin(angle0);

// TODO add 'inner' drag box when innerRadius > 0 !!
Copy link
Collaborator

Choose a reason for hiding this comment

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

not quite sure what this means, but if it's something like the clamped inner edge still jumps to be a circle at the center then yeah, we'd better avoid that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This TODO refers to the radial drag box. Currently we have 1 radial drag box per polar subplot out beyond the subplot radius. We don't drag one at the other end of the radial axis as it would conflict with the "main" drag over the subplot.

peek 2018-09-07 12-22

But, when polar.hole > 0 we could add an inner radial drag box that doesn't conflict with the main drag. This inner radial drag box would update radialaxis.range[0].

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh right! Yes, we should add that. Would be confusing if there's room for one but it's not there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

peek 2018-09-07 16-41

I'm really liking this effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in d466146


d3.select(radialDrag)
.attr('transform', strTranslate(tx, ty));

Expand Down