Skip to content

Conversation

ayjayt
Copy link
Contributor

@ayjayt ayjayt commented Feb 9, 2024

Fixes #6890

Breakdown of the fix:

ax._depth is not calculated properly in the standoff conditional:

} else if(ax.title.hasOwnProperty('standoff')) {
seq.push(function() {
ax._depth = majorTickSigns[4] * (getLabelLevelBbox()[ax.side] - mainLinePositionShift);
});
}

Lots of positioning calculations rely on that getLabelLevelBbox() which, bypassing the cache, is this:

function calcLabelLevelBbox(ax, cls) {
    var top, bottom;
    var left, right;

    if(ax._selections[cls].size()) {
        ... // iterate over ticklabels and get their DOM properties
    } else {
        top = bottom = left = right = 0;
    }

    return { top: top, bottom: bottom, left: left, right: right, height: bottom - top, width: right - left };
}

The issue being the else branch, where we have no ticklabels on which to loop. We shouldn't return 0 as null- 0 is a legitimate coordinate and will fool consumers. Instead, if we accept that an empty bounding box still has a position, but with a width and height of 0, we can calculate said position (where the ticklabel would be) and return a bounding box with no width or height. That's what I do in the fix using makeLabelFns(), and it seems to work well.

var dummyCalc = axes.makeLabelFns(ax, mainLinePositionShift);
top = bottom = dummyCalc.yFn({dx: 0, dy: 0, fontSize: 0}); // I don't know what dx, dy is
left = right = dummyCalc.xFn({dx: 0, dy: 0, fontSize: 0});

Let's see if these initial commits past the tests. Probably get rid of the mocks?

@ayjayt ayjayt marked this pull request as ready for review February 9, 2024 13:10
@archmoj archmoj added bug something broken community community contribution status: reviewable labels Mar 6, 2024
@archmoj
Copy link
Contributor

archmoj commented Mar 6, 2024

Thanks for the PR. It seems I don't have permission to push to your repository.
So I merge this and add the tests in a separate PR.
💃

@archmoj archmoj merged commit 5b1e104 into plotly:master Mar 6, 2024
@ayjayt
Copy link
Contributor Author

ayjayt commented Mar 6, 2024

Thanks @archmoj , sorry if I'm forgetting the protocol a bit with PRs here. I'm a bit swamped too but I'll get back to these probably by the end of the week to do merges/changelogs/tests/todo lists etc.

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

Labels

bug something broken community community contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: standoff causes axis title position to break if no tick labels

2 participants