Skip to content

fix: Handle 'pixel' size mode for shape labels #7790

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

Open
camdecoster wants to merge 6 commits into
base: master
Choose a base branch
from
Open

fix: Handle 'pixel' size mode for shape labels #7790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8278abe
Linting/formatting
camdecoster Apr 30, 2026
51e1de9
fix: Handle 'pixel' size mode for shape labels
camdecoster May 1, 2026
a2a7e70
Add test
camdecoster May 1, 2026
48ff512
Add mock and baseline image
camdecoster May 1, 2026
b277856
Add draftlog
camdecoster May 1, 2026
5616f24
Use valid type value
camdecoster May 1, 2026
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
1 change: 1 addition & 0 deletions draftlogs/7790_fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Handle 'pixel' size mode for shape labels [[#7790](https://github.com/plotly/plotly.js/pull/7790)]
34 changes: 24 additions & 10 deletions src/components/shapes/display_labels.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,30 @@ module.exports = function drawLabel(gd, index, options, shapeGroup) {
const xRefType1 = Axes.getRefType(isArrayXref ? options.xref[1] : options.xref);
const yRefType0 = Axes.getRefType(isArrayYref ? options.yref[0] : options.yref);
const yRefType1 = Axes.getRefType(isArrayYref ? options.yref[1] : options.yref);
const x2p = function(v, shift, xa, xRefType) {
return helpers.getDataToPixel(gd, xa, shift, false, xRefType)(v);
};
const y2p = function(v, shift, ya, yRefType) {
return helpers.getDataToPixel(gd, ya, shift, true, yRefType)(v);
};
shapex0 = x2p(options.x0, options.x0shift, xa0, xRefType0);
shapex1 = x2p(options.x1, options.x1shift, xa1, xRefType1);
shapey0 = y2p(options.y0, options.y0shift, ya0, yRefType0);
shapey1 = y2p(options.y1, options.y1shift, ya1, yRefType1);
const x2p = (v, shift, xa, xRefType) => helpers.getDataToPixel(gd, xa, shift, false, xRefType)(v);
const y2p = (v, shift, ya, yRefType) => helpers.getDataToPixel(gd, ya, shift, true, yRefType)(v);
// When using pixel offset mode it's necessary to add the anchor position for the
// correct final value
if (options.xsizemode === 'pixel') {
const xAnchorPos = x2p(options.xanchor, undefined, xa0, xRefType0);
const xShift0 = helpers.getPixelShift(xa0, options.x0shift);
const xShift1 = helpers.getPixelShift(xa0, options.x1shift);
shapex0 = xAnchorPos + options.x0 + xShift0;
shapex1 = xAnchorPos + options.x1 + xShift1;
} else {
shapex0 = x2p(options.x0, options.x0shift, xa0, xRefType0);
shapex1 = x2p(options.x1, options.x1shift, xa1, xRefType1);
}
if (options.ysizemode === 'pixel') {
const yAnchorPos = y2p(options.yanchor, undefined, ya0, yRefType0);
const yShift0 = helpers.getPixelShift(ya0, options.y0shift);
const yShift1 = helpers.getPixelShift(ya0, options.y1shift);
shapey0 = yAnchorPos - options.y0 + yShift0;
shapey1 = yAnchorPos - options.y1 + yShift1;
} else {
shapey0 = y2p(options.y0, options.y0shift, ya0, yRefType0);
shapey1 = y2p(options.y1, options.y1shift, ya1, yRefType1);
}
Comment on lines +107 to +130
Copy link
Copy Markdown
Contributor

@emilykl emilykl May 7, 2026

Choose a reason for hiding this comment

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

If I'm understanding the issue correctly (when xsizemode is "pixel", we are supposed to determine shapex0 and shapex1 by first converting xanchor from data to pixels and THEN adding x0 and x1 which are already given in pixels)

... would something like the following work? It feels more intuitive to me, but could be just me.

Suggested change
const x2p = (v, shift, xa, xRefType) => helpers.getDataToPixel(gd, xa, shift, false, xRefType)(v);
const y2p = (v, shift, ya, yRefType) => helpers.getDataToPixel(gd, ya, shift, true, yRefType)(v);
// When using pixel offset mode it's necessary to add the anchor position for the
// correct final value
if (options.xsizemode === 'pixel') {
const xAnchorPos = x2p(options.xanchor, undefined, xa0, xRefType0);
const xShift0 = helpers.getPixelShift(xa0, options.x0shift);
const xShift1 = helpers.getPixelShift(xa0, options.x1shift);
shapex0 = xAnchorPos + options.x0 + xShift0;
shapex1 = xAnchorPos + options.x1 + xShift1;
} else {
shapex0 = x2p(options.x0, options.x0shift, xa0, xRefType0);
shapex1 = x2p(options.x1, options.x1shift, xa1, xRefType1);
}
if (options.ysizemode === 'pixel') {
const yAnchorPos = y2p(options.yanchor, undefined, ya0, yRefType0);
const yShift0 = helpers.getPixelShift(ya0, options.y0shift);
const yShift1 = helpers.getPixelShift(ya0, options.y1shift);
shapey0 = yAnchorPos - options.y0 + yShift0;
shapey1 = yAnchorPos - options.y1 + yShift1;
} else {
shapey0 = y2p(options.y0, options.y0shift, ya0, yRefType0);
shapey1 = y2p(options.y1, options.y1shift, ya1, yRefType1);
}
var x2p, y2p;
// When using pixel offset mode it's necessary to compute position relative to xanchor/yanchor
if (options.xsizemode === 'pixel') {
x2p = (v, shift, xa, xRefType) => helpers.getDataToPixel(gd, xa, shift, false, xRefType)(options.xanchor) + v;
} else {
x2p = (v, shift, xa, xRefType) => helpers.getDataToPixel(gd, xa, shift, false, xRefType)(v);
}
if (options.ysizemode === 'pixel') {
y2p = (v, shift, ya, yRefType) => helpers.getDataToPixel(gd, ya, shift, true, yRefType)(options.yanchor) + v;
} else {
y2p = (v, shift, ya, yRefType) => helpers.getDataToPixel(gd, ya, shift, true, yRefType)(v);
}
shapex0 = x2p(options.x0, options.x0shift, xa0, xRefType0);
shapex1 = x2p(options.x1, options.x1shift, xa1, xRefType1);
shapey0 = y2p(options.y0, options.y0shift, ya0, yRefType0);
shapey1 = y2p(options.y1, options.y1shift, ya1, yRefType1);

}

// Handle `auto` angle
Expand Down
1 change: 1 addition & 0 deletions src/components/shapes/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ function convertPath(options, x2p, y2p) {
});
}

exports.getPixelShift = getPixelShift;
function getPixelShift(axis, shift) {
shift = shift || 0;
var shiftPixels = 0;
Expand Down
Binary file added test/image/baselines/shape_label_pixel_sizemode.png
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@emilykl emilykl May 7, 2026

Choose a reason for hiding this comment

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

@camdecoster Would it be possible to edit the existing shape label mock rather than adding a new one?

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
52 changes: 52 additions & 0 deletions test/image/mocks/shape_label_pixel_sizemode.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
{
"data": [{ "type": "scatter", "x": [1, 2], "y": [1, 2] }],
"layout": {
"margin": { "t": 92 },
"shapes": [
{
"label": {
"text": "Circle",
"textangle": 0,
"textposition": "middle center",
"xanchor": "center",
"yanchor": "middle"
},
"showlegend": false,
"type": "circle",
"x0": -25,
"x1": 25,
"xanchor": 1,
"xref": "x",
"xsizemode": "pixel",
"y0": 25,
"y1": 75,
"yanchor": 1,
"yref": "paper",
"ysizemode": "pixel"
},
{
"label": {
"text": "Rect",
"textangle": 0,
"textposition": "middle center",
"xanchor": "center",
"yanchor": "middle"
},
"showlegend": false,
"type": "rect",
"x0": -25,
"x1": 25,
"xanchor": 2,
"xref": "x",
"xsizemode": "pixel",
"y0": 25,
"y1": 75,
"yanchor": 1,
"yref": "paper",
"ysizemode": "pixel"
}
],
"width": 500,
"height": 500
}
}
Loading
Loading