Skip to content

Commit

Permalink
fix(#8350): fix thin bar when there are too many categories
Browse files Browse the repository at this point in the history
  • Loading branch information
@kanitw
kanitw committed Aug 9, 2022
1 parent 448d8b3 commit 4cd5f2c
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 24 deletions.
11 changes: 9 additions & 2 deletions src/compile/mark/encode/position-rect.ts
View file
Expand Up @@ -77,7 +77,12 @@ 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}`;
}
// TODO(#8351): make 0.25 here configurable
return {signal: `max(0.25, ${bandWidth})`};
} else if (bandSize.band !== 1) {
log.warn(log.message.cannotUseRelativeBandSizeWithNonBandScale(scaleType));
bandSize = undefined;
Expand Down Expand Up @@ -139,6 +144,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});
Expand All @@ -156,7 +162,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';
Expand Down
28 changes: 11 additions & 17 deletions test/compile/mark/bar.test.ts
View file
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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'))`});
});
});

Expand All @@ -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'))`});
});
});

Expand Down Expand Up @@ -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'))`});
});
});

Expand All @@ -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'))`});
});
});

Expand Down Expand Up @@ -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'))`});
});
});

Expand Down
10 changes: 5 additions & 5 deletions test/compile/mark/rect.test.ts
View file
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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'))`});
});
});

Expand Down

0 comments on commit 4cd5f2c

Please sign in to comment.