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.

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

Add sync option to tickmode of cartesian axes to sync ticks with overlaying axis #6356

Merged
merged 60 commits into from Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
e982570
Added first working version of sync axes
filipesantiagoAM Oct 31, 2022
b68537e
Null verification to the overlaying axis
filipesantiagoAM Nov 1, 2022
debd59b
added comments for readability
Nov 1, 2022
c3fcb6d
npm run schema
Nov 1, 2022
48e60c2
Merge branch 'plotly:master' into first_poc_tickmode_sync
fsantiago99 Nov 2, 2022
6a4a5ba
Removed math.abs() from prepTicks sync
filipesantiagoAM Nov 14, 2022
76debab
Rename new tickmode sync mock to z-new_tickmode_sync
filipesantiagoAM Nov 14, 2022
4ab851e
Add baseline image for new tickmode sync mock
filipesantiagoAM Nov 14, 2022
e983614
Fix mock-validation tests for z-new_tickmode_sync mock
filipesantiagoAM Nov 14, 2022
6446faf
Merge branch 'first_poc_tickmode_sync' of https://github.com/VictorBe…
filipesantiagoAM Nov 14, 2022
e62be46
Remove unused tickmode sync mode to some plot types
filipesantiagoAM Nov 26, 2022
93f4a8e
Moves tickmode sync to use last position on the grid
filipesantiagoAM Dec 1, 2022
22c703a
Removed tickmode sync from non-relevant plot types
filipesantiagoAM Dec 1, 2022
3fb6996
Updated tickmode test image baseline
filipesantiagoAM Dec 1, 2022
e4909bf
Merge remote-tracking branch 'origin/master' into first_poc_tickmode_…
filipesantiagoAM Dec 1, 2022
7590ef8
Removed grid from the overlayed axis
filipesantiagoAM Dec 1, 2022
3f5e118
Fixed baseline image for tickmode sync
filipesantiagoAM Dec 2, 2022
ed7b3ee
Merge branch 'first_poc_tickmode_sync' of https://github.com/VictorBe…
filipesantiagoAM Dec 2, 2022
6352d93
Trigger Build
filipesantiagoAM Dec 2, 2022
b03faea
Moved tickmode sync logic to syncTicks
filipesantiagoAM Dec 4, 2022
b5e957b
Updated baseline image for z-new-tickmode-sync mock
filipesantiagoAM Dec 4, 2022
6fd182c
Fix minor ticks for tickmode sync
filipesantiagoAM Dec 14, 2022
a4417a6
Update baseline image and revert lint fixes
filipesantiagoAM Dec 14, 2022
8c25bf6
Fix lint errors
filipesantiagoAM Dec 14, 2022
5d5c8da
Improved tickmode sync logic to round ticks
filipesantiagoAM Dec 15, 2022
bb3e7ae
Update baseline image for z-new_tickmode_sync
filipesantiagoAM Dec 15, 2022
262141b
Sync overlayed axis when main axis redraw
filipesantiagoAM Dec 16, 2022
92281af
Fix validation for redraw
filipesantiagoAM Dec 16, 2022
5ad0185
Merge branch 'plotly:master' into first_poc_tickmode_sync
filipesantiagoAM Dec 16, 2022
8ddfcc3
Reorder axes with dependency tickmode sync
filipesantiagoAM Dec 21, 2022
ea84512
Merge branch 'first_poc_tickmode_sync' of https://github.com/VictorBe…
filipesantiagoAM Dec 21, 2022
434d703
Merge branch 'plotly:master' into first_poc_tickmode_sync
filipesantiagoAM Dec 21, 2022
77dee05
Merge branch 'master' into first_poc_tickmode_sync
filipesantiagoAM Dec 23, 2022
25d015c
Renamed sync axes function
filipesantiagoAM Dec 23, 2022
0ff7ca5
Merge branch 'first_poc_tickmode_sync' of https://github.com/VictorBe…
filipesantiagoAM Dec 23, 2022
3a87dd4
Add test for tickmode sync xAxes
filipesantiagoAM Dec 23, 2022
179dd5b
Sync overlayed axis relayout
filipesantiagoAM Dec 23, 2022
a08766b
Update baseline image z-new_tickmode_sync
filipesantiagoAM Dec 23, 2022
5afad5e
Remove unused tickmode sync logic
filipesantiagoAM Dec 24, 2022
7a9f1e7
Reorder axes with dependency tickmode sync
filipesantiagoAM Dec 26, 2022
16aa4be
Lint error fix
filipesantiagoAM Dec 26, 2022
74509ef
Fix gridcolor for tickmode sync
filipesantiagoAM Jan 3, 2023
43e17de
Update baseline image for tickmode sync
filipesantiagoAM Jan 3, 2023
45616b4
z-new_tickmode_sync.json adjustments
filipesantiagoAM Jan 3, 2023
30ebad1
Update baseline image for tickmode sync
filipesantiagoAM Jan 3, 2023
ac768c0
Ticks sync round improvement
filipesantiagoAM Jan 3, 2023
3ca4aed
Improved ticks sync calculations
filipesantiagoAM Jan 3, 2023
f28ee6c
Reverted "Improved ticks sync calculations"
filipesantiagoAM Jan 3, 2023
9dd3003
Update baseline image for tickmode sync
filipesantiagoAM Jan 3, 2023
12ae78b
Merge branch 'first_poc_tickmode_sync' of https://github.com/VictorBe…
filipesantiagoAM Jan 3, 2023
0e35a0f
Improve z-new_tickmode_sync.json
filipesantiagoAM Jan 4, 2023
685bef6
Add grid to z-new_tickmode_sync.json
filipesantiagoAM Jan 4, 2023
6c363ad
Update baseline image for tickmode sync
filipesantiagoAM Jan 4, 2023
fd78e8d
Removing tickmode sync from minor ticks
filipesantiagoAM Jan 4, 2023
795350a
Update plot-schema without tickmode for minor axes
filipesantiagoAM Jan 4, 2023
50165f6
Fix lint
filipesantiagoAM Jan 5, 2023
04cc07e
Merge branch 'plotly:master' into first_poc_tickmode_sync
filipesantiagoAM Jan 11, 2023
6e317d4
Improved tickmode schema declaration
filipesantiagoAM Jan 11, 2023
196b7dd
Fix lint error
filipesantiagoAM Jan 11, 2023
25e1389
added commit file to draftlogs
Jan 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/components/colorbar/attributes.js
Expand Up @@ -129,7 +129,19 @@ module.exports = overrideAll({
description: 'Sets the color of padded area.'
},
// tick and title properties named and function exactly as in axes
tickmode: axesAttrs.tickmode,
tickmode: extendFlat({}, axesAttrs.tickmode, {
values: ['auto', 'linear', 'array'],
description: [
'Sets the tick mode for this axis.',
'If *auto*, the number of ticks is set via `nticks`.',
'If *linear*, the placement of the ticks is determined by',
'a starting position `tick0` and a tick step `dtick`',
'(*linear* is the default value if `tick0` and `dtick` are provided).',
'If *array*, the placement of the ticks is set via `tickvals`',
'and the tick text is `ticktext`.',
'(*array* is the default value if `tickvals` is provided).'
].join(' ')
}),
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved
nticks: axesAttrs.nticks,
tick0: axesAttrs.tick0,
dtick: axesAttrs.dtick,
Expand Down
76 changes: 70 additions & 6 deletions src/plots/cartesian/axes.js
Expand Up @@ -951,6 +951,13 @@ axes.calcTicks = function calcTicks(ax, opts) {
continue;
}

// fill tickVals based on overlaying axis
if(mockAx.tickmode === 'sync') {
tickVals = [];
ticksOut = syncTicks(ax);
continue;
}

// add a tiny bit so we get ticks which may have rounded out
var exRng = expandRange(rng);
var startTick = exRng[0];
Expand Down Expand Up @@ -1203,6 +1210,51 @@ axes.calcTicks = function calcTicks(ax, opts) {
return ticksOut;
};

function filterRangeBreaks(ax, ticksOut) {
if(ax.rangebreaks) {
// remove ticks falling inside rangebreaks
ticksOut = ticksOut.filter(function(d) {
return ax.maskBreaks(d.x) !== BADNUM;
});
}

return ticksOut;
}

function syncTicks(ax) {
// get the overlaying axis
var baseAxis = ax._mainAxis;

var ticksOut = [];
if(baseAxis._vals) {
for(var i = 0; i < baseAxis._vals.length; i++) {
// get the position of the every tick
var pos = baseAxis.l2p(baseAxis._vals[i].x);

// get the tick for the current axis based on position
var vali = ax.p2l(pos);
var val1 = ax.p2l(pos - 0.1);
Copy link
Contributor

Choose a reason for hiding this comment

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

@archmoj Now this code makes sense? Now I feel we are at the same position we were before when you suggest this piece of code.

            var vali = ax.p2l(pos);
            var val1 = ax.p2l(pos - 0.5);
            var val2 = ax.p2l(pos + 0.5);
            var d = 1 + Math.round(Math.log10(Math.abs(val2 - val1)));
            var e = Math.pow(10, -d);
            var valR = Math.round(vali * e) / e;
            var objR = axes.tickText(ax, valR);
            var obj = axes.tickText(ax, vali);
            obj.text = objR.text;
Suggested change
var val1 = ax.p2l(pos - 0.1);
var vali = ax.p2l(pos);
var obj = axes.tickText(ax, vali);

I agree we need to improve the roundings but not sure how to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call @filipesantiagoAM.
Please feel free to simplify this:

            // get the tick for the current axis based on position
            var vali = ax.p2l(pos);
            var obj = axes.tickText(ax, vali);

var val2 = ax.p2l(pos + 0.1);
var d = 1 + Math.round(Math.log10(Math.abs(val2 - val1)));
filipesantiagoAM marked this conversation as resolved.
Show resolved Hide resolved
var e = Math.pow(10, -d);
var valR = Math.round(vali * e) / e;
var obj = axes.tickText(ax, valR);

archmoj marked this conversation as resolved.
Show resolved Hide resolved
// assign minor ticks
if(baseAxis._vals[i].minor) {
obj.minor = true;
obj.text = '';
}

ticksOut.push(obj);
}
}

ticksOut = filterRangeBreaks(ax, ticksOut);

return ticksOut;
}

function arrayTicks(ax) {
var rng = Lib.simpleMap(ax.range, ax.r2l);
var exRng = expandRange(rng);
Expand Down Expand Up @@ -1249,12 +1301,7 @@ function arrayTicks(ax) {
}
}

if(ax.rangebreaks) {
// remove ticks falling inside rangebreaks
ticksOut = ticksOut.filter(function(d) {
return ax.maskBreaks(d.x) !== BADNUM;
});
}
ticksOut = filterRangeBreaks(ax, ticksOut);

return ticksOut;
}
Expand Down Expand Up @@ -2256,6 +2303,18 @@ axes.draw = function(gd, arg, opts) {
return ax.overlaying;
});

// order axes that have dependency to other axes
axList.map(function(axId) {
var ax = axes.getFromId(gd, axId);

if(ax.tickmode === 'sync' && ax.overlaying) {
var overlayingIndex = axList.findIndex(function(axis) {return axis === ax.overlaying;});

if(overlayingIndex >= 0) {
axList.unshift(axList.splice(overlayingIndex, 1).shift());
}
}
});

var axShifts = {'false': {'left': 0, 'right': 0}};

Expand Down Expand Up @@ -3247,6 +3306,11 @@ axes.drawTicks = function(gd, ax, opts) {
axes.drawGrid = function(gd, ax, opts) {
opts = opts || {};

if(ax.tickmode === 'sync') {
// for tickmode sync we use the overlaying axis grid
return;
}

var cls = ax._id + 'grid';

var hasMinor = ax.minor && ax.minor.showgrid;
Expand Down
8 changes: 8 additions & 0 deletions src/plots/cartesian/dragbox.js
Expand Up @@ -727,15 +727,23 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
}
}

function pushActiveAxIdsSynced(axList, axisType) {
for(i = 0; i < axList.length; i++) {
if(!axList[i].fixedrange && axList[i][axisType]) {activeAxIds.push(axList[i][axisType]._id);}
}
}

if(editX) {
pushActiveAxIds(xaxes);
pushActiveAxIds(links.xaxes);
pushActiveAxIds(matches.xaxes);
pushActiveAxIdsSynced(plotinfo.overlays, 'xaxis');
}
if(editY) {
pushActiveAxIds(yaxes);
pushActiveAxIds(links.yaxes);
pushActiveAxIds(matches.yaxes);
pushActiveAxIdsSynced(plotinfo.overlays, 'yaxis');
}

updates = {};
Expand Down
6 changes: 4 additions & 2 deletions src/plots/cartesian/layout_attributes.js
Expand Up @@ -14,7 +14,7 @@ var DAY_OF_WEEK = constants.WEEKDAY_PATTERN;

var tickmode = {
valType: 'enumerated',
values: ['auto', 'linear', 'array'],
values: ['auto', 'linear', 'array', 'sync'],
editType: 'ticks',
impliedEdits: {tick0: undefined, dtick: undefined},
description: [
archmoj marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -25,7 +25,9 @@ var tickmode = {
'(*linear* is the default value if `tick0` and `dtick` are provided).',
'If *array*, the placement of the ticks is set via `tickvals`',
'and the tick text is `ticktext`.',
'(*array* is the default value if `tickvals` is provided).'
'(*array* is the default value if `tickvals` is provided).',
'If *sync*, the number of ticks will sync with the overlayed axis',
'set by `overlaying` property.'
].join(' ')
};

Expand Down
2 changes: 2 additions & 0 deletions src/plots/cartesian/tick_value_defaults.js
Expand Up @@ -28,6 +28,8 @@ module.exports = function handleTickValueDefaults(containerIn, containerOut, coe

if(tickmode === 'auto') {
coerce(prefix + 'nticks');
} else if(tickmode === 'sync') {
coerce('overlaying');
Copy link
Contributor

Choose a reason for hiding this comment

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

@alexcjohnson what do you suggest here?

It looks like we shouldn't use overlaying attribute "directly":

// overlaying: not used directly, just put here for reference
// values are false and any other same-letter axis id that's not
// itself overlaying anything
overlaying: {
valType: 'enumerated',
values: [
'free',
constants.idRegex.x.toString(),
constants.idRegex.y.toString()
],
editType: 'plot',
description: [
'If set a same-letter axis id, this axis is overlaid on top of',
'the corresponding same-letter axis, with traces and axes visible for both',
'axes.',
'If *false*, this axis does not overlay any same-letter axes.',
'In this case, for axes with overlapping domains only the highest-numbered',
'axis will be visible.'
].join(' ')
},

Instead overlaying is coerced here:

overlaying = Lib.coerce(containerIn, containerOut, {
overlaying: {
valType: 'enumerated',
values: [false].concat(overlayableAxes),
dflt: false
}

Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm understanding this correctly, the situation we want to avoid is tickmode:'sync' with no overlaying. And the challenge is that when we get here, overlaying hasn't been coerced yet. But @archmoj is right, we can't use the attribute directly and we don't have enough information at this point to coerce it correctly anyway, since we don't know what other axes exist yet.

So what I'd suggest is to leave that block out here, but fix it when we DO have enough information to do that - right after that spot @archmoj pointed to where we coerce overlaying, there's this block:

If we get in there with tickmode === 'sync', we should force it to 'auto', which means we also need nticks:

if(containerOut.tickmode === 'sync') {
    containerOut.tickmode = 'auto';
    coerce('nticks');
}

} else if(tickmode === 'linear') {
// dtick is usually a positive number, but there are some
// special strings available for log or date axes
Expand Down
14 changes: 13 additions & 1 deletion src/plots/gl3d/layout/axis_attributes.js
Expand Up @@ -74,7 +74,19 @@ module.exports = overrideAll({
anim: false
}),
// ticks
tickmode: axesAttrs.tickmode,
tickmode: extendFlat({}, axesAttrs.tickmode, {
values: ['auto', 'linear', 'array'],
description: [
'Sets the tick mode for this axis.',
'If *auto*, the number of ticks is set via `nticks`.',
'If *linear*, the placement of the ticks is determined by',
'a starting position `tick0` and a tick step `dtick`',
'(*linear* is the default value if `tick0` and `dtick` are provided).',
'If *array*, the placement of the ticks is set via `tickvals`',
'and the tick text is `ticktext`.',
'(*array* is the default value if `tickvals` is provided).'
].join(' ')
}),
nticks: axesAttrs.nticks,
tick0: axesAttrs.tick0,
dtick: axesAttrs.dtick,
Expand Down
14 changes: 13 additions & 1 deletion src/plots/polar/layout_attributes.js
Expand Up @@ -23,7 +23,19 @@ var axisLineGridAttr = overrideAll({
}, 'plot', 'from-root');

var axisTickAttrs = overrideAll({
tickmode: axesAttrs.tickmode,
tickmode: extendFlat({}, axesAttrs.tickmode, {
values: ['auto', 'linear', 'array'],
description: [
'Sets the tick mode for this axis.',
'If *auto*, the number of ticks is set via `nticks`.',
'If *linear*, the placement of the ticks is determined by',
'a starting position `tick0` and a tick step `dtick`',
'(*linear* is the default value if `tick0` and `dtick` are provided).',
'If *array*, the placement of the ticks is set via `tickvals`',
'and the tick text is `ticktext`.',
'(*array* is the default value if `tickvals` is provided).'
].join(' ')
}),
nticks: axesAttrs.nticks,
tick0: axesAttrs.tick0,
dtick: axesAttrs.dtick,
Expand Down
14 changes: 13 additions & 1 deletion src/plots/ternary/layout_attributes.js
Expand Up @@ -15,7 +15,19 @@ var ternaryAxesAttrs = {
},
color: axesAttrs.color,
// ticks
tickmode: axesAttrs.tickmode,
tickmode: extendFlat({}, axesAttrs.tickmode, {
values: ['auto', 'linear', 'array'],
description: [
'Sets the tick mode for this axis.',
'If *auto*, the number of ticks is set via `nticks`.',
'If *linear*, the placement of the ticks is determined by',
'a starting position `tick0` and a tick step `dtick`',
'(*linear* is the default value if `tick0` and `dtick` are provided).',
'If *array*, the placement of the ticks is set via `tickvals`',
'and the tick text is `ticktext`.',
'(*array* is the default value if `tickvals` is provided).'
].join(' ')
}),
nticks: extendFlat({}, axesAttrs.nticks, {dflt: 6, min: 1}),
tick0: axesAttrs.tick0,
dtick: axesAttrs.dtick,
Expand Down
14 changes: 13 additions & 1 deletion src/traces/indicator/attributes.js
Expand Up @@ -307,7 +307,19 @@ module.exports = {
dflt: true
}),
// tick and title properties named and function exactly as in axes
tickmode: axesAttrs.tickmode,
tickmode: extendFlat({}, axesAttrs.tickmode, {
values: ['auto', 'linear', 'array'],
description: [
'Sets the tick mode for this axis.',
'If *auto*, the number of ticks is set via `nticks`.',
'If *linear*, the placement of the ticks is determined by',
'a starting position `tick0` and a tick step `dtick`',
'(*linear* is the default value if `tick0` and `dtick` are provided).',
'If *array*, the placement of the ticks is set via `tickvals`',
'and the tick text is `ticktext`.',
'(*array* is the default value if `tickvals` is provided).'
].join(' ')
}),
nticks: axesAttrs.nticks,
tick0: axesAttrs.tick0,
dtick: axesAttrs.dtick,
Expand Down
Binary file added test/image/baselines/z-new_tickmode_sync.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
91 changes: 91 additions & 0 deletions test/image/mocks/z-new_tickmode_sync.json
@@ -0,0 +1,91 @@
{
"data": [
{
"name": "Apples",
"type": "bar",
"x": [
"Jan",
archmoj marked this conversation as resolved.
Show resolved Hide resolved
"Feb",
"Mar",
"Apr",
"May"
],
"y": [
232,
2506,
470,
1864,
-190
]
},
{
"name": "Oranges",
"type": "scatter",
"x": [
archmoj marked this conversation as resolved.
Show resolved Hide resolved
"A",
"B",
"C"
],
"y": [
0,
0.5,
1
],
"yaxis": "y2",
"xaxis": "x2"
}
],
"layout": {
"margin": {
"t": 40,
"r": 70,
"b": 40,
"l": 70
},
"width": 700,
"showlegend": false,
"xaxis2": {
archmoj marked this conversation as resolved.
Show resolved Hide resolved
"anchor": "y2",
"side": "top",
"overlaying": "x",
"tickmode": "sync",
"domain": [
archmoj marked this conversation as resolved.
Show resolved Hide resolved
0.52,
1
]
},
archmoj marked this conversation as resolved.
Show resolved Hide resolved
"yaxis": {
archmoj marked this conversation as resolved.
Show resolved Hide resolved
"title": {
"text": "Apples"
},
"side": "left",
archmoj marked this conversation as resolved.
Show resolved Hide resolved
"range": [
0,
2506
archmoj marked this conversation as resolved.
Show resolved Hide resolved
],
"ticklen": 10,
"tickwidth": 3,
"minor": {
"showgrid": true
}
},
"yaxis2": {
archmoj marked this conversation as resolved.
Show resolved Hide resolved
"title": {
"text": "Oranges"
},
"side": "right",
"range": [
0,
0.5
],
"ticklen": 10,
"tickwidth": 3,
"overlaying": "y",
"tickmode": "sync",
"minor": {
"showgrid": true
},
"zeroline": false
}
}
}