Nice find and diagnosis@my-tien! I wonder though if there's a better way to do this - specifically, adding this whole extra loop at the end, in addition to being unnecessary vs. just doing the desired thing from the start, I worry it will cause those labels to be missing later like if you drag the axis around, either during or after the drag.

Instead, I wonder if we can avoid this extra step by instead ofopacity: 0 | 1 usedisplay: none | block here:

and here:

Because unlikeopacity: 0,display: none causesgetBoundingClientRect to give zero size.

Hey Alex, thanks a lot for taking the time! 👍
You are absolutely right, in fact I tried to do this exact same change before, but did it incorrectly: I triedt.style('display', null), but of course it should bet.style('display', 'none'). Now I tried it correctly and it seems to work fine. I will push an update later today.

@alexcjohnson Pushed new implementation withdisplay: 'none' | null as is being done at several other locations in axes.js.

@my-tien this looks great. There are some more test failures inaxes_test.js, seehttps://app.circleci.com/pipelines/github/plotly/plotly.js/11937/workflows/35ad298f-0a05-44b8-95de-f8adbfef3f09/jobs/264167 - as mentioned@emilykl is working on the image test failures in#7418 but please make sure thatonly image tests are failing 😅

The failing tests were exactly those I adjusted so that they passed on my machine. I reverted the changes.

Excellent, looks great! This time the image tests actually ran, and for some reasoncontainer-colorbar-vertical-w-margin failed to render in both cases - can you try this mock on your branch and see if something really did break there? Once that passes we'll also need a baseline image for your new mock, but that's easiest to do off the artifacts in thetest-baselines job, once we get to that point.

So the issue is related to automargin logic: A bbox was calculated from thedisplay: 'none' label ifautomargin was set totrue . I also saw thatadjustTicklabelOverflow was called again and again, alternating the label display betweennull and'none'.
I now have new failing jasmine tests which I'll have to figure out.

Comments on my attempted fix welcomed as I'm not yet very clear about the required rendering and re-rendering steps.

Okay, I think I understand a little bit more what's going on. Next commit incoming soon... But this change will probably change a number of baseline images because hidden tick labels affected a couple of them.

@my-tien Likewise please mergemainmaster into this branch to take advantage of the changes in#7418 . (I'm not sure you're actually hitting those issues in this branch, but just in case.)

@alexcjohnson@my-tien One interesting case from the image diffs -- I don't think the rightward image iswrong, since it was kind of just a fortunate edge case that the gridlines were aligned before, but for a user who finds themself in this situation, is there an easy way in the figure definition to force the gridlines to be aligned?

main	my-tien:label-spacing

@my-tien Thecontainer-colorbar image diffs still seem wrong to me (given the way the legend extends off the bottom of the chart) -- are you still working on theautomargin behavior?

Otherwise this is looking good to me.

Hi Emily,
thanks for looking at this!

@my-tien Thecontainer-colorbar image diffs still seem wrong to me (given the way the legend extends off the bottom of the chart) -- are you still working on theautomargin behavior?

Otherwise this is looking good to me.

I'm not 100% sure what legend you mean, do you mean the yellow box around the colorbar?
(previous)

(new)

The colorbar container and the x-axis ticks are now cut-off because the hidden -0.5 label on the y-axis doesn't take up space anymore and therefore doesn't push down the margins, i.e. same behavior as whenyaxis.automargin is set to false. (I also think it is correct, that -0.5 is hidden, because the defaultticklabeloverflow behavior for this axis ishide past (graph)div).
If I now setlayout.title.automargin: false as well, the chart is exactly 300 px high as specified in the layout height.

Compare to the current behavior when automargin is completely turned off, the colorbar container is also cut-off. So if this is a bug, I think it happens somewhere else:

codepen

One interesting case from the image diffs -- I don't think the rightward image is wrong, since it was kind of just a fortunate edge case that the gridlines were aligned before, but for a user who finds themself in this situation, is there an easy way in the figure definition to force the gridlines to be aligned?

The main situation you'd want these aligned is when they're either literally the same x axis or the two x axes are matched. Hopefully that's handled correctly already? Absent that I'd say the behavior here is correct.

Thecontainer-colorbar image diffs still seem wrong to me (given the way the legend extends off the bottom of the chart)

Hmph there are a bunch of issues here I'm not sure we ever really considered re: automargin, and I think@my-tien is right that this is not a problem with this PR but with colorbars. This colorbar has default position (y=0.5) and size (len=1), which means it extends beyond the plot area only due to however we decide to treat its border: does the border fitinside its specified space, in which case the thicker you make the border the more it needs to squeeze its content, or does the border gooutside the specified space, in which case it should impact margins. Or is the midline of the border on the specified box, in which case it's a bit of each? I don't know the right answer here but this shouldn't block this PR, we should make a new issue and consider how we can best align the behavior with other components (legends, text annotations, axis lines).

There are also the tick marks, which are apparently not hidden on the x axis but don't trigger automargin even though arguably they should. That's another issue independent of this PR, though I'd call that relatively low-priority: how often do real plots have tick marks with no labels at all?

Just to make sure: from my side no additional changes are required since the existing problems weren't caused by this PR, right?

Just to make sure: from my side no additional changes are required since the existing problems weren't caused by this PR, right?

@my-tien I believe that's correct, I don't think the Jasmine test failures are related.

Could you download the changed baseline images from the "Artifacts" tab of thetest-baselines CircleCI job, and commit them to this PR? You may need to push a dummy commit first to trigger the pipeline again, I wasn't able to re-run it manually.

Could you also mergemaster into this branch again? When you do so, that should trigger the pipeline.

Merged and updated baseline images

Hi@emilykl, is there any chance for this PR to make it into the next release?

@my-tien Yes I hope we can get this into the next release. I need to figure out why the test pipelines are failing which I hope to do today (no action required on your part). Thank you for your patience.

@my-tien Could you please merge master into this branch?
Thank you.

@my-tien, could you please cherry-pick commitdfcdfdf onto this branch?

Thanks@emilykl and@archmoj and@alexcjohnson! <3