Skip to content

Commit

Permalink
fix(sheet): reference sheet name
Browse files Browse the repository at this point in the history
  • Loading branch information
Dushusir committed Dec 3, 2024
1 parent 1d34bc1 commit 22e637a
Show file tree
Hide file tree
Showing 13 changed files with 335 additions and 31 deletions.
80 changes: 80 additions & 0 deletions e2e/visual-comparison/sheets/sheets-visual-comparison.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,83 @@ test('diff facade sheet hooks', async () => {
await page.waitForTimeout(2000);
await browser.close();
});

test('diff set force string cell', async () => {
const browser = await chromium.launch({
headless: !!isCI, // Set to false to see the browser window
});
const context = await browser.newContext({
viewport: { width: 1280, height: 1280 },
deviceScaleFactor: 2, // Set your desired DPR
});
const page = await context.newPage();
await page.goto('http://localhost:3000/sheets/');
await page.waitForTimeout(2000);

await page.evaluate(async () => {
const activeWorkbook = window.univerAPI.getActiveWorkbook();
const activeSheet = activeWorkbook.getActiveSheet();

const sheetId = activeSheet.getSheetId();
const unitId = activeWorkbook.getId();

await window.univerAPI.executeCommand('sheet.operation.set-selections', {
selections: [
{
range: {
startRow: 0,
startColumn: 7,
endRow: 0,
endColumn: 7,
rangeType: 0,
unitId,
sheetId,
},
primary: {
actualRow: 0,
actualColumn: 7,
isMerged: false,
isMergedMainCell: false,
startRow: 0,
startColumn: 7,
endRow: 0,
endColumn: 7,
},
style: {
strokeWidth: 1,
stroke: '#274fee',
fill: 'rgba(39,79,238,0.07)',
widgets: {},
widgetSize: 6,
widgetStrokeWidth: 1,
widgetStroke: '#ffffff',
autofillSize: 6,
autofillStrokeWidth: 1,
autofillStroke: '#ffffff',
rowHeaderFill: 'rgba(39,79,238,0.07)',
rowHeaderStroke: '#274fee',
rowHeaderStrokeWidth: 1,
columnHeaderFill: 'rgba(39,79,238,0.07)',
columnHeaderStroke: '#274fee',
columnHeaderStrokeWidth: 1,
expandCornerSize: 40,
},
},
],
unitId,
subUnitId: sheetId,
type: 2,
});

activeWorkbook.startEditing();
await window.univerAPI.getActiveDocument().appendText("'1");
activeWorkbook.endEditing(true);
});

const filename = generateSnapshotName('set-force-string-cell');
const screenshot = await page.locator(SHEET_MAIN_CANVAS_ID).screenshot();
await expect(screenshot).toMatchSnapshot(filename, { maxDiffPixels: 5 });

await page.waitForTimeout(2000);
await browser.close();
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ describe('Test Reference', () => {
'12a',
'💩a',
'❤️b',
"Sheet'",
"Sheet'1",
'!Sheet',
'!Sheet',
'Sheet1(副本)',
Expand Down Expand Up @@ -357,11 +357,40 @@ describe('Test Reference', () => {
unitId: '',
});

// with single quote
expect(handleRefStringInfo("'sheet''1'!A1")).toStrictEqual({
refBody: 'A1',
sheetName: "sheet'1",
unitId: '',
});

// with double quote
expect(handleRefStringInfo("'sheet''''1'!A1")).toStrictEqual({
refBody: 'A1',
sheetName: "sheet''1",
unitId: '',
});

expect(handleRefStringInfo("'[Book-1.xlsx]Sheet1'!$A$4")).toStrictEqual({
refBody: '$A$4',
sheetName: 'Sheet1',
unitId: 'Book-1.xlsx',
});

// with single quote
expect(handleRefStringInfo("'[Book''1.xlsx]Sheet1'!$A$4")).toStrictEqual({
refBody: '$A$4',
sheetName: 'Sheet1',
unitId: "Book'1.xlsx",
});

// with double quote
expect(handleRefStringInfo("'[Book''''1.xlsx]Sheet1'!$A$4")).toStrictEqual({
refBody: '$A$4',
sheetName: 'Sheet1',
unitId: "Book''1.xlsx",
});

expect(handleRefStringInfo("'[Book-1.xlsx]sheet-1'!$A$4")).toStrictEqual({
refBody: '$A$4',
sheetName: 'sheet-1',
Expand Down
43 changes: 29 additions & 14 deletions packages/engine-formula/src/engine/utils/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,7 @@ export function serializeRange(range: IRange): string {
* @param range
*/
export function serializeRangeWithSheet(sheetName: string, range: IRange): string {
if (needsQuoting(sheetName)) {
return `'${quoteSheetName(sheetName)}'!${serializeRange(range)}`;
}
return `${sheetName}!${serializeRange(range)}`;
return `${addQuotesBothSides(sheetName)}!${serializeRange(range)}`;
}

/**
Expand Down Expand Up @@ -206,7 +203,7 @@ export function handleRefStringInfo(refString: string) {

if (unitIdMatch != null) {
unitId = unitIdMatch[0].trim();
unitId = unitId.slice(1, unitId.length - 1);
unitId = unquoteSheetName(unitId.slice(1, unitId.length - 1));
refString = refString.replace(UNIT_NAME_REGEX_PRECOMPILING, '');
}

Expand All @@ -218,6 +215,8 @@ export function handleRefStringInfo(refString: string) {
if (sheetName[0] === "'" && sheetName[sheetName.length - 1] === "'") {
sheetName = sheetName.substring(1, sheetName.length - 1);
}

sheetName = unquoteSheetName(sheetName);
refBody = refString.substring(sheetNameIndex + 1);
} else {
refBody = refString;
Expand Down Expand Up @@ -426,6 +425,31 @@ export function needsQuoting(name: string) {
return false;
}

/**
* Add quotes to the sheet name
*/
export function addQuotesBothSides(name: string) {
return needsQuoting(name) ? `'${quoteSheetName(name)}'` : name;
}

/**
* Add a single quote before the single quote
* @param name
* @returns Quoted name
*/
function quoteSheetName(name: string) {
return name.replace(/'/g, "''");
}

/**
* Replace double single quotes with single quotes
* @param name
* @returns Unquoted name
*/
function unquoteSheetName(name: string) {
return name.replace(/''/g, "'");
}

function isA1Notation(name: string) {
const match = name.match(/[1-9][0-9]{0,6}/);
// Excel has a limit on the number of rows and columns: targetRow > 1048576 || targetColumn > 16384, Univer has no limit
Expand All @@ -440,12 +464,3 @@ function startsWithNonAlphabetic(name: string) {
// Check if the first character is not a letter (including non-English characters)
return !/^\p{Letter}/u.test(name.charAt(0));
}

/**
* Add a single quote before the single quote
* @param name
* @returns
*/
export function quoteSheetName(name: string) {
return name.replace(/'/g, "''");
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@
import { describe, expect, it } from 'vitest';

import { ErrorType } from '../../../../basics/error-type';
import { ArrayValueObject, transformToValue, transformToValueObject } from '../../../../engine/value-object/array-value-object';
import { ErrorValueObject } from '../../../../engine/value-object/base-value-object';
import {
BooleanValueObject,
NumberValueObject,
StringValueObject,
} from '../../../../engine/value-object/primitive-object';
import { getObjectValue } from '../../../__tests__/create-function-test-bed';
import { FUNCTION_NAMES_LOOKUP } from '../../function-names';
import { Address } from '../index';
import { ArrayValueObject, transformToValue, transformToValueObject } from '../../../../engine/value-object/array-value-object';
import { getObjectValue } from '../../../__tests__/create-function-test-bed';

describe('Test address', () => {
const testFunction = new Address(FUNCTION_NAMES_LOOKUP.ADDRESS);
Expand Down Expand Up @@ -65,6 +65,10 @@ describe('Test address', () => {
expect(calculate(2, 3, 1, false, '[Book1]Sheet1')).toBe("'[Book1]Sheet1'!R2C3");
});

it('Absolute reference to sheet name with single quote', async () => {
expect(calculate(1, 1, 1, true, "Sheet'1")).toBe("'Sheet''1'!$A$1");
});

it('Absolute reference to another worksheet', async () => {
expect(calculate(2, 3, 1, false, 'EXCEL SHEET')).toBe("'EXCEL SHEET'!R2C3");
});
Expand Down
10 changes: 5 additions & 5 deletions packages/engine-formula/src/functions/lookup/address/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@
* limitations under the License.
*/

import { AbsoluteRefType, type IRange } from '@univerjs/core';
import type { ArrayValueObject } from '../../../engine/value-object/array-value-object';

import { AbsoluteRefType, type IRange } from '@univerjs/core';
import { ErrorType } from '../../../basics/error-type';
import { expandArrayValueObject } from '../../../engine/utils/array-object';
import { serializeRangeToR1C1 } from '../../../engine/utils/r1c1-reference';
import { needsQuoting, serializeRange } from '../../../engine/utils/reference';
import { addQuotesBothSides, serializeRange } from '../../../engine/utils/reference';
import { type BaseValueObject, ErrorValueObject } from '../../../engine/value-object/base-value-object';
import { BooleanValueObject, NumberValueObject, StringValueObject } from '../../../engine/value-object/primitive-object';
import { BaseFunction } from '../../base-function';
import type { ArrayValueObject } from '../../../engine/value-object/array-value-object';
import { expandArrayValueObject } from '../../../engine/utils/array-object';

export class Address extends BaseFunction {
override minParams = 2;
Expand Down Expand Up @@ -132,7 +132,7 @@ export class Address extends BaseFunction {
const absType = transformAbsoluteRefType(absNumberValue);
const a1Value = this.getZeroOrOneByOneDefault(a1);
const sheetTextValue = `${sheetText.getValue()}`;
const sheetName = needsQuoting(sheetTextValue) ? `'${sheetTextValue}'` : sheetTextValue;
const sheetName = addQuotesBothSides(sheetTextValue);

const range: IRange = {
startRow: row,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/**
* Copyright 2023-present DreamNum Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import type { Workbook } from '@univerjs/core';
import type { IRenderContext, IRenderModule } from '@univerjs/engine-render';
import { Disposable, Inject, isRealNum, LocaleService } from '@univerjs/core';
import { FormulaDataModel } from '@univerjs/engine-formula';
import { DEFAULT_TEXT_FORMAT } from '@univerjs/engine-numfmt';
import { INumfmtService } from '@univerjs/sheets';
import { CellAlertManagerService, CellAlertType, HoverManagerService } from '@univerjs/sheets-ui';
import { IZenZoneService } from '@univerjs/ui';
import { debounceTime } from 'rxjs';

const ALERT_KEY = 'SHEET_NUMFMT_ALERT';

export class NumfmtAlertRenderController extends Disposable implements IRenderModule {
constructor(
private readonly _context: IRenderContext<Workbook>,
@Inject(HoverManagerService) private readonly _hoverManagerService: HoverManagerService,
@Inject(CellAlertManagerService) private readonly _cellAlertManagerService: CellAlertManagerService,
@Inject(LocaleService) private readonly _localeService: LocaleService,
@Inject(FormulaDataModel) private readonly _formulaDataModel: FormulaDataModel,
@IZenZoneService private readonly _zenZoneService: IZenZoneService,
@Inject(INumfmtService) private _numfmtService: INumfmtService
) {
super();
this._init();
}

private _init() {
this._initCellAlertPopup();
this._initZenService();
}

private _initCellAlertPopup() {
this.disposeWithMe(this._hoverManagerService.currentCell$.pipe(debounceTime(100)).subscribe((cellPos) => {
if (cellPos) {
const location = cellPos.location;
const workbook = this._context.unit;
const worksheet = workbook.getActiveSheet();
if (!worksheet) return;

const unitId = location.unitId;
const sheetId = location.subUnitId;
let numfmtValue;

const cellData = worksheet.getCell(location.row, location.col);

if (cellData?.s) {
const style = workbook.getStyles().get(cellData.s);
if (style?.n) {
numfmtValue = style.n;
}
}

if (!numfmtValue) {
numfmtValue = this._numfmtService.getValue(unitId, sheetId, location.row, location.col);
}

if (!numfmtValue) {
this._hideAlert();
return;
}

// Preventing blank object
if (numfmtValue.pattern === DEFAULT_TEXT_FORMAT && cellData?.v && isRealNum(cellData.v)) {
const currentAlert = this._cellAlertManagerService.currentAlert.get(ALERT_KEY);
const currentLoc = currentAlert?.alert?.location;
if (
currentLoc &&
currentLoc.row === location.row &&
currentLoc.col === location.col &&
currentLoc.subUnitId === location.subUnitId &&
currentLoc.unitId === location.unitId
) {
return;
}

this._cellAlertManagerService.showAlert({
type: CellAlertType.ERROR,
title: this._localeService.t('info.error'),
message: this._localeService.t('info.forceStringInfo'),
location,
width: 200,
height: 74,
key: ALERT_KEY,
});
return;
}
}

this._hideAlert();
}));
}

private _initZenService() {
this.disposeWithMe(this._zenZoneService.visible$.subscribe((visible) => {
if (visible) {
this._hideAlert();
}
}));
}

private _hideAlert() {
this._cellAlertManagerService.removeAlert(ALERT_KEY);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ export class NumfmtEditorController extends Disposable {
return next(value);
}

if (currentNumfmtValue?.pattern === DEFAULT_TEXT_FORMAT) {
// if the cell is text format or force string, do not convert the value
if (currentNumfmtValue?.pattern === DEFAULT_TEXT_FORMAT || value.t === CellValueType.FORCE_STRING) {
return next(value);
}

Expand Down
Loading

0 comments on commit 22e637a

Please sign in to comment.