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 support for xref/yref paper for shapes with layer "between" #6989

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6aa17d5
shape.layer = "between": Add support for xref/yref paper
my-tien Apr 30, 2024
6722977
Don't draw cartesianLayerBelow in rangesliders.
my-tien Apr 30, 2024
822a89e
Add draftlog for PR 6989
my-tien Apr 30, 2024
eea649f
Indicate imageLayerBetween and shapeLayerBetween as private by prepen…
my-tien May 1, 2024
6414991
Fix clear after large splom graph
my-tien May 1, 2024
cd3c002
Update baseline shapes_layer_below_traces
my-tien May 2, 2024
8c7c92b
don't change cartesian layer CSS class name
my-tien May 2, 2024
bf1167d
don't change xy CSS class name
my-tien May 2, 2024
d5d0d34
Adjust number of sublots in plot_interact_test
my-tien May 3, 2024
0a1a5a3
Update cartesian_tests
my-tien May 3, 2024
5514c8c
splom_test: Remove test that ensures that xaxislayer-above comes afte…
my-tien May 4, 2024
5cd2fec
splom_test: update test on how many inner nodes are expected in carte…
my-tien May 4, 2024
ae69416
Add comment explaining why rangeslider's plotinfo has plotgroup: [und…
my-tien May 10, 2024
bad1bcf
reverse for loop for clearing <g subplot> in cartesian plot.
my-tien May 10, 2024
b9ec442
plot_interact_test: replace countSubplots with countSubplotsBackgroun…
my-tien May 10, 2024
dcc77e5
Revert changes to shapes_layer_below_traces and move those changes in…
my-tien May 10, 2024
21c3ea3
Fix for loop start index for clearing subplots
my-tien May 10, 2024
6c2164a
Add baseline image for mock zz_shapes_layer_between_xref_paper
my-tien May 10, 2024
1e3c760
Merge remote-tracking branch 'origin-plotly/master' into shapes_paper…
my-tien Jun 18, 2024
313480d
Merge remote-tracking branch 'origin-plotly/master' into shapes_paper…
my-tien Jun 19, 2024
4e0d697
Merge branch 'master' into shapes_paper_layer_between
my-tien Jun 27, 2024
cb495d3
Update leftover property name _cartesianlayer -> _cartesianlayerAbove
my-tien Jun 27, 2024
ea6875c
Merge remote-tracking branch 'origin-plotly/master' into shapes_paper…
my-tien Nov 6, 2024
bed5ca4
Fix drawFramework after merging zorder changes.
my-tien Nov 8, 2024
68441e0
Fix expected number of subplots in splom_test
my-tien Nov 13, 2024
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/6989_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Add support for xref and yref "paper" for shapes with layer "between" [[#6989](https://github.com/plotly/plotly.js/pull/6989)]
2 changes: 1 addition & 1 deletion src/components/rangeslider/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ function drawRangePlot(rangeSlider, gd, axisOpts, opts) {

var plotinfo = {
id: id,
plotgroup: plotgroup,
plotgroup: [undefined, plotgroup],
my-tien marked this conversation as resolved.
Show resolved Hide resolved
xaxis: xa,
yaxis: ya,
isRangePlot: true
Expand Down
4 changes: 2 additions & 2 deletions src/components/shapes/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ function drawOne(gd, index) {

if(options.layer === 'above') {
drawShape(gd._fullLayout._shapeUpperLayer);
} else if(options.layer === 'between') {
drawShape(gd._fullLayout._shapeLayerBetween);
} else if(options.xref === 'paper' || options.yref === 'paper') {
drawShape(gd._fullLayout._shapeLowerLayer);
} else if(options.layer === 'between') {
drawShape(plotinfo.shapelayerBetween);
} else {
if(plotinfo._hadPlotinfo) {
var mainPlot = plotinfo.mainplotinfo || plotinfo;
Expand Down
7 changes: 5 additions & 2 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -3782,8 +3782,11 @@ function makePlotFramework(gd) {
.classed('shapelayer', true);

// single cartesian layer for the whole plot
fullLayout._cartesianlayer = fullLayout._paper.append('g').classed('cartesianlayer', true);

fullLayout._cartesianlayerBelow = fullLayout._paper.append('g').classed('cartesianlayer-below', true);
var layerBetween = fullLayout._paper.append('g').classed('layer-between', true);
fullLayout._imageLayerBetween = layerBetween.append('g').classed('imagelayer', true);
fullLayout._shapeLayerBetween = layerBetween.append('g').classed('shapelayer', true);
fullLayout._cartesianlayerAbove = fullLayout._paper.append('g').classed('cartesianlayer', true);
// single polar layer for the whole plot
fullLayout._polarlayer = fullLayout._paper.append('g').classed('polarlayer', true);

Expand Down
3 changes: 1 addition & 2 deletions src/plot_api/subroutines.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ function lsInner(gd) {
} else {
var xDomain = plotinfo.xaxis.domain;
var yDomain = plotinfo.yaxis.domain;
var plotgroup = plotinfo.plotgroup;

var plotgroup = plotinfo.plotgroup[0];
if(overlappingDomain(xDomain, yDomain, lowerDomains)) {
var pgNode = plotgroup.node();
var plotgroupBg = plotinfo.bg = Lib.ensureSingle(plotgroup, 'rect', 'bg');
Expand Down
127 changes: 80 additions & 47 deletions src/plots/cartesian/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,11 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout)
if(oldFullLayout._hasOnlyLargeSploms && !newFullLayout._hasOnlyLargeSploms) {
for(k in oldPlots) {
plotinfo = oldPlots[k];
if(plotinfo.plotgroup) plotinfo.plotgroup.remove();
if(plotinfo.plotgroup) {
for(var idx = 0; idx < plotinfo.plotgroup.length; idx++) {
my-tien marked this conversation as resolved.
Show resolved Hide resolved
plotinfo.plotgroup[idx].remove();
}
}
}
}

Expand Down Expand Up @@ -353,8 +357,8 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout)

if(hadCartesian && !hasCartesian) {
// if we've gotten rid of all cartesian traces, remove all the subplot svg items

purgeSubplotLayers(oldFullLayout._cartesianlayer.selectAll('.subplot'), oldFullLayout);
purgeSubplotLayers(oldFullLayout._cartesianlayerBelow.selectAll('.subplot'), oldFullLayout);
purgeSubplotLayers(oldFullLayout._cartesianlayerAbove.selectAll('.subplot'), oldFullLayout);
oldFullLayout._defs.selectAll('.axesclip').remove();
delete oldFullLayout._axisConstraintGroups;
delete oldFullLayout._axisMatchGroups;
Expand All @@ -365,7 +369,8 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout)
var oldSubplotId = oldSubplotList.cartesian[i];
if(!newPlots[oldSubplotId]) {
var selector = '.' + oldSubplotId + ',.' + oldSubplotId + '-x,.' + oldSubplotId + '-y';
oldFullLayout._cartesianlayer.selectAll(selector).remove();
oldFullLayout._cartesianlayerAbove.selectAll(selector).remove();
oldFullLayout._cartesianlayerBelow.selectAll(selector).remove();
removeSubplotExtras(oldSubplotId, oldFullLayout);
}
}
Expand All @@ -376,23 +381,39 @@ exports.drawFramework = function(gd) {
var fullLayout = gd._fullLayout;
var subplotData = makeSubplotData(gd);

var subplotLayers = fullLayout._cartesianlayer.selectAll('.subplot')
var subplotLayersAbove = fullLayout._cartesianlayerAbove.selectAll('.subplot')
.data(subplotData, String);

subplotLayers.enter().append('g')
subplotLayersAbove.enter().append('g')
.attr('class', function(d) { return 'subplot ' + d[0]; });
subplotLayersAbove.order();
subplotLayersAbove.exit()
.call(purgeSubplotLayers, fullLayout);

subplotLayers.order();

subplotLayers.exit()
var subplotLayersBelow = fullLayout._cartesianlayerBelow.selectAll('.subplot')
.data(subplotData, String);
subplotLayersBelow.enter().append('g')
.attr('class', function(d) { return 'subplot ' + d[0] + '-below'; });
subplotLayersBelow.order();
subplotLayersBelow.exit()
.call(purgeSubplotLayers, fullLayout);

subplotLayers.each(function(d) {
subplotLayersBelow.each(function(d) {
var id = d[0];
var plotinfo = fullLayout._plots[id];

plotinfo.plotgroup = d3.select(this);
makeSubplotLayer(gd, plotinfo);
plotinfo.plotgroup = [d3.select(this)];
makeSubplotLayerBelow(gd, plotinfo);

// make separate drag layers for each subplot,
// but append them to paper rather than the plot groups,
// so they end up on top of the rest
plotinfo.draglayer = ensureSingle(fullLayout._draggers, 'g', id);
});
subplotLayersAbove.each(function(d) {
var id = d[0];
var plotinfo = fullLayout._plots[id];
plotinfo.plotgroup = plotinfo.plotgroup.concat([d3.select(this)]);
makeSubplotLayerAbove(gd, plotinfo);

// make separate drag layers for each subplot,
// but append them to paper rather than the plot groups,
Expand All @@ -402,7 +423,7 @@ exports.drawFramework = function(gd) {
};

exports.rangePlot = function(gd, plotinfo, cdSubplot) {
makeSubplotLayer(gd, plotinfo);
makeSubplotLayerAbove(gd, plotinfo);
plotOne(gd, plotinfo, cdSubplot);
Plots.style(gd);
};
Expand Down Expand Up @@ -468,8 +489,50 @@ function makeSubplotData(gd) {
return subplotData;
}

function makeSubplotLayer(gd, plotinfo) {
var plotgroup = plotinfo.plotgroup;
function makeSubplotLayerBelow(gd, plotinfo) {
var plotgroup = plotinfo.plotgroup[0];
var hasOnlyLargeSploms = gd._fullLayout._hasOnlyLargeSploms;

if(!plotinfo.mainplot) {
var backLayer = ensureSingle(plotgroup, 'g', 'layer-subplot');
plotinfo.shapelayer = ensureSingle(backLayer, 'g', 'shapelayer');
plotinfo.imagelayer = ensureSingle(backLayer, 'g', 'imagelayer');

plotinfo.minorGridlayer = ensureSingle(plotgroup, 'g', 'minor-gridlayer');
plotinfo.gridlayer = ensureSingle(plotgroup, 'g', 'gridlayer');
plotinfo.zerolinelayer = ensureSingle(plotgroup, 'g', 'zerolinelayer');
} else {
var mainplotinfo = plotinfo.mainplotinfo;

// now make the components of overlaid subplots
// overlays don't have backgrounds, and append all
// their other components to the corresponding
// extra groups of their main plots.

plotinfo.minorGridlayer = mainplotinfo.minorGridlayer;
plotinfo.gridlayer = mainplotinfo.gridlayer;
plotinfo.zerolinelayer = mainplotinfo.zerolinelayer;
}

// common attributes for all subplots, overlays or not

if(!hasOnlyLargeSploms) {
ensureSingleAndAddDatum(plotinfo.minorGridlayer, 'g', plotinfo.xaxis._id);
ensureSingleAndAddDatum(plotinfo.minorGridlayer, 'g', plotinfo.yaxis._id);
plotinfo.minorGridlayer.selectAll('g')
.map(function(d) { return d[0]; })
.sort(axisIds.idSort);

ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.xaxis._id);
ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.yaxis._id);
plotinfo.gridlayer.selectAll('g')
.map(function(d) { return d[0]; })
.sort(axisIds.idSort);
}
}

function makeSubplotLayerAbove(gd, plotinfo) {
var plotgroup = plotinfo.plotgroup[1];
var id = plotinfo.id;
var xLayer = constants.layerValue2layerClass[plotinfo.xaxis.layer];
var yLayer = constants.layerValue2layerClass[plotinfo.yaxis.layer];
Expand All @@ -487,18 +550,6 @@ function makeSubplotLayer(gd, plotinfo) {
plotinfo.xaxislayer = ensureSingle(plotgroup, 'g', 'xaxislayer-above');
plotinfo.yaxislayer = ensureSingle(plotgroup, 'g', 'yaxislayer-above');
} else {
var backLayer = ensureSingle(plotgroup, 'g', 'layer-subplot');
plotinfo.shapelayer = ensureSingle(backLayer, 'g', 'shapelayer');
plotinfo.imagelayer = ensureSingle(backLayer, 'g', 'imagelayer');

plotinfo.minorGridlayer = ensureSingle(plotgroup, 'g', 'minor-gridlayer');
plotinfo.gridlayer = ensureSingle(plotgroup, 'g', 'gridlayer');
plotinfo.zerolinelayer = ensureSingle(plotgroup, 'g', 'zerolinelayer');

var betweenLayer = ensureSingle(plotgroup, 'g', 'layer-between');
plotinfo.shapelayerBetween = ensureSingle(betweenLayer, 'g', 'shapelayer');
plotinfo.imagelayerBetween = ensureSingle(betweenLayer, 'g', 'imagelayer');

ensureSingle(plotgroup, 'path', 'xlines-below');
ensureSingle(plotgroup, 'path', 'ylines-below');
plotinfo.overlinesBelow = ensureSingle(plotgroup, 'g', 'overlines-below');
Expand Down Expand Up @@ -526,7 +577,7 @@ function makeSubplotLayer(gd, plotinfo) {
}
} else {
var mainplotinfo = plotinfo.mainplotinfo;
var mainplotgroup = mainplotinfo.plotgroup;
var mainplotgroup = mainplotinfo.plotgroup[1];
var xId = id + '-x';
var yId = id + '-y';

Expand All @@ -535,10 +586,6 @@ function makeSubplotLayer(gd, plotinfo) {
// their other components to the corresponding
// extra groups of their main plots.

plotinfo.minorGridlayer = mainplotinfo.minorGridlayer;
plotinfo.gridlayer = mainplotinfo.gridlayer;
plotinfo.zerolinelayer = mainplotinfo.zerolinelayer;

ensureSingle(mainplotinfo.overlinesBelow, 'path', xId);
ensureSingle(mainplotinfo.overlinesBelow, 'path', yId);
ensureSingle(mainplotinfo.overaxesBelow, 'g', xId);
Expand All @@ -560,20 +607,6 @@ function makeSubplotLayer(gd, plotinfo) {

// common attributes for all subplots, overlays or not

if(!hasOnlyLargeSploms) {
my-tien marked this conversation as resolved.
Show resolved Hide resolved
ensureSingleAndAddDatum(plotinfo.minorGridlayer, 'g', plotinfo.xaxis._id);
ensureSingleAndAddDatum(plotinfo.minorGridlayer, 'g', plotinfo.yaxis._id);
plotinfo.minorGridlayer.selectAll('g')
.map(function(d) { return d[0]; })
.sort(axisIds.idSort);

ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.xaxis._id);
ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.yaxis._id);
plotinfo.gridlayer.selectAll('g')
.map(function(d) { return d[0]; })
.sort(axisIds.idSort);
}

plotinfo.xlines
.style('fill', 'none')
.classed('crisp', true);
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,20 @@
],
"layout": {
"shapes": [
{
my-tien marked this conversation as resolved.
Show resolved Hide resolved
"layer": "between",
"name": "my zeroline",
"line": {
"color": "yellow",
"width": 3.0
},
"x0": 0,
"x1": 1,
"xref": "paper",
"y0": 0,
"y1": 0,
"yref": "y"
},
{
"name": "dashdot line",
"fillcolor": "rgba(179,179,179,1)",
Expand Down Expand Up @@ -125,12 +139,15 @@
"0",
"215"
],
"type": "linear"
"type": "linear",
"zerolinewidth": 6,
"zerolinecolor": "rgba(128,128,128,05)"
},
"yaxis2": {
"gridwidth": 2,
"side": "right",
"overlaying": "y"
"overlaying": "y",
"zeroline": false
}

}
Expand Down
9 changes: 5 additions & 4 deletions test/jasmine/tests/cartesian_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ describe('subplot creation / deletion:', function() {

function checkBGLayers(behindCount, x2y2Count, subplots) {
expect(gd.querySelectorAll('.bglayer rect.bg').length).toBe(behindCount);
expect(gd.querySelectorAll('.subplot.x2y2 rect.bg').length).toBe(x2y2Count);
expect(gd.querySelectorAll('.subplot.x2y2-below rect.bg').length).toBe(x2y2Count);

// xy is the first subplot, so it never gets put in front of others
expect(gd.querySelectorAll('.subplot.xy rect.bg').length).toBe(0);
Expand All @@ -445,7 +445,7 @@ describe('subplot creation / deletion:', function() {
expect(gd.querySelectorAll('.subplot.xy3 rect.bg').length).toBe(0);

// verify that these are *all* the subplots and backgrounds we have
expect(gd.querySelectorAll('.subplot').length).toBe(subplots.length);
expect(gd.querySelectorAll('.subplot').length).toBe(subplots.length * 2);
subplots.forEach(function(subplot) {
expect(gd.querySelectorAll('.subplot.' + subplot).length).toBe(1);
});
Expand Down Expand Up @@ -791,14 +791,15 @@ describe('subplot creation / deletion:', function() {
it('clear axis ticks, labels and title when relayout an axis to `*visible:false*', function(done) {
function _assert(xaxis, yaxis) {
var g = d3Select('.subplot.xy');
var gBelow = d3Select('.subplot.xy-below');
var info = d3Select('.infolayer');

expect(g.selectAll('.xtick').size()).toBe(xaxis[0], 'x tick cnt');
expect(g.selectAll('.gridlayer .xgrid').size()).toBe(xaxis[1], 'x gridline cnt');
expect(gBelow.selectAll('.gridlayer .xgrid').size()).toBe(xaxis[1], 'x gridline cnt');
expect(info.selectAll('.g-xtitle').size()).toBe(xaxis[2], 'x title cnt');

expect(g.selectAll('.ytick').size()).toBe(yaxis[0], 'y tick cnt');
expect(g.selectAll('.gridlayer .ygrid').size()).toBe(yaxis[1], 'y gridline cnt');
expect(gBelow.selectAll('.gridlayer .ygrid').size()).toBe(yaxis[1], 'y gridline cnt');
expect(info.selectAll('.g-ytitle').size()).toBe(yaxis[2], 'y title cnt');
}

Expand Down
Loading