auto-margin top and bottom (closes #1859)

fix a bug with inferred labels, which can be empty ("Date", "Year"… are filtered out).
observe marginLeft and marginRight when specified in the axis mark (this wasn't tested and had lead to a regression)

Author: Fil

src/dimensions.js

@@ -4,6 +4,7 @@ import {isOrdinalScale} from "./scales.js";
 import {offset} from "./style.js";
 import {defaultWidth, monospaceWidth} from "./marks/text.js";
 import {outerDimensions} from "./scales.js";
+import {formatAxisLabel} from "./marks/axis.js";
 
 const marginMedium = 60;
 const marginLarge = 90;
@@ -21,18 +22,28 @@ export function autoMarginK(
   dimensions,
   context
 ) {
+  const actualLabel = formatAxisLabel(scale, scales[scale], {...options, label});
   let {data, facets, channels} = stateByMark.get(mark);
   if (mark.initializer) ({channels} = mark.initializer(data, facets, {}, scales, dimensions, context));
-  const width = mark.monospace ? monospaceWidth : defaultWidth;
-  const labelPenalty =
-    (label ?? scales[scale].label ?? "") !== "" &&
-    (labelAnchor === "center" || (labelAnchor == null && scales[scale].bandwidth));
-  const l =
-    max(channels.text.value, (t) => (t ? width(`${t}`) : NaN)) * (mark.monospace ? 0.6 : 1) + (labelPenalty ? 100 : 0);
-  const m = l >= 500 ? marginLarge : l >= 295 ? marginMedium : null;
+  if (scale === "y" || scale === "fy") {
+    const width = mark.monospace ? monospaceWidth : defaultWidth;
+    const labelPenalty = actualLabel && (labelAnchor === "center" || (labelAnchor == null && scales[scale].bandwidth));
+    const l =
+      max(channels.text.value, (t) => (t ? width(`${t}`) : NaN)) * (mark.monospace ? 0.6 : 1) +
+      (labelPenalty ? 100 : 0);
+    const m = l >= 500 ? marginLarge : l >= 295 ? marginMedium : null;
+    return m === null
+      ? options
+      : scale === "fy"
+      ? {...options, facet: {[margin]: m, ...options.facet}}
+      : {[margin]: m, ...options};
+  }
+  // For the x scale, we bump the margin only if the axis uses multi-line ticks!
+  const re = new RegExp(/\n/);
+  const m = actualLabel && channels.text.value.some((d) => re.test(d)) ? 40 : null;
   return m === null
     ? options
-    : scale === "fy"
+    : scale === "fx"
     ? {...options, facet: {[margin]: m, ...options.facet}}
     : {[margin]: m, ...options};
 }
@@ -53,9 +64,21 @@ export function createDimensions(scales, marks, options = {}) {
   // revise the dimensions if necessary.
   const autoMargins = [];
   for (const m of marks) {
-    let {marginTop, marginRight, marginBottom, marginLeft, autoMarginRight, autoMarginLeft, frameAnchor} = m;
-    if (autoMarginLeft && frameAnchor === "left") autoMargins.push(["marginLeft", autoMarginLeft, m]);
+    let {
+      marginTop,
+      marginRight,
+      marginBottom,
+      marginLeft,
+      autoMarginTop,
+      autoMarginRight,
+      autoMarginBottom,
+      autoMarginLeft,
+      frameAnchor
+    } = m;
+    if (autoMarginTop) autoMargins.push(["marginTop", autoMarginTop, m]);
     if (autoMarginRight && frameAnchor === "right") autoMargins.push(["marginRight", autoMarginRight, m]);
+    if (autoMarginBottom) autoMargins.push(["marginBottom", autoMarginBottom, m]);
+    if (autoMarginLeft && frameAnchor === "left") autoMargins.push(["marginLeft", autoMarginLeft, m]);
     if (marginTop > marginTopDefault) marginTopDefault = marginTop;
     if (marginRight > marginRightDefault) marginRightDefault = marginRight;
     if (marginBottom > marginBottomDefault) marginBottomDefault = marginBottom;

src/marks/axis.js

@@ -100,12 +100,12 @@ function axisKy(
     marginRight === undefined &&
     anchor === "right" &&
     x == null && {scale: k, labelAnchor, label};
+  marginRight ??= margin === undefined ? (anchor === "right" ? 40 : 0) : margin;
   const autoMarginLeft = margin === undefined &&
     marginLeft === undefined &&
     anchor === "left" &&
     x == null && {scale: k, labelAnchor, label};
-  marginRight = margin === undefined ? (anchor === "right" ? 40 : 0) : margin;
-  marginLeft = margin === undefined ? (anchor === "left" ? 40 : 0) : margin;
+  marginLeft ??= margin === undefined ? (anchor === "left" ? 40 : 0) : margin;
   return marks(
     tickSize && !isNoneish(stroke)
       ? axisTickKy(k, anchor, data, {
@@ -193,9 +193,9 @@ function axisKx(
     tickRotate,
     y,
     margin,
-    marginTop = margin === undefined ? (anchor === "top" ? 30 : 0) : margin,
+    marginTop,
     marginRight = margin === undefined ? 20 : margin,
-    marginBottom = margin === undefined ? (anchor === "bottom" ? 30 : 0) : margin,
+    marginBottom,
     marginLeft = margin === undefined ? 20 : margin,
     label,
     labelAnchor,
@@ -209,6 +209,16 @@ function axisKx(
   tickRotate = number(tickRotate);
   if (labelAnchor !== undefined) labelAnchor = keyword(labelAnchor, "labelAnchor", ["center", "left", "right"]);
   labelArrow = maybeLabelArrow(labelArrow);
+  const autoMarginTop = margin === undefined &&
+    marginTop === undefined &&
+    anchor === "top" &&
+    y == null && {scale: k, labelAnchor, label};
+  marginTop ??= margin === undefined ? (anchor === "top" ? 30 : 0) : margin;
+  const autoMarginBottom = margin === undefined &&
+    marginBottom === undefined &&
+    anchor === "bottom" &&
+    y == null && {scale: k, labelAnchor, label};
+  marginBottom ??= margin === undefined ? (anchor === "bottom" ? 30 : 0) : margin;
   return marks(
     tickSize && !isNoneish(stroke)
       ? axisTickKx(k, anchor, data, {
@@ -238,6 +248,8 @@ function axisKx(
           marginRight,
           marginBottom,
           marginLeft,
+          autoMarginTop,
+          autoMarginBottom,
           ...options
         })
       : null,
@@ -627,8 +639,10 @@ function axisMark(mark, k, anchor, ariaLabel, data, options, initialize) {
     channels = {};
   }
   m.ariaLabel = ariaLabel;
-  m.autoMarginLeft = options.autoMarginLeft;
+  m.autoMarginTop = options.autoMarginTop;
   m.autoMarginRight = options.autoMarginRight;
+  m.autoMarginBottom = options.autoMarginBottom;
+  m.autoMarginLeft = options.autoMarginLeft;
   if (m.clip === undefined) m.clip = false; // don’t clip axes by default
   return m;
 }
@@ -709,7 +723,7 @@ function inferScaleOrder(scale) {
 
 // Takes the scale label, and if this is not an ordinal scale and the label was
 // inferred from an associated channel, adds an orientation-appropriate arrow.
-function formatAxisLabel(k, scale, {anchor, label = scale.label, labelAnchor, labelArrow} = {}) {
+export function formatAxisLabel(k, scale, {anchor, label = scale.label, labelAnchor, labelArrow} = {}) {
   if (label == null || (label.inferred && hasTemporalDomain(scale) && /^(date|time|year)$/i.test(label))) return;
   label = String(label); // coerce to a string after checking if inferred
   if (labelArrow === "auto") labelArrow = (!scale.bandwidth || scale.interval) && !/[↑↓→←]/.test(label);