Skip to content

Commit

Permalink
fix: 修复紧凑模式下单元格宽度计算忽略了icon宽度的问题 (#2673)
Browse files Browse the repository at this point in the history
* fix: 修复存在字段标记时, 紧凑模式宽度计算错误

* fix: 修复紧凑模式下单元格宽度计算忽略了icon宽度的问题

* fix: 修改类型引用路径

* test: 补充utils单测

---------

Co-authored-by: lijinke666 <[email protected]>
Co-authored-by: 张斌 <[email protected]>
  • Loading branch information
3 people authored Apr 25, 2024
1 parent 52dcea4 commit dd053c3
Show file tree
Hide file tree
Showing 10 changed files with 352 additions and 66 deletions.
106 changes: 95 additions & 11 deletions packages/s2-core/__tests__/bugs/issue-2385-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
* @description spec for issue #2385
* https://github.com/antvis/S2/issues/2385
*/
import { createPivotSheet, getContainer } from '../util/helpers';
import type { S2Options, SpreadSheet } from '../../src';
import * as mockDataConfig from '../data/data-issue-2385.json';
import type { S2Options } from '../../src';
import { getContainer } from '../util/helpers';
import { PivotSheet, TableSheet } from '@/sheet-type';

const s2Options: S2Options = {
Expand All @@ -16,11 +16,32 @@ const s2Options: S2Options = {
},
layoutWidthType: 'compact',
},
showDefaultHeaderActionIcon: false,
};

describe('Compare Layout Tests', () => {
test('should get max col width for pivot sheet', () => {
const s2 = new PivotSheet(getContainer(), mockDataConfig, s2Options);
const mapWidthList = (s2: SpreadSheet) => {
const colLeafNodeWidthList = s2.facet.layoutResult.colLeafNodes.map(
(node) => Math.floor(node.width),
);
const dataCellWidthList = s2.interaction
.getPanelGroupAllDataCells()
.map((cell) => Math.floor(cell.getMeta().width));

return {
colLeafNodeWidthList,
dataCellWidthList,
};
};

test.each([
{ showDefaultHeaderActionIcon: true },
{ showDefaultHeaderActionIcon: false },
])('should get max col width for pivot sheet by %o', (options) => {
const s2 = new PivotSheet(getContainer(), mockDataConfig, {
...s2Options,
...options,
});
s2.setTheme({
dataCell: {
text: {
Expand All @@ -30,13 +51,26 @@ describe('Compare Layout Tests', () => {
});
s2.render();

const colLeafNodes = s2.facet.layoutResult.colLeafNodes;
expect(Math.floor(colLeafNodes[0].width)).toBeCloseTo(179);
expect(Math.floor(colLeafNodes[1].width)).toEqual(98);
const { dataCellWidthList, colLeafNodeWidthList } = mapWidthList(s2);

expect(dataCellWidthList).toEqual(
options.showDefaultHeaderActionIcon
? [179, 179, 179, 179, 98, 98, 98, 98, 81, 81, 81, 81]
: [179, 179, 179, 179, 98, 98, 98, 98, 63, 63, 63, 63],
);
expect(colLeafNodeWidthList).toEqual(
options.showDefaultHeaderActionIcon ? [179, 98, 81] : [179, 98, 63],
);
});

test('should get max col width for table sheet', () => {
const s2 = new TableSheet(getContainer(), mockDataConfig, s2Options);
test.each([
{ showDefaultHeaderActionIcon: true },
{ showDefaultHeaderActionIcon: false },
])('should get max col width for table sheet by %o', (options) => {
const s2 = new TableSheet(getContainer(), mockDataConfig, {
...s2Options,
...options,
});
s2.setDataCfg({
fields: {
columns: ['price'],
Expand All @@ -51,7 +85,57 @@ describe('Compare Layout Tests', () => {
});
s2.render();

const colLeafNodes = s2.facet.layoutResult.colLeafNodes;
expect(Math.floor(colLeafNodes[0].width)).toBeCloseTo(165);
const { dataCellWidthList, colLeafNodeWidthList } = mapWidthList(s2);

expect(dataCellWidthList.every((width) => width === 165)).toBeTruthy();
expect(colLeafNodeWidthList).toEqual(
options.showDefaultHeaderActionIcon ? [165] : [165],
);
});

test.each([
{ showDefaultHeaderActionIcon: true },
{ showDefaultHeaderActionIcon: false },
])(
'should get max col width for pivot sheet by condition and %o',
(options) => {
const s2 = new PivotSheet(getContainer(), mockDataConfig, {
...s2Options,
...options,
conditions: {
icon: [
{
field: 'price',
position: 'left',
mapping: () => {
return {
icon: 'Plus',
fill: '#396',
};
},
},
],
},
});
s2.setTheme({
dataCell: {
text: {
fontSize: 20,
},
},
});
s2.render();

const { dataCellWidthList, colLeafNodeWidthList } = mapWidthList(s2);

expect(dataCellWidthList).toEqual(
options.showDefaultHeaderActionIcon
? [197, 197, 197, 197, 116, 116, 116, 116, 81, 81, 81, 81]
: [197, 197, 197, 197, 116, 116, 116, 116, 62, 62, 62, 62],
);
expect(colLeafNodeWidthList).toEqual(
options.showDefaultHeaderActionIcon ? [197, 116, 81] : [197, 116, 62],
);
},
);
});
28 changes: 28 additions & 0 deletions packages/s2-core/__tests__/data/data-issue-2385.json
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,34 @@
"province": "吉林",
"price": "3",
"cost": "1.5"
},
{
"type": "圆规",
"province": "浙江",
"city": "杭州",
"price": "111",
"cost": "1.5"
},
{
"type": "圆规",
"province": "浙江",
"city": "舟山",
"price": "111",
"cost": "1.5"
},
{
"type": "圆规",
"province": "吉林",
"city": "长春",
"price": "111",
"cost": "1.5"
},
{
"type": "圆规",
"province": "吉林",
"city": "白山",
"price": "111",
"cost": "1.5"
}
],
"fields": {
Expand Down
32 changes: 32 additions & 0 deletions packages/s2-core/__tests__/unit/utils/condition/condition-spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
getIconPositionCfg,
getIntervalScale,
findFieldCondition,
} from '@/utils/condition/condition';

describe('getIconLayoutPosition Test', () => {
Expand All @@ -24,6 +25,37 @@ describe('getIconLayoutPosition Test', () => {
});
});

describe('getFieldCondition Test', () => {
test('should find the condition where fill is green', () => {
const conditions = [
{
field: 'value',
mapping: () => ({ fill: 'red' }),
},
{ field: 'price', mapping: () => ({ fill: 'blue' }) },
{ field: 'price', mapping: () => ({ fill: 'green' }) },
];
expect(findFieldCondition(conditions, 'price').mapping().fill).toBe(
'green',
);
});

test('should not find the condition where fill is orange', () => {
const conditions = [
{
field: 'value',
mapping: () => ({ fill: 'red' }),
},
{ field: 'price', mapping: () => ({ fill: 'blue' }) },
{ field: /price/, mapping: () => ({ fill: 'orange' }) },
{ field: 'p', mapping: () => ({ fill: 'pink' }) },
];
expect(findFieldCondition(conditions, 'price').mapping().fill).toBe(
'orange',
);
});
});

describe('getIntervalScale Test', () => {
test('should get scale when both of minValue and maxValue are greater then 0', () => {
const getScale = getIntervalScale(100, 200);
Expand Down
84 changes: 84 additions & 0 deletions packages/s2-core/__tests__/unit/utils/layout/icon-spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { getDataCellIconStyle, normalizeIconCfg } from '@/utils/layout/icon';

describe('normalizeIconCfg Test', () => {
test('should return a complete IconCfg object', () => {
expect(normalizeIconCfg()).toEqual({
size: 0,
position: 'right',
margin: {
left: 0,
right: 0,
},
});
});

test('should return the input object', () => {
const iconCfg = {
size: 10,
position: 'left',
margin: {
left: 8,
right: 8,
},
};
expect(normalizeIconCfg(iconCfg)).toEqual(iconCfg);
});
});

describe('getDataCellIconStyle Test', () => {
const conditions = [
{
field: 'value',
mapping: () => ({ fill: 'red' }),
},
{ field: 'price', mapping: () => ({ fill: 'blue' }) },
{ field: /price/, mapping: () => ({ fill: 'orange' }), position: 'left' },
{ field: 'p', mapping: () => ({ fill: 'pink' }) },
];
test('should return default iconCfg object', () => {
expect(
getDataCellIconStyle(
{ icon: conditions },
{ size: 10, margin: { left: 4, right: 4 } },
'errorField',
),
).toEqual({
size: 0,
position: 'right',
margin: {
left: 0,
right: 0,
},
});
expect(
getDataCellIconStyle(
{ icon: [] },
{ size: 10, margin: { left: 4, right: 4 } },
'price',
),
).toEqual({
size: 0,
position: 'right',
margin: {
left: 0,
right: 0,
},
});
});
test('should return correct iconCfg object', () => {
expect(
getDataCellIconStyle(
{ icon: conditions },
{ size: 10, margin: { left: 4, right: 4 } },
'price',
),
).toEqual({
size: 10,
position: 'left',
margin: {
left: 4,
right: 4,
},
});
});
});
33 changes: 14 additions & 19 deletions packages/s2-core/src/cell/data-cell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import type {
Condition,
FormatResult,
IconCfg,
IconCondition,
MappingResult,
TextTheme,
ViewMeta,
Expand All @@ -27,7 +26,10 @@ import {
shouldUpdateBySelectedCellsHighlight,
updateBySelectedCellsHighlight,
} from '../utils/cell/data-cell';
import { getIconPositionCfg } from '../utils/condition/condition';
import {
getIconPositionCfg,
findFieldCondition,
} from '../utils/condition/condition';
import { renderLine, renderRect, updateShapeAttr } from '../utils/g-renders';
import { EMPTY_PLACEHOLDER } from '../common/constant/basic';
import { drawInterval } from '../utils/g-mini-charts';
Expand All @@ -36,6 +38,8 @@ import {
REVERSE_FONT_COLOR,
} from '../common/constant/condition';
import { shouldReverseFontColor } from '../utils/color';
import { LayoutWidthTypes } from '../common/constant/options';
import { getDataCellIconStyle } from '../utils/layout';

/**
* DataCell for panelGroup area
Expand Down Expand Up @@ -244,18 +248,11 @@ export class DataCell extends BaseCell<ViewMeta> {
}

public getIconStyle(): IconCfg | undefined {
const { size, margin } = this.theme.dataCell.icon;
const iconCondition: IconCondition = this.findFieldCondition(
this.conditions?.icon,
return getDataCellIconStyle(
this.conditions,
this.theme.dataCell.icon,
this.meta.valueField,
);

const iconCfg: IconCfg = iconCondition &&
iconCondition.mapping && {
size,
margin,
position: getIconPositionCfg(iconCondition),
};
return iconCfg;
}

protected drawConditionIntervalShape() {
Expand Down Expand Up @@ -312,7 +309,9 @@ export class DataCell extends BaseCell<ViewMeta> {

protected getMaxTextWidth(): number {
const { width } = this.getContentArea();
return getMaxTextWidth(width, this.getIconStyle());
const iconCfg = this.getIconStyle();

return getMaxTextWidth(width, iconCfg);
}

protected getTextPosition(): Point {
Expand Down Expand Up @@ -455,11 +454,7 @@ export class DataCell extends BaseCell<ViewMeta> {
* @param conditions
*/
public findFieldCondition(conditions: Condition[]): Condition {
return findLast(conditions, (item) => {
return item.field instanceof RegExp
? item.field.test(this.meta.valueField)
: item.field === this.meta.valueField;
});
return findFieldCondition(conditions, this.meta.valueField);
}

/**
Expand Down
Loading

0 comments on commit dd053c3

Please sign in to comment.