fix: orient logic for when bar with x=T + simplify logic (vega#8739)

Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>

examples/compiled/bar_1d_temporal.vg.json

@@ -0,0 +1,76 @@
+{
+  "$schema": "https://vega.github.io/schema/vega/v5.json",
+  "background": "white",
+  "padding": 5,
+  "width": 200,
+  "height": 20,
+  "style": "cell",
+  "data": [
+    {
+      "name": "source_0",
+      "url": "data/cars.json",
+      "format": {"type": "json", "parse": {"Year": "date"}},
+      "transform": [
+        {
+          "type": "filter",
+          "expr": "(isDate(datum[\"Year\"]) || (isValid(datum[\"Year\"]) && isFinite(+datum[\"Year\"])))"
+        }
+      ]
+    }
+  ],
+  "marks": [
+    {
+      "name": "marks",
+      "type": "rect",
+      "style": ["bar"],
+      "from": {"data": "source_0"},
+      "encode": {
+        "update": {
+          "fill": {"value": "#4c78a8"},
+          "ariaRoleDescription": {"value": "bar"},
+          "description": {
+            "signal": "\"Year: \" + (timeFormat(datum[\"Year\"], '%b %d, %Y'))"
+          },
+          "xc": {"scale": "x", "field": "Year"},
+          "width": {"value": 5},
+          "y": {"value": 0},
+          "y2": {"field": {"group": "height"}}
+        }
+      }
+    }
+  ],
+  "scales": [
+    {
+      "name": "x",
+      "type": "time",
+      "domain": {"data": "source_0", "field": "Year"},
+      "range": [0, {"signal": "width"}],
+      "padding": 5
+    }
+  ],
+  "axes": [
+    {
+      "scale": "x",
+      "orient": "bottom",
+      "grid": true,
+      "tickCount": {"signal": "ceil(width/40)"},
+      "domain": false,
+      "labels": false,
+      "aria": false,
+      "maxExtent": 0,
+      "minExtent": 0,
+      "ticks": false,
+      "zindex": 0
+    },
+    {
+      "scale": "x",
+      "orient": "bottom",
+      "grid": false,
+      "title": "Year",
+      "labelFlush": true,
+      "labelOverlap": true,
+      "tickCount": {"signal": "ceil(width/40)"},
+      "zindex": 0
+    }
+  ]
+}

examples/specs/bar_1d_temporal.vl.json

@@ -0,0 +1,11 @@
+{
+  "$schema": "https://vega.github.io/schema/vega-lite/v5.json",
+  "data": {"url": "data/cars.json"},
+  "mark": {"type": "bar", "orient": "vertical"},
+  "encoding": {
+    "x": {
+      "field": "Year",
+      "type": "temporal"
+    }
+  }
+}

src/channeldef.ts

@@ -701,9 +701,9 @@ export function isContinuousFieldOrDatumDef<F extends Field>(
   return (isTypedFieldDef(cd) && !isDiscrete(cd)) || isNumericDataDef(cd);
 }
 
-export function isQuantitativeFieldOrDatumDef<F extends Field>(cd: ChannelDef<F>) {
+export function isUnbinnedQuantitativeFieldOrDatumDef<F extends Field>(cd: ChannelDef<F>) {
   // TODO: make datum support DateTime object
-  return channelDefType(cd) === 'quantitative' || isNumericDataDef(cd);
+  return (isTypedFieldDef(cd) && cd.type === 'quantitative' && !cd.bin) || isNumericDataDef(cd);
 }
 
 export function isNumericDataDef<F extends Field>(cd: ChannelDef<F>): cd is DatumDef<F, number> {

src/compile/mark/init.ts

@@ -1,6 +1,6 @@
 import {Orientation, SignalRef} from 'vega';
 import {isBinned, isBinning} from '../../bin';
-import {isContinuousFieldOrDatumDef, isFieldDef, isNumericDataDef, TypedFieldDef} from '../../channeldef';
+import {isFieldDef, isNumericDataDef, isUnbinnedQuantitativeFieldOrDatumDef, isTypedFieldDef} from '../../channeldef';
 import {Config} from '../../config';
 import {Encoding, isAggregate} from '../../encoding';
 import {replaceExprRef} from '../../expr';
@@ -178,39 +178,31 @@ function orient(mark: Mark, encoding: Encoding<string>, specifiedOrient: Orienta
     // falls through
     case LINE:
     case TICK: {
-      // Tick is opposite to bar, line, area and never have ranged mark.
-      const xIsContinuous = isContinuousFieldOrDatumDef(x);
-      const yIsContinuous = isContinuousFieldOrDatumDef(y);
+      const xIsMeasure = isUnbinnedQuantitativeFieldOrDatumDef(x);
+      const yIsMeasure = isUnbinnedQuantitativeFieldOrDatumDef(y);
 
       if (specifiedOrient) {
         return specifiedOrient;
-      } else if (xIsContinuous && !yIsContinuous) {
+      } else if (xIsMeasure && !yIsMeasure) {
+        // Tick is opposite to bar, line, area
         return mark !== 'tick' ? 'horizontal' : 'vertical';
-      } else if (!xIsContinuous && yIsContinuous) {
+      } else if (!xIsMeasure && yIsMeasure) {
+        // Tick is opposite to bar, line, area
         return mark !== 'tick' ? 'vertical' : 'horizontal';
-      } else if (xIsContinuous && yIsContinuous) {
-        const xDef = x as TypedFieldDef<string>; // we can cast here since they are surely fieldDef
-        const yDef = y as TypedFieldDef<string>;
-
-        const xIsTemporal = xDef.type === TEMPORAL;
-        const yIsTemporal = yDef.type === TEMPORAL;
+      } else if (xIsMeasure && yIsMeasure) {
+        return 'vertical';
+      } else {
+        const xIsTemporal = isTypedFieldDef(x) && x.type === TEMPORAL;
+        const yIsTemporal = isTypedFieldDef(y) && y.type === TEMPORAL;
 
-        // temporal without timeUnit is considered continuous, but better serves as dimension
+        // x: T, y: N --> vertical tick
         if (xIsTemporal && !yIsTemporal) {
-          return mark !== 'tick' ? 'vertical' : 'horizontal';
+          return 'vertical';
         } else if (!xIsTemporal && yIsTemporal) {
-          return mark !== 'tick' ? 'horizontal' : 'vertical';
-        }
-
-        if (!xDef.aggregate && yDef.aggregate) {
-          return mark !== 'tick' ? 'vertical' : 'horizontal';
-        } else if (xDef.aggregate && !yDef.aggregate) {
-          return mark !== 'tick' ? 'horizontal' : 'vertical';
+          return 'horizontal';
         }
-        return 'vertical';
-      } else {
-        return undefined;
       }
+      return undefined;
     }
   }
   return 'vertical';

test/compile/mark/init.test.ts

@@ -102,6 +102,15 @@ describe('compile/mark/init', () => {
       });
       expect(model.markDef.orient).toBe('vertical');
     });
+    it('should return correct default for T', () => {
+      const model = parseUnitModelWithScaleAndLayoutSize({
+        mark: 'bar',
+        encoding: {
+          x: {type: 'temporal', field: 'bar'}
+        }
+      });
+      expect(model.markDef.orient).toBe('vertical');
+    });
 
     it('should return correct default for empty plot', () => {
       const model = parseUnitModelWithScaleAndLayoutSize({