Skip to content

Fix react category order of matching axes#4832

Merged
archmoj merged 4 commits into
masterfrom
react-data-order-matching-axes
May 14, 2020
Merged

Fix react category order of matching axes#4832
archmoj merged 4 commits into
masterfrom
react-data-order-matching-axes

Conversation

@archmoj

@archmoj archmoj commented May 14, 2020

Copy link
Copy Markdown
Contributor

Fixes #4718 and fixes #4826 | before vs after.

@plotly/plotly_js

@archmoj archmoj added bug something broken status: reviewable labels May 14, 2020
@archmoj archmoj added this to the v1.54.2 milestone May 14, 2020
Comment thread src/lib/relink_private.js Outdated

@alexcjohnson alexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💃 Looks good, and for the short term this is probably the safest solution. But my comment about "maybe we can avoid relinking these axes entirely" was sincere - see #749, it's very likely that we're majorly overdoing this, and we'll continue creating new bugs with this step until we identify the specific reason why some keys need relinking, others need to NOT be relinked, and still others don't care. The end result, I suspect, will be an opt-in relink step rather than the current opt-out.

@archmoj

archmoj commented May 14, 2020

Copy link
Copy Markdown
Contributor Author

What is hapening (I think) is that this block

if(matchGroups && matchGroups.length) {
var found = false;
for(var i = 0; i < matchGroups.length; i++) {
var group = matchGroups[i];
if(group[axId]) {
found = true;
var categories = null;
var categoriesMap = null;
for(var axId2 in group) {
var ax2 = fullLayout[axisIds.id2name(axId2)];
if(ax2._categories) {
categories = ax2._categories;
categoriesMap = ax2._categoriesMap;
break;
}
}
if(categories && categoriesMap) {
ax._categories = categories;
ax._categoriesMap = categoriesMap;
} else {
emptyCategories();
}
break;
}
}
if(!found) emptyCategories();
} else {
emptyCategories();
}
links the matching items and then those would be replaced by the relink function.
I added few more tests in c2dc252.
& merging...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

faceting react() bug Data order is not applied by React when using matching axes

2 participants