From dd053c3c54ef084255f07a97f5dfef42e0b50d79 Mon Sep 17 00:00:00 2001 From: xxxxZ-zhang <48509562+xxxxZ-zhang@users.noreply.github.com> Date: Thu, 25 Apr 2024 10:10:31 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8D=E7=B4=A7=E5=87=91?= =?UTF-8?q?=E6=A8=A1=E5=BC=8F=E4=B8=8B=E5=8D=95=E5=85=83=E6=A0=BC=E5=AE=BD?= =?UTF-8?q?=E5=BA=A6=E8=AE=A1=E7=AE=97=E5=BF=BD=E7=95=A5=E4=BA=86icon?= =?UTF-8?q?=E5=AE=BD=E5=BA=A6=E7=9A=84=E9=97=AE=E9=A2=98=20(#2673)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 修复存在字段标记时, 紧凑模式宽度计算错误 * fix: 修复紧凑模式下单元格宽度计算忽略了icon宽度的问题 * fix: 修改类型引用路径 * test: 补充utils单测 --------- Co-authored-by: lijinke666 Co-authored-by: 张斌 --- .../s2-core/__tests__/bugs/issue-2385-spec.ts | 106 ++++++++++++++++-- .../__tests__/data/data-issue-2385.json | 28 +++++ .../unit/utils/condition/condition-spec.ts | 32 ++++++ .../__tests__/unit/utils/layout/icon-spec.ts | 84 ++++++++++++++ packages/s2-core/src/cell/data-cell.ts | 33 +++--- packages/s2-core/src/facet/pivot-facet.ts | 61 ++++++---- packages/s2-core/src/utils/cell/cell.ts | 16 +-- .../s2-core/src/utils/condition/condition.ts | 15 ++- packages/s2-core/src/utils/layout/icon.ts | 42 +++++++ packages/s2-core/src/utils/layout/index.ts | 1 + 10 files changed, 352 insertions(+), 66 deletions(-) create mode 100644 packages/s2-core/__tests__/unit/utils/layout/icon-spec.ts create mode 100644 packages/s2-core/src/utils/layout/icon.ts diff --git a/packages/s2-core/__tests__/bugs/issue-2385-spec.ts b/packages/s2-core/__tests__/bugs/issue-2385-spec.ts index 4e3e56430a..6ace741deb 100644 --- a/packages/s2-core/__tests__/bugs/issue-2385-spec.ts +++ b/packages/s2-core/__tests__/bugs/issue-2385-spec.ts @@ -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 = { @@ -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: { @@ -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'], @@ -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], + ); + }, + ); }); diff --git a/packages/s2-core/__tests__/data/data-issue-2385.json b/packages/s2-core/__tests__/data/data-issue-2385.json index d6f20caac5..e77dd0305c 100644 --- a/packages/s2-core/__tests__/data/data-issue-2385.json +++ b/packages/s2-core/__tests__/data/data-issue-2385.json @@ -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": { diff --git a/packages/s2-core/__tests__/unit/utils/condition/condition-spec.ts b/packages/s2-core/__tests__/unit/utils/condition/condition-spec.ts index 5ea40a1219..3ee03cf8f3 100644 --- a/packages/s2-core/__tests__/unit/utils/condition/condition-spec.ts +++ b/packages/s2-core/__tests__/unit/utils/condition/condition-spec.ts @@ -1,6 +1,7 @@ import { getIconPositionCfg, getIntervalScale, + findFieldCondition, } from '@/utils/condition/condition'; describe('getIconLayoutPosition Test', () => { @@ -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); diff --git a/packages/s2-core/__tests__/unit/utils/layout/icon-spec.ts b/packages/s2-core/__tests__/unit/utils/layout/icon-spec.ts new file mode 100644 index 0000000000..26f6609ae4 --- /dev/null +++ b/packages/s2-core/__tests__/unit/utils/layout/icon-spec.ts @@ -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, + }, + }); + }); +}); diff --git a/packages/s2-core/src/cell/data-cell.ts b/packages/s2-core/src/cell/data-cell.ts index ece1b92141..bc470f2c1a 100644 --- a/packages/s2-core/src/cell/data-cell.ts +++ b/packages/s2-core/src/cell/data-cell.ts @@ -15,7 +15,6 @@ import type { Condition, FormatResult, IconCfg, - IconCondition, MappingResult, TextTheme, ViewMeta, @@ -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'; @@ -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 @@ -244,18 +248,11 @@ export class DataCell extends BaseCell { } 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() { @@ -312,7 +309,9 @@ export class DataCell extends BaseCell { protected getMaxTextWidth(): number { const { width } = this.getContentArea(); - return getMaxTextWidth(width, this.getIconStyle()); + const iconCfg = this.getIconStyle(); + + return getMaxTextWidth(width, iconCfg); } protected getTextPosition(): Point { @@ -455,11 +454,7 @@ export class DataCell extends BaseCell { * @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); } /** diff --git a/packages/s2-core/src/facet/pivot-facet.ts b/packages/s2-core/src/facet/pivot-facet.ts index 4580829e72..d5cafda295 100644 --- a/packages/s2-core/src/facet/pivot-facet.ts +++ b/packages/s2-core/src/facet/pivot-facet.ts @@ -40,7 +40,7 @@ import { getIndexRangeWithOffsets } from '../utils/facet'; import { getCellWidth, safeJsonParse } from '../utils/text'; import { getHeaderTotalStatus } from '../utils/dataset/pivot-data-set'; import { getRowsForGrid } from '../utils/grid'; -import { renderLine } from '..'; +import { getDataCellIconStyle, renderLine } from '..'; import { FrozenFacet } from './frozen-facet'; import { buildHeaderHierarchy } from './layout/build-header-hierarchy'; import type { Hierarchy } from './layout/hierarchy'; @@ -199,18 +199,7 @@ export class PivotFacet extends FrozenFacet { row.isTotalMeasure || col.isTotals || col.isTotalMeasure; - const { hierarchyType } = spreadsheet.options; - const hideMeasure = - get(spreadsheet, 'facet.cfg.colCfg.hideMeasureColumn') ?? false; - // 如果在非自定义目录情况下hide measure query中是没有度量信息的，所以需要自动补上 - // 存在一个场景的冲突，如果是多个度量，定位数据数据是无法知道哪一列代表什么 - // 因此默认只会去 第一个度量拼接query - const measureInfo = - hideMeasure && hierarchyType !== 'customTree' - ? { - [EXTRA_FIELD]: dataSet.fields.values?.[0], - } - : {}; + const measureInfo = this.getMeasureInfo(); const dataQuery = merge({}, rowQuery, colQuery, measureInfo); const totalStatus = getHeaderTotalStatus(row, col); const data = dataSet.getCellData({ @@ -257,6 +246,22 @@ export class PivotFacet extends FrozenFacet { return layoutDataPosition(this.cfg, layoutResult); } + protected getMeasureInfo() { + const { dataSet, spreadsheet } = this.cfg; + const { hierarchyType } = spreadsheet.options; + const hideMeasure = + get(spreadsheet, 'facet.cfg.colCfg.hideMeasureColumn') ?? false; + + // 如果在非自定义目录情况下hide measure query中是没有度量信息的，所以需要自动补上 + // 存在一个场景的冲突，如果是多个度量，定位数据数据是无法知道哪一列代表什么 + // 因此默认只会去 第一个度量拼接query + return hideMeasure && hierarchyType !== 'customTree' + ? { + [EXTRA_FIELD]: dataSet.fields.values?.[0], + } + : {}; + } + private calculateNodesCoordinate( rowLeafNodes: Node[], rowsHierarchy: Hierarchy, @@ -410,7 +415,7 @@ export class PivotFacet extends FrozenFacet { col.field, ); const leafNodeLabel = cellFormatter?.(col.value) ?? col.label; - const iconWidth = this.getExpectedCellIconWidth( + const colIconWidth = this.getExpectedCellIconWidth( CellTypes.COL_CELL, this.spreadsheet.isValueInCols() && this.spreadsheet.options.showDefaultHeaderActionIcon, @@ -420,11 +425,14 @@ export class PivotFacet extends FrozenFacet { this.spreadsheet.measureTextWidthRoughly( leafNodeLabel, colCellTextStyle, - ) + iconWidth; + ) + colIconWidth; + + const measureInfo = this.getMeasureInfo(); // 采样 50 个 label，逐个计算找出最长的 label let maxDataLabel: string; let maxDataLabelWidth = 0; + let iconWidthOfMaxDataLabel = 0; for (let index = 0; index < LAYOUT_SAMPLE_COUNT; index++) { const rowNode = rowLeafNodes[index]; if (rowNode) { @@ -447,14 +455,27 @@ export class PivotFacet extends FrozenFacet { cellData[EXTRA_FIELD], )?.(valueData) ?? valueData; const cellLabel = `${formattedValue}`; - const cellLabelWidth = this.spreadsheet.measureTextWidthRoughly( - cellLabel, - dataCellTextStyle, + const dataQuery = merge({}, rowNode.query, col.query, measureInfo); + const valueField = dataQuery[EXTRA_FIELD]; + const { + size, + margin: { left, right }, + } = getDataCellIconStyle( + this.spreadsheet.options.conditions, + this.spreadsheet.theme.dataCell.icon, + valueField, ); + const dataCellIconWidth = size + left + right; + const cellLabelWidth = + this.spreadsheet.measureTextWidthRoughly( + cellLabel, + dataCellTextStyle, + ) + dataCellIconWidth; if (cellLabelWidth > maxDataLabelWidth) { maxDataLabel = cellLabel; maxDataLabelWidth = cellLabelWidth; + iconWidthOfMaxDataLabel = dataCellIconWidth; } } } @@ -462,7 +483,9 @@ export class PivotFacet extends FrozenFacet { const isLeafNodeWidthLonger = leafNodeRoughWidth > maxDataLabelWidth; const maxLabel = isLeafNodeWidthLonger ? leafNodeLabel : maxDataLabel; - const appendedWidth = isLeafNodeWidthLonger ? iconWidth : 0; + const appendedWidth = isLeafNodeWidthLonger + ? colIconWidth + : iconWidthOfMaxDataLabel; DebuggerUtil.getInstance().logger( 'Max Label In Col:', diff --git a/packages/s2-core/src/utils/cell/cell.ts b/packages/s2-core/src/utils/cell/cell.ts index 42db386cca..dae062377c 100644 --- a/packages/s2-core/src/utils/cell/cell.ts +++ b/packages/s2-core/src/utils/cell/cell.ts @@ -1,5 +1,4 @@ import type { SimpleBBox } from '@antv/g-canvas'; -import { merge } from 'lodash'; import type { AreaRange, CellTheme, @@ -10,6 +9,7 @@ import type { TextBaseline, } from '../../common/interface'; import { CellBorderPosition } from '../../common/interface'; +import { normalizeIconCfg } from '../../utils/layout'; /** * ----------------------------- @@ -43,20 +43,6 @@ export const getContentArea = (bbox: SimpleBBox, padding: Padding) => { * 2. 其他的情况，需要根据实际 text width 确定 icon bbox 开始位置 */ -const normalizeIconCfg = (iconCfg?: IconCfg): IconCfg => { - return merge( - { - size: 0, - position: 'right', - margin: { - left: 0, - right: 0, - }, - }, - iconCfg, - ); -}; - export const getMaxTextWidth = (contentWidth: number, iconCfg?: IconCfg) => { iconCfg = normalizeIconCfg(iconCfg); return ( diff --git a/packages/s2-core/src/utils/condition/condition.ts b/packages/s2-core/src/utils/condition/condition.ts index ffb5c25fb4..b080d39624 100644 --- a/packages/s2-core/src/utils/condition/condition.ts +++ b/packages/s2-core/src/utils/condition/condition.ts @@ -1,5 +1,5 @@ -import { clamp } from 'lodash'; -import type { IconCondition } from '../../common/interface'; +import { clamp, findLast } from 'lodash'; +import type { Condition, IconCondition } from '../../common/interface'; import { parseNumberWithPrecision } from '../formatter'; export const getIconPositionCfg = (condition: IconCondition) => { @@ -52,3 +52,14 @@ export const getIntervalScale = (minValue = 0, maxValue = 0) => { return { zeroScale, scale }; }; }; + +export const findFieldCondition = ( + conditions: Condition[], + valueField: string, +) => { + return findLast(conditions, (item) => { + return item.field instanceof RegExp + ? item.field.test(valueField) + : item.field === valueField; + }); +}; diff --git a/packages/s2-core/src/utils/layout/icon.ts b/packages/s2-core/src/utils/layout/icon.ts new file mode 100644 index 0000000000..cd7e512006 --- /dev/null +++ b/packages/s2-core/src/utils/layout/icon.ts @@ -0,0 +1,42 @@ +import { merge } from 'lodash'; +import { findFieldCondition, getIconPositionCfg } from '../condition/condition'; +import type { + Conditions, + IconCfg, + IconCondition, + IconTheme, +} from '../../common'; + +export const normalizeIconCfg = (iconCfg?: IconCfg): IconCfg => { + return merge( + { + size: 0, + position: 'right', + margin: { + left: 0, + right: 0, + }, + }, + iconCfg, + ); +}; + +export const getDataCellIconStyle = ( + conditions: Conditions, + IconTheme: IconTheme, + valueField: string, +) => { + const { size, margin } = IconTheme; + const iconCondition: IconCondition = findFieldCondition( + conditions?.icon, + valueField, + ); + + const iconCfg: IconCfg = iconCondition && + iconCondition.mapping && { + size, + margin, + position: getIconPositionCfg(iconCondition), + }; + return normalizeIconCfg(iconCfg); +}; diff --git a/packages/s2-core/src/utils/layout/index.ts b/packages/s2-core/src/utils/layout/index.ts index 3eedfac60c..a6808bd25a 100644 --- a/packages/s2-core/src/utils/layout/index.ts +++ b/packages/s2-core/src/utils/layout/index.ts @@ -1,2 +1,3 @@ export { generateId } from './generate-id'; export * from './frozen'; +export * from './icon';