plot
plot copied to clipboard
auto margins
Alternative to #1714. The axis mark sets the value of marginLeft (etc.) not to a number, but to a special value ("auto", not a symbol if we want to keep this usable from outside); the actual numeric value of that special value is computed in createDimensions—where the scales’ types and domains are already materialized
TODO:
- [x] facet axes?
- [x] find a “good” heuristic (consider the scales’ domain, ticks, tickFormat…)
- [x] consider labelAnchor ("center" needs more space)
- [x] use approx. text metrics instead of string.length
- [x] account for the possibility of a centered axis label
- [x] fit & test monospace (see #1877 for easier tests?)
closes #1451 closes #383 closes #1859
This feels almost ready; needs a bit of manual testing (and more snapshot tests to capture more ranges of ticks?). In particular with the auto mark.
cc @tophtucker
Can you regenerate the snapshots so we can more easily review the impact of this change? Thank you. 🙏
Here's a playground to develop tests: https://observablehq.com/@observablehq/auto-margins-1722
Computing all the ticks is probably more adequate, but it is also quite a bit more complicated, since the ticks are computed from the instantiated scales. I've tried to approximate it by doing half-way measures (like, apply tickFormat if given… but on what ticks?) and it seemed worse. My conclusion was that either we have a light system (this one, to which we could add a few common cases such as type: "log"), or we go full-in and measure the actual text that will be rendered, by running a (fake) axisY mark with all the options?
Yes, it’s complicated. 🤔
As discussed, we want to initialize (but not fully render) the axis text mark for y scales; this will give us the text channel which we can use to infer appropriate left & right margins.
I've implemented the new strategy. Still needs a bit of work:
- ~~clean up the code~~
- ~~fix the two test cases mentioned above~~
- ~~keyword check on "auto"~~ (no more keywords!)
- ~~consider auto for top and bottom margins~~
I think this is good to go! Does it need documentation?
Yes I feel good about the result: the tests are all looking great and the only changes are improvements. I think the algorithm is correct. The implementation can certainly be better.
One thing that bugged me is that monospaceWidth and defaultWidth are not aligned; maybe there's still time to change this by making monospace chars 60 instead of 100 — it would be a slightly breaking change, but it would make the two lengths comparable, which is something that was needed here.
There was still a bug with the aspectRatio option.
Here's a "normal" scenario under which the ticks depend on the margins, when the chart has a set aspectRatio. Suppose that with margins of 40, the y axis has ticks such as "1,000", "1,000.5", "1,001". Adding margins on left and right makes the x range smaller, thus giving us a smaller range for y too (because aspectRatio), thus removing the 1000.5 tick. Now, these eventual ticks ("1,000", "1,001") would have fitted with the 40px marginLeft. But we're keeping the 60px.
This is captured with the new marginAspectRatio test.
Probably worth resurrecting this one! 😅
bump