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.

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

Fix sankey select error and improve handling of zero indexed subplots #6265

Merged
merged 5 commits into from
Jul 15, 2022
Merged

Fix sankey select error and improve handling of zero indexed subplots #6265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7f0163e
strict subplot test to detect zero subplot e.g. sankey in determineSe…
archmoj Jul 15, 2022
015fe2f
draft log for PR 6265
archmoj Jul 15, 2022
d4b64f7
revise function and variable names
archmoj Jul 15, 2022
c8e9c2c
fix isCartesian checks in select.js
archmoj Jul 15, 2022
b1cee56
fix outline stroke color
archmoj Jul 15, 2022
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/6265_fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix `sankey` select error [[#6265](https://github.com/plotly/plotly.js/pull/6265)]
24 changes: 18 additions & 6 deletions src/components/selections/select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,14 @@ var p2r = helpers.p2r;
var axValue = helpers.axValue;
var getTransform = helpers.getTransform;

function hasSubplot(dragOptions) {
// N.B. subplot may be falsy e.g zero sankey index!
return dragOptions.subplot !== undefined;
}

function prepSelect(evt, startX, startY, dragOptions, mode) {
var isCartesian = !hasSubplot(dragOptions);

var isFreeMode = freeMode(mode);
var isRectMode = rectMode(mode);
var isOpenMode = openMode(mode);
Expand All @@ -64,7 +71,7 @@ function prepSelect(evt, startX, startY, dragOptions, mode) {
var gd = dragOptions.gd;
var fullLayout = gd._fullLayout;
var immediateSelect = isSelectMode && fullLayout.newselection.mode === 'immediate' &&
!dragOptions.subplot; // N.B. only cartesian subplots have persistent selection
isCartesian; // N.B. only cartesian subplots have persistent selection

var zoomLayer = fullLayout._zoomlayer;
var dragBBox = dragOptions.element.getBoundingClientRect();
Expand Down Expand Up @@ -112,9 +119,9 @@ function prepSelect(evt, startX, startY, dragOptions, mode) {
opacity: isDrawMode ? newStyle.opacity / 2 : 1,
fill: (isDrawMode && !isOpenMode) ? newStyle.fillcolor : 'none',
stroke: newStyle.line.color || (
dragOptions.subplot !== undefined ?
'#7f7f7f' : // non-cartesian subplot
Color.contrast(gd._fullLayout.plot_bgcolor) // cartesian subplot
isCartesian ?
Color.contrast(gd._fullLayout.plot_bgcolor) :
'#7f7f7f' // non-cartesian subplot
),
'stroke-dasharray': dashStyle(newStyle.line.dash, newStyle.line.width),
'stroke-width': newStyle.line.width + 'px',
Expand Down Expand Up @@ -145,6 +152,8 @@ function prepSelect(evt, startX, startY, dragOptions, mode) {

if(immediateSelect && !evt.shiftKey) {
dragOptions._clearSubplotSelections = function() {
if(!isCartesian) return;

var xRef = xAxis._id;
var yRef = yAxis._id;
deselectSubplot(gd, xRef, yRef, searchTraces);
Expand Down Expand Up @@ -709,7 +718,7 @@ function clearSelectionsCache(dragOptions, immediateSelect) {
var selections;
if(
isSelectMode &&
!dragOptions.subplot // only allow cartesian - no mapbox for now
!hasSubplot(dragOptions) // only allow cartesian - no mapbox for now
) {
selections = newSelections(outlines, dragOptions);
}
Expand Down Expand Up @@ -748,7 +757,10 @@ function determineSearchTraces(gd, xAxes, yAxes, subplot) {

if(trace.visible !== true || !trace._module || !trace._module.selectPoints) continue;

if(subplot && (trace.subplot === subplot || trace.geo === subplot)) {
if(
hasSubplot({subplot: subplot}) &&
(trace.subplot === subplot || trace.geo === subplot)
) {
searchTraces.push(createSearchInfo(trace._module, cd, xAxes[0], yAxes[0]));
} else if(trace.type === 'splom') {
// FIXME: make sure we don't have more than single axis for splom
Expand Down