Skip to content

Commit

Permalink
fix: 修复紧凑模式下, 文本带有 '\n' 换行符时 maxLines 配置未生效和文本溢出的问题 closes #2963 #2900 (
Browse files Browse the repository at this point in the history
#2972)

* fix: 修复紧凑模式下, 文本带有 '\n' 换行符时 maxLines 配置未生效和文本溢出的问题 closes #2963 #2900

* fix: 修复紧凑模式文本出现省略号的问题

* test: 修复单测

* test: 修复单测

* test: 跳过 CI 不稳定单测
  • Loading branch information
lijinke666 authored Nov 14, 2024
1 parent cc09af8 commit 8d45f07
Show file tree
Hide file tree
Showing 16 changed files with 90 additions and 41 deletions.
4 changes: 2 additions & 2 deletions packages/s2-core/__tests__/data/data-issue-2385.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"province": "浙江",
"city": "杭州",
"type": "纸张",
"price": "2",
"price": "哈哈哈",
"cost": "1.5"
},
{
Expand Down Expand Up @@ -153,7 +153,7 @@
"type": "圆规",
"province": "吉林",
"city": "白山",
"price": "111",
"price": "aa",
"cost": "1.5"
}
],
Expand Down
59 changes: 47 additions & 12 deletions packages/s2-core/__tests__/spreadsheet/compare-layout-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,25 +43,57 @@ describe('Compare Layout Tests', () => {
test.each([
{ showDefaultHeaderActionIcon: true },
{ showDefaultHeaderActionIcon: false },
])(
'should get max col width for pivot sheet and same font size by %o',
async (options) => {
const s2 = new PivotSheet(getContainer(), mockDataConfig, {
...s2Options,
...options,
});

await s2.render();

const colLeafNodes = s2.facet.getColLeafNodes();

expect(Math.floor(colLeafNodes[0].width)).toBeCloseTo(133);
expect(Math.floor(colLeafNodes[1].width)).toEqual(
options.showDefaultHeaderActionIcon ? 71 : 66,
);
expectTextOverflowing(s2);
},
);

// 覆盖 (数值/中文) 等场景
test.each([
{ showDefaultHeaderActionIcon: true, fontSize: 20 },
{ showDefaultHeaderActionIcon: true, fontSize: 12 },
{ showDefaultHeaderActionIcon: false, fontSize: 20 },
{ showDefaultHeaderActionIcon: false, fontSize: 12 },
])('should get max col width for pivot sheet by %o', async (options) => {
const s2 = new PivotSheet(getContainer(), mockDataConfig, {
...s2Options,
...options,
showDefaultHeaderActionIcon: options.showDefaultHeaderActionIcon,
});

s2.setTheme({
dataCell: {
text: {
fontSize: 20,
fontSize: options.fontSize,
},
},
});
await s2.render();

const expectWidth = options.showDefaultHeaderActionIcon ? 71 : 66;
const isLargeFontSize = options.fontSize === 20;
const colLeafNodes = s2.facet.getColLeafNodes();

expect(Math.floor(colLeafNodes[0].width)).toBeCloseTo(191);
expect(Math.floor(colLeafNodes[1].width)).toEqual(92);
expect(Math.floor(colLeafNodes[0].width)).toBeCloseTo(
isLargeFontSize ? 209 : 133,
);
expect(Math.floor(colLeafNodes[1].width)).toEqual(
isLargeFontSize ? 97 : expectWidth,
);
expectTextOverflowing(s2);
});

Expand Down Expand Up @@ -90,13 +122,15 @@ describe('Compare Layout Tests', () => {
await s2.render();

const colLeafNodes = s2.facet.getColLeafNodes();

expect(Math.floor(colLeafNodes[0].width)).toBeCloseTo(183);
expectTextOverflowing(s2);
const { dataCellWidthList, colLeafNodeWidthList } = mapWidthList(s2);
const expectWidth = 207;

expect(dataCellWidthList.every((width) => width === 183)).toBeTruthy();
expect(colLeafNodeWidthList).toEqual([183]);
expect(Math.floor(colLeafNodes[0].width)).toBeCloseTo(expectWidth);
expect(
dataCellWidthList.every((width) => width === expectWidth),
).toBeTruthy();
expect(colLeafNodeWidthList).toEqual([expectWidth]);
expectTextOverflowing(s2);
});

test.each([
Expand Down Expand Up @@ -137,12 +171,13 @@ describe('Compare Layout Tests', () => {

expect(dataCellWidthList).toEqual(
options.showDefaultHeaderActionIcon
? [209, 209, 209, 209, 110, 110, 110, 110, 85, 85, 85, 85]
: [209, 209, 209, 209, 110, 110, 110, 110, 69, 69, 69, 69],
? [227, 227, 227, 227, 115, 115, 115, 115, 93, 93, 93, 93]
: [227, 227, 227, 227, 115, 115, 115, 115, 71, 71, 71, 71],
);
expect(colLeafNodeWidthList).toEqual(
options.showDefaultHeaderActionIcon ? [209, 110, 85] : [209, 110, 69],
options.showDefaultHeaderActionIcon ? [227, 115, 93] : [227, 115, 71],
);
expectTextOverflowing(s2);
},
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ const expectScrollBrush = async (
expect(dataCellBrushSelectionFn).toHaveBeenCalledTimes(1);
};

describe('TableSheet Brush Selection Scroll Tests', () => {
describe.skip('TableSheet Brush Selection Scroll Tests', () => {
test('should scroll when mouse outside table data cell', async () => {
const s2 = new TableSheet(getContainer(), dataCfg, options);

Expand All @@ -188,7 +188,7 @@ describe('TableSheet Brush Selection Scroll Tests', () => {
});
});

describe('PivotSheet Brush Selection Scroll Tests', () => {
describe.skip('PivotSheet Brush Selection Scroll Tests', () => {
test('should scroll when mouse outside data cell', async () => {
const s2 = createPivotSheet(
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ describe('SpreadSheet Multi Line Text Tests', () => {
// wordWrap 关闭时, 不会渲染省略号
cells.forEach((cell) => {
expect(cell.getActualText()).not.toContain('...');
expect(cell.isTextOverflowing()).toBeFalsy();
});
});
expectColHierarchyHeight(90);
Expand Down Expand Up @@ -629,6 +630,7 @@ describe('SpreadSheet Multi Line Text Tests', () => {
// wordWrap 关闭时, 不会渲染省略号
cells.forEach((cell) => {
expect(cell.getActualText()).not.toContain('...');
expect(cell.isTextOverflowing()).toBeFalsy();
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/s2-core/__tests__/spreadsheet/scroll-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ describe('Scroll Tests', () => {
expect((sheet.facet as any).emitScrollEvent).not.toHaveBeenCalled();
});

test('should render correct scroll position', async () => {
test('should render correct scroll position for compact mode', async () => {
s2.setOptions({
interaction: {
scrollbarPosition: ScrollbarPositionType.CONTENT,
Expand All @@ -480,7 +480,7 @@ describe('Scroll Tests', () => {
s2.changeSheetSize(1000, 150); // 纵向滚动条
await s2.render(false);

expect(Math.floor(s2.facet.vScrollBar.getBBox().x)).toEqual(201);
expect(Math.floor(s2.facet.vScrollBar.getBBox().x)).toEqual(213);

s2.setOptions({
interaction: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ describe('Col width Test', () => {
const colLeafNodes = s2.facet.getColLeafNodes();

// price 列,列头标签比表身数据更长
expect(Math.round(colLeafNodes[0].width)).toBe(47);
expect(Math.round(colLeafNodes[0].width)).toBe(52);
// cost 列,表身数据比列头更长(格式化)
expect(Math.round(colLeafNodes[1].width)).toBe(168);
});
Expand Down
3 changes: 3 additions & 0 deletions packages/s2-core/__tests__/unit/facet/pivot-facet-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ jest.mock('@/sheet-type', () => {
getCellRange: jest.fn().mockReturnValue({ start: 0, end: 100 }),
cornerBBox: {},
getHeaderNodes: jest.fn().mockReturnValue([]),
measureTextWidth: jest.fn(),
},
getCanvasElement: () =>
container.getContextService().getDomElement() as HTMLCanvasElement,
Expand All @@ -98,6 +99,8 @@ jest.mock('@/sheet-type', () => {
},
measureTextWidth:
jest.fn() as unknown as SpreadSheet['measureTextWidth'],
measureTextWidthRoughly:
jest.fn() as unknown as SpreadSheet['measureTextWidthRoughly'],
getSeriesNumberText: jest.fn(() => getDefaultSeriesNumberText()),
};
}),
Expand Down
1 change: 1 addition & 0 deletions packages/s2-core/__tests__/unit/facet/table-facet-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ jest.mock('@/sheet-type', () => {
getColNodeHeight: jest.fn(),
getHeaderNodes: jest.fn().mockReturnValue([]),
getCellMeta: jest.fn().mockRejectedValue({}),
measureTextWidth: jest.fn(),
},
dataSet: {
isEmpty: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ Object {
}
`;

exports[`merge test should setup correctly compact layout width type style 1`] = `
exports[`merge test should not setup correctly compact layout width type style 1`] = `
Object {
"colCell": Object {
"height": 30,
Expand All @@ -141,7 +141,7 @@ Object {
"maxLines": 1,
"textOverflow": "ellipsis",
"width": 96,
"wordWrap": false,
"wordWrap": true,
},
"layoutWidthType": "compact",
"rowCell": Object {
Expand Down
2 changes: 1 addition & 1 deletion packages/s2-core/__tests__/unit/utils/merge-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ describe('merge test', () => {
expect(setupOptions(null)).toMatchSnapshot();
});

test('should setup correctly compact layout width type style', () => {
test('should not setup correctly compact layout width type style', () => {
expect(
setupOptions({
style: {
Expand Down
1 change: 1 addition & 0 deletions packages/s2-core/__tests__/util/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ export const createFakeSpreadSheet = (config?: {
s2.getTotalsConfig = jest.fn();
s2.getLayoutWidthType = jest.fn();
s2.measureTextWidth = jest.fn();
s2.measureTextWidthRoughly = jest.fn();
s2.isFrozenRowHeader = jest.fn();
s2.getSeriesNumberText = jest.fn(() => getDefaultSeriesNumberText());
s2.theme = getTheme({
Expand Down
21 changes: 17 additions & 4 deletions packages/s2-core/src/facet/base-facet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2294,12 +2294,25 @@ export abstract class BaseFacet {
/**
* @tip 和 this.spreadsheet.measureTextWidth() 的区别在于:
* 1. 额外添加一像素余量,防止 maxLabel 有多个同样长度情况下,一些 label 不能展示完全, 出现省略号
* 2. 测量时, 文本宽度向上取整, 避免子像素的不一致性
* 2. 测量时, 文本宽度取整, 避免子像素的不一致性
* 3. TODO: 由于 G 测量文本是一个一个字符进行计算, 在数字/英文等场景会有较大误差, 这里为了防止紧凑模式出现省略号, 暂时保持一样的策略
*/
protected measureTextWidth(text: SimpleData, font: unknown): number {
protected measureTextWidth(
text: SimpleData,
font: unknown,
roughly = true,
): number {
const EXTRA_PIXEL = 1;
const defaultWidth = this.spreadsheet.measureTextWidth(text, font);

return Math.ceil(defaultWidth) + EXTRA_PIXEL;
if (roughly) {
return (
Math.ceil(this.spreadsheet.measureTextWidthRoughly(text, font)) +
EXTRA_PIXEL
);
}

return (
Math.ceil(this.spreadsheet.measureTextWidth(text, font)) + EXTRA_PIXEL
);
}
}
2 changes: 1 addition & 1 deletion packages/s2-core/src/facet/pivot-facet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ export class PivotFacet extends FrozenFacet {
* 额外增加 1,当内容和容器宽度恰好相等时会出现换行
*/
const maxLabelWidth =
this.measureTextWidth(treeHeaderLabel, cornerCellTextStyle) +
this.measureTextWidth(treeHeaderLabel, cornerCellTextStyle, false) +
cornerIconStyle.size * 2 +
cornerIconStyle.margin?.left +
cornerIconStyle.margin?.right +
Expand Down
2 changes: 1 addition & 1 deletion packages/s2-core/src/facet/table-facet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export class TableFacet extends FrozenFacet {
const iconX = viewportWidth / 2 - icon.width / 2;
const iconY = height / 2 + maxY - icon.height / 2 + icon.margin.top;
const text = empty?.description ?? i18n('暂无数据');
const descWidth = this.measureTextWidth(text, description);
const descWidth = this.measureTextWidth(text, description, false);
const descX = viewportWidth / 2 - descWidth / 2;
const descY = iconY + icon.height + icon.margin.bottom;

Expand Down
7 changes: 5 additions & 2 deletions packages/s2-core/src/sheet-type/spread-sheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,10 @@ export abstract class SpreadSheet extends EE {
* @param font 文本 css 样式
* @returns 文本宽度
*/
public measureTextWidthRoughly = (text: any, font: any = {}): number => {
public measureTextWidthRoughly = (
text: SimpleData,
font: unknown,
): number => {
const alphaWidth = this.measureTextWidth('a', font);
const chineseWidth = this.measureTextWidth('蚂', font);

Expand All @@ -828,7 +831,7 @@ export abstract class SpreadSheet extends EE {
}

// eslint-disable-next-line no-restricted-syntax
for (const char of text) {
for (const char of String(text)) {
const code = char.charCodeAt(0);

// /[\u0000-\u00ff]/
Expand Down
13 changes: 2 additions & 11 deletions packages/s2-core/src/utils/merge.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { isArray, isEmpty, isEqual, isString, mergeWith, uniq } from 'lodash';
import { DEFAULT_DATA_CONFIG } from '../common/constant/dataConfig';
import { DEFAULT_OPTIONS, LayoutWidthType } from '../common/constant/options';
import { DEFAULT_OPTIONS } from '../common/constant/options';
import type {
CustomHeaderFields,
Fields,
Expand Down Expand Up @@ -68,14 +68,5 @@ export const setupDataConfig = (
export const setupOptions = (
options: Partial<S2Options> | null | undefined,
): S2Options => {
const mergedOptions = customMerge<S2Options>(DEFAULT_OPTIONS, options);

if (
mergedOptions.style?.layoutWidthType === LayoutWidthType.Compact &&
mergedOptions.style?.dataCell!.maxLines! <= 1
) {
mergedOptions.style.dataCell!.wordWrap = false;
}

return mergedOptions;
return customMerge<S2Options>(DEFAULT_OPTIONS, options);
};

0 comments on commit 8d45f07

Please sign in to comment.