Skip to content

Multi-axis Shapes #7666

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

Draft
alexshoe wants to merge 4 commits into
base: master
Choose a base branch
from
+104 −12
Draft

Multi-axis Shapes #7666

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c8be45f
Extend `xref` and `yref` attributes
alexshoe Dec 2, 2025
a7b3bb2
Add function to help validate number of defining shape vertices
alexshoe Dec 3, 2025
ace820b
Update shape defaults to handle an array of references
alexshoe Dec 4, 2025
a910d42
Modify shape xref/yref coercion logic
alexshoe Dec 5, 2025
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
20 changes: 14 additions & 6 deletions src/components/shapes/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,16 @@ module.exports = templatedArray('shape', {
].join(' ')
},

xref: extendFlat({}, annAttrs.xref, {
xref: {
valType: 'any',
editType: 'calc',
description: [
'Sets the shape\'s x coordinate axis.',
axisPlaceableObjs.axisRefDescription('x', 'left', 'right')
axisPlaceableObjs.axisRefDescription('x', 'left', 'right'),
'If an array of axis IDs is provided, each `x` value will refer to the corresponding axis',
'(e.g., [\'x\', \'x2\'] for a rectangle means `x0` uses the `x` axis and `x1` uses the `x2` axis).',
].join(' ')
}),
},
xsizemode: {
valType: 'enumerated',
values: ['scaled', 'pixel'],
Expand Down Expand Up @@ -193,12 +197,16 @@ module.exports = templatedArray('shape', {
'corresponds to the end of the category.'
].join(' ')
},
yref: extendFlat({}, annAttrs.yref, {
yref: {
valType: 'any',
editType: 'calc',
description: [
'Sets the shape\'s y coordinate axis.',
axisPlaceableObjs.axisRefDescription('y', 'bottom', 'top')
axisPlaceableObjs.axisRefDescription('y', 'bottom', 'top'),
'If an array of axis IDs is provided, each `y` value will refer to the corresponding axis',
'(e.g., [\'y\', \'y2\'] for a rectangle means `y0` uses the `y` axis and `y1` uses the `y2` axis).',
].join(' ')
}),
},
ysizemode: {
valType: 'enumerated',
values: ['scaled', 'pixel'],
Expand Down
33 changes: 27 additions & 6 deletions src/components/shapes/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,39 @@ function handleShapeDefaults(shapeIn, shapeOut, fullLayout) {

// positioning
var axLetters = ['x', 'y'];
for(var i = 0; i < 2; i++) {
var axLetter = axLetters[i];
axLetters.forEach(function(axLetter) {
var attrAnchor = axLetter + 'anchor';
var sizeMode = axLetter === 'x' ? xSizeMode : ySizeMode;
var gdMock = {_fullLayout: fullLayout};
var ax;
var pos2r;
var r2pos;

// xref, yref
var axRef = Axes.coerceRef(shapeIn, shapeOut, gdMock, axLetter, undefined,
'paper');
// xref, yref - handle both string and array values
var axRef;
var refAttr = axLetter + 'ref';
var inputRef = shapeIn[refAttr];

if(Array.isArray(inputRef) && inputRef.length > 0) {
// Array case: use coerceRefArray for validation
var expectedLen = helpers.countDefiningCoords(path, noPath);
axRef = Axes.coerceRefArray(shapeIn, shapeOut, gdMock, axLetter, expectedLen);
shapeOut['_' + axLetter + 'refArray'] = true;

// Need to register the shape with all referenced axes for redrawing purposes
axRef.forEach(function(ref) {
if(Axes.getRefType(ref) === 'range') {
ax = Axes.getFromId(gdMock, ref);
if(ax && ax._shapeIndices.indexOf(shapeOut._index) === -1) {
ax._shapeIndices.push(shapeOut._index);
}
}
Comment on lines +90 to +95
Copy link
Contributor

@camdecoster camdecoster Dec 8, 2025

Choose a reason for hiding this comment

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

Does anything need to happen in the else case?

Copy link
Contributor Author

@alexshoe alexshoe Dec 8, 2025

Choose a reason for hiding this comment

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

I don't believe so since a paper value would not have a valid axis to register with, and paper/domain values wouldn't require a redraw in case of panning/changes to the axis.

});
} else {
// String/undefined case: use coerceRef
axRef = Axes.coerceRef(shapeIn, shapeOut, gdMock, axLetter, undefined, 'paper');
}

var axRefType = Axes.getRefType(axRef);

if(axRefType === 'range') {
Expand Down Expand Up @@ -136,7 +157,7 @@ function handleShapeDefaults(shapeIn, shapeOut, fullLayout) {
shapeOut[attrAnchor] = r2pos(shapeOut[attrAnchor]);
shapeIn[attrAnchor] = inAnchor;
}
}
});

if(noPath) {
Lib.noneOrAll(shapeIn, shapeOut, ['x0', 'x1', 'y0', 'y1']);
Expand Down
19 changes: 19 additions & 0 deletions src/components/shapes/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,25 @@ exports.extractPathCoords = function(path, paramsToUse, isRaw) {
return extractedCoordinates;
};

exports.countDefiningCoords = function(path, isNotPath) {
// non-path shapes always have 2 defining coordinates
if(isNotPath) return 2;
if(!path) return 0;

var segments = path.match(constants.segmentRE);
if(!segments) return 0;

var coordCount = 0;
segments.forEach(function(segment) {
// for each path command, check if there is a drawn coordinate
var segmentType = segment.charAt(0);
var hasDrawnX = constants.paramIsX[segmentType].drawn !== undefined;
var hasDrawnY = constants.paramIsY[segmentType].drawn !== undefined;
if(hasDrawnX || hasDrawnY) coordCount++;
Comment on lines +66 to +70
Copy link
Contributor

@emilykl emilykl Dec 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm. H and V segments cause an odd usability issue I hadn't considered: if a path contains H and V segments then the expected lengths of xref and yref may be different, and the corresponding list items will be "out of sync" with respect to which path segments they correspond to (i.e. it will not always be the case that xref[n] and yref[n] refer to the same path segment).

For example here is a rectangular path: M0,0H10V5H0Z (see visualization)

There are four segments (not counting Z), but only 3 x-values and 2 y-values are needed to define the shape. So you would have:

{
    "path": "M0,0H10V5H0Z",
    "xref": ["x", "x2", "x"],
    "yref": ["y", "y2"],
}

which is a bit odd, and sort a high cognitive load for the user to figure out the right xref and yref lists.

That said, it seems probably better than the other alternative that comes to mind, which would be to force the user to add an xref and a yref for every single segment, even if it will be ignored for some segments.

});
return coordCount;
Comment on lines +64 to +72
Copy link
Contributor

@camdecoster camdecoster Dec 8, 2025

Choose a reason for hiding this comment

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

Could you transform this into an Array.reduce call and return that?

};

exports.getDataToPixel = function(gd, axis, shift, isVertical, refType) {
var gs = gd._fullLayout._size;
var dataToPixel;
Expand Down
40 changes: 40 additions & 0 deletions src/plots/cartesian/axes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var Drawing = require('../../components/drawing');

var axAttrs = require('./layout_attributes');
var cleanTicks = require('./clean_ticks');
var cartesianConstants = require('./constants');

var constants = require('../../constants/numerical');
var ONEMAXYEAR = constants.ONEMAXYEAR;
Expand Down Expand Up @@ -124,6 +125,44 @@ axes.coerceRef = function(containerIn, containerOut, gd, attr, dflt, extraOption
return Lib.coerce(containerIn, containerOut, attrDef, refAttr);
};

/*
* Coerce an array of axis references. Used by shapes for per-coordinate axis references.
*
* attr: the attribute we're generating a reference for. Should end in 'x' or 'y'
* but can be prefixed, like 'ax' for annotation's arrow x
* dflt: the default to coerce to, or blank to use the first axis (falling back on
* extraOption if there is no axis)
* extraOption: aside from existing axes with this letter, what non-axis value is allowed?
Copy link
Contributor

@camdecoster camdecoster Dec 8, 2025

Choose a reason for hiding this comment

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

Did you mean for this to be a question?

Copy link
Contributor Author

@alexshoe alexshoe Dec 8, 2025

Choose a reason for hiding this comment

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

Oops, I just copied this from the original coerceRef comment, I'll fix this

* Only required if it's different from `dflt`
*/
axes.coerceRefArray = function(containerIn, containerOut, gd, attr, expectedLen) {
var axLetter = attr.charAt(attr.length - 1);
var axlist = gd._fullLayout._subplots[axLetter + 'axis'];
axlist = axlist.concat(axlist.map(function(x) { return x + ' domain'; }));
var refAttr = attr + 'ref';
var axRef = containerIn[refAttr];
var dflt = axlist.length ? axlist[0] : 'paper';

// Handle array length mismatch
if(axRef.length > expectedLen) {
// if the array is longer than the expected length, truncate it
axRef = axRef.slice(0, expectedLen);
} else if(axRef.length < expectedLen) {
// if the array is shorter than the expected length, extend using the default value
axRef = axRef.concat(Array(expectedLen - axRef.length).fill(dflt));
}

// Check all references, replace with default if invalid
for(var i = 0; i < axRef.length; i++) {
if(!(axRef[i] === 'paper' || cartesianConstants.idRegex[axLetter].test(axRef[i]))) {
axRef[i] = dflt;
}
}

containerOut[refAttr] = axRef;
Copy link
Contributor

@emilykl emilykl Dec 4, 2025

Choose a reason for hiding this comment

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

The core Lib.coerce() function uses more complex logic here for setting the attribute in the out container -- I'm not 100% sure whether that's needed here, but something to verify.

Copy link
Contributor Author

@alexshoe alexshoe Dec 8, 2025

Choose a reason for hiding this comment

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

just documenting that we chatted about this and decided we'd look deeper into Lib.coerce + if it's alright to not call it here.

return axRef;
};

/*
* Get the type of an axis reference. This can be 'range', 'domain', or 'paper'.
* This assumes ar is a valid axis reference and returns 'range' if it doesn't
Expand All @@ -134,6 +173,7 @@ axes.coerceRef = function(containerIn, containerOut, gd, attr, dflt, extraOption
*/
axes.getRefType = function(ar) {
if(ar === undefined) { return ar; }
if(Array.isArray(ar)) { return 'array'; }
if(ar === 'paper') { return 'paper'; }
if(ar === 'pixel') { return 'pixel'; }
if(/( domain)$/.test(ar)) { return 'domain'; } else { return 'range'; }
Expand Down