From f56ffa9a9f007eb55de6dfdd94cc45b51d45f686 Mon Sep 17 00:00:00 2001
From: Kanit Wongsuphasawat <kanit.w@databricks.com>
Date: Mon, 8 Aug 2022 17:53:46 -0700
Subject: [PATCH] fix(#8350): fix thin bar when there are too many categories

---
 src/compile/mark/encode/position-rect.ts | 10 +++++++--
 test/compile/mark/bar.test.ts            | 28 ++++++++++--------------
 test/compile/mark/rect.test.ts           | 10 ++++-----
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/compile/mark/encode/position-rect.ts b/src/compile/mark/encode/position-rect.ts
index 891cdbce0ca..48fdfc2b407 100644
--- a/src/compile/mark/encode/position-rect.ts
+++ b/src/compile/mark/encode/position-rect.ts
@@ -77,7 +77,11 @@ function defaultSizeRef(
     if (scale) {
       const scaleType = scale.get('type');
       if (scaleType === 'band') {
-        return {scale: scaleName, band: bandSize.band};
+        let bandWidth = `bandwidth('${scaleName}')`;
+        if (bandSize.band !== 1) {
+          bandWidth = `${bandSize.band} * ${bandWidth}`;
+        }
+        return {signal: `max(0.25, ${bandWidth})`};
       } else if (bandSize.band !== 1) {
         log.warn(log.message.cannotUseRelativeBandSizeWithNonBandScale(scaleType));
         bandSize = undefined;
@@ -139,6 +143,7 @@ function positionAndSize(
       log.warn(log.message.cannotApplySizeToNonOrientedMark(markDef.type));
     }
   }
+  const hasFixedSizeMixins = !!sizeMixins;
 
   // Otherwise, apply default value
   const bandSize = getBandSize({channel, fieldDef, markDef, config, scaleType: scale?.get('type'), useVlSizeChannel});
@@ -156,7 +161,8 @@ function positionAndSize(
     If band is 0.6, the the x/y position in such case should be `(1 - band) / 2` = 0.2
    */
 
-  const defaultBandAlign = scale?.get('type') !== 'band' || !('band' in sizeMixins[vgSizeChannel]) ? 'middle' : 'top';
+  const defaultBandAlign =
+    scale?.get('type') === 'band' && isRelativeBandSize(bandSize) && !hasFixedSizeMixins ? 'top' : 'middle';
 
   const vgChannel = vgAlignedPositionChannel(channel, markDef, config, defaultBandAlign);
   const center = vgChannel === 'xc' || vgChannel === 'yc';
diff --git a/test/compile/mark/bar.test.ts b/test/compile/mark/bar.test.ts
index 25856d993c3..7c7d354843e 100644
--- a/test/compile/mark/bar.test.ts
+++ b/test/compile/mark/bar.test.ts
@@ -18,7 +18,7 @@ describe('Mark: Bar', () => {
 
     it('should draw bar, with y from zero to field value and with band value for x/width', () => {
       expect(props.x).toEqual({scale: 'x', field: 'Origin'});
-      expect(props.width).toEqual({scale: 'x', band: 1});
+      expect(props.width).toEqual({signal: `max(0.25, bandwidth('x'))`});
       expect(props.y).toEqual({scale: 'y', field: 'mean_Acceleration'});
       expect(props.y2).toEqual({scale: 'y', value: 0});
       expect(props.height).toBeUndefined();
@@ -37,7 +37,7 @@ describe('Mark: Bar', () => {
     });
     const props = bar.encodeEntry(model);
     expect(props.x).toEqual({scale: 'x', field: 'Origin', offset: {scale: 'xOffset', field: 'SubOrigin'}});
-    expect(props.width).toEqual({scale: 'xOffset', band: 1});
+    expect(props.width).toEqual({signal: `max(0.25, bandwidth('xOffset'))`});
     expect(props.y).toEqual({scale: 'y', field: 'mean_Acceleration'});
     expect(props.y2).toEqual({scale: 'y', value: 0});
     expect(props.height).toBeUndefined();
@@ -75,7 +75,7 @@ describe('Mark: Bar', () => {
     const props = bar.encodeEntry(model);
 
     expect(props.x).toEqual({scale: 'x', field: 'Origin'});
-    expect(props.width).toEqual({scale: 'x', band: 1});
+    expect(props.width).toEqual({signal: `max(0.25, bandwidth('x'))`});
     expect(props.y).toEqual({scale: 'y', field: 'mean_Acceleration'});
     expect(props.y2).toEqual({scale: 'y', value: 0});
     expect(props.height).toBeUndefined();
@@ -154,7 +154,7 @@ describe('Mark: Bar', () => {
 
     it('should draw bar from zero to field value and with band value for x/width', () => {
       expect(props.y).toEqual({scale: 'y', field: 'Origin'});
-      expect(props.height).toEqual({scale: 'y', band: 1});
+      expect(props.height).toEqual({signal: `max(0.25, bandwidth('y'))`});
       expect(props.x).toEqual({scale: 'x', field: 'mean_Acceleration'});
       expect(props.x2).toEqual({scale: 'x', value: 0});
       expect(props.width).toBeUndefined();
@@ -174,7 +174,7 @@ describe('Mark: Bar', () => {
 
     it('should draw bar from zero to field value and with band value for x/width', () => {
       expect(props.y).toEqual({scale: 'y', field: 'Origin', band: 0.2});
-      expect(props.height).toEqual({scale: 'y', band: 0.6});
+      expect(props.height).toEqual({signal: `max(0.25, 0.6 * bandwidth('y'))`});
       expect(props.x).toEqual({scale: 'x', field: 'mean_Acceleration'});
       expect(props.x2).toEqual({scale: 'x', value: 0});
       expect(props.width).toBeUndefined();
@@ -464,7 +464,7 @@ describe('Mark: Bar', () => {
 
     it('should draw bar with y', () => {
       expect(props.y).toEqual({scale: 'y', field: 'bin_maxbins_10_Horsepower_range'});
-      expect(props.height).toEqual({scale: 'y', band: 1});
+      expect(props.height).toEqual({signal: `max(0.25, bandwidth('y'))`});
     });
   });
 
@@ -481,7 +481,7 @@ describe('Mark: Bar', () => {
 
     it('should draw bar with y', () => {
       expect(props.x).toEqual({scale: 'x', field: 'bin_maxbins_10_Horsepower_range'});
-      expect(props.width).toEqual({scale: 'x', band: 1});
+      expect(props.width).toEqual({signal: `max(0.25, bandwidth('x'))`});
     });
   });
 
@@ -620,10 +620,7 @@ describe('Mark: Bar', () => {
         scale: 'x',
         field: 'Origin'
       });
-      expect(props.width).toEqual({
-        scale: 'x',
-        band: 1
-      });
+      expect(props.width).toEqual({signal: `max(0.25, bandwidth('x'))`});
     });
   });
 
@@ -645,10 +642,7 @@ describe('Mark: Bar', () => {
         scale: 'y',
         field: 'Origin'
       });
-      expect(props.height).toEqual({
-        scale: 'y',
-        band: 1
-      });
+      expect(props.height).toEqual({signal: `max(0.25, bandwidth('y'))`});
     });
   });
 
@@ -918,9 +912,9 @@ describe('Mark: Bar', () => {
       const props = bar.encodeEntry(model);
 
       expect(props.x).toEqual({scale: 'x', field: 'Origin'});
-      expect(props.width).toEqual({scale: 'x', band: 1});
+      expect(props.width).toEqual({signal: `max(0.25, bandwidth('x'))`});
       expect(props.y).toEqual({scale: 'y', field: 'Cylinders'});
-      expect(props.height).toEqual({scale: 'y', band: 1});
+      expect(props.height).toEqual({signal: `max(0.25, bandwidth('y'))`});
     });
   });
 
diff --git a/test/compile/mark/rect.test.ts b/test/compile/mark/rect.test.ts
index 8f9844d3170..da374aec61c 100644
--- a/test/compile/mark/rect.test.ts
+++ b/test/compile/mark/rect.test.ts
@@ -110,7 +110,7 @@ describe('Mark: Rect', () => {
 
     it('should draw bar, with y from zero to field value and x band', () => {
       expect(props.x).toEqual({scale: 'x', field: 'Origin'});
-      expect(props.width).toEqual({scale: 'x', band: 1});
+      expect(props.width).toEqual({signal: `max(0.25, bandwidth('x'))`});
       expect(props.y).toEqual({scale: 'y', field: 'mean_Acceleration'});
       expect(props.y2).toEqual({scale: 'y', value: 0});
       expect(props.height).toBeUndefined();
@@ -170,7 +170,7 @@ describe('Mark: Rect', () => {
 
     it('should draw bar from zero to field value and y band', () => {
       expect(props.y).toEqual({scale: 'y', field: 'Origin'});
-      expect(props.height).toEqual({scale: 'y', band: 1});
+      expect(props.height).toEqual({signal: `max(0.25, bandwidth('y'))`});
       expect(props.x).toEqual({scale: 'x', field: 'mean_Acceleration'});
       expect(props.x2).toEqual({scale: 'x', value: 0});
       expect(props.width).toBeUndefined();
@@ -192,7 +192,7 @@ describe('Mark: Rect', () => {
         });
         const props = rect.encodeEntry(model);
         expect(props.y).toEqual({scale: 'y', field: 'Origin'});
-        expect(props.height).toEqual({scale: 'y', band: 1});
+        expect(props.height).toEqual({signal: `max(0.25, bandwidth('y'))`});
         expect(props.x).toEqual({scale: 'x', field: 'mean_Acceleration'});
         expect(props.x2).toEqual({scale: 'x', value: 0});
         expect(props.width).toBeUndefined();
@@ -275,9 +275,9 @@ describe('Mark: Rect', () => {
 
     it('should draw rect with x and y bands', () => {
       expect(props.x).toEqual({scale: 'x', field: 'Cylinders'});
-      expect(props.width).toEqual({scale: 'x', band: 1});
+      expect(props.width).toEqual({signal: `max(0.25, bandwidth('x'))`});
       expect(props.y).toEqual({scale: 'y', field: 'Origin'});
-      expect(props.height).toEqual({scale: 'y', band: 1});
+      expect(props.height).toEqual({signal: `max(0.25, bandwidth('y'))`});
     });
   });