Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
wjgogogo marked this conversation as resolved.
Show resolved Hide resolved
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 (
wjgogogo marked this conversation as resolved.
Show resolved Hide resolved
mergedOptions.style?.layoutWidthType === LayoutWidthType.Compact &&
mergedOptions.style?.dataCell!.maxLines! <= 1
) {
mergedOptions.style.dataCell!.wordWrap = false;
}

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