Skip to content

Commit 05565e2

Browse files
committed
[FIX] composer: set cursor on prettified formula
Steps to reproduce: - type a long formula, e.g. =SUM(11111111, 22222222, 33333333, 44444444, 55555555, 66666666, 77777777, 88888888) - in the top bar composer, click somewhere that won't be on the first line when prettified (in the fours) => the cursor is badly positioned on the prettified formula Task: 5095470
1 parent f443c9a commit 05565e2

File tree

8 files changed

+99
-33
lines changed

8 files changed

+99
-33
lines changed

src/components/composer/composer/abstract_composer_store.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ export abstract class AbstractComposerStore extends SpreadsheetStore {
103103
});
104104
}
105105
protected abstract confirmEdition(content: string): void;
106-
protected abstract getComposerContent(position: CellPosition): string;
106+
protected abstract getComposerContent(
107+
position: CellPosition,
108+
selection?: ComposerSelection
109+
): { content: string; adjustedSelection?: ComposerSelection };
107110

108111
abstract stopEdition(direction?: Direction): void;
109112

@@ -150,12 +153,8 @@ export abstract class AbstractComposerStore extends SpreadsheetStore {
150153
}
151154

152155
startEdition(text?: string, selection?: ComposerSelection) {
153-
if (selection) {
154-
const content = text || this.getComposerContent(this.getters.getActivePosition());
155-
const validSelection = this.isSelectionValid(content.length, selection.start, selection.end);
156-
if (!validSelection) {
157-
return;
158-
}
156+
if (selection && text && !this.isSelectionValid(text.length, selection.start, selection.end)) {
157+
return;
159158
}
160159
const { col, row } = this.getters.getActivePosition();
161160
this.model.dispatch("SELECT_FIGURE", { figureId: null });
@@ -220,7 +219,7 @@ export abstract class AbstractComposerStore extends SpreadsheetStore {
220219

221220
get currentContent(): string {
222221
if (this.editionMode === "inactive") {
223-
return this.getComposerContent(this.getters.getActivePosition());
222+
return this.getComposerContent(this.getters.getActivePosition()).content;
224223
}
225224
return this._currentContent;
226225
}
@@ -447,8 +446,12 @@ export abstract class AbstractComposerStore extends SpreadsheetStore {
447446
this.sheetId = sheetId;
448447
this.row = row;
449448
this.editionMode = "editing";
450-
this.initialContent = this.getComposerContent({ sheetId, col, row });
451-
this.setContent(str || this.initialContent, selection);
449+
const { content, adjustedSelection } = this.getComposerContent(
450+
{ sheetId, col, row },
451+
selection
452+
);
453+
this.initialContent = content;
454+
this.setContent(str || this.initialContent, adjustedSelection ?? selection);
452455
this.colorIndexByRange = {};
453456
const zone = positionToZone({ col: this.col, row: this.row });
454457
this.captureSelection(zone, col, row);

src/components/composer/composer/cell_composer_store.ts

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
RemoveColumnsRowsCommand,
3030
isMatrix,
3131
} from "../../../types";
32-
import { AbstractComposerStore } from "./abstract_composer_store";
32+
import { AbstractComposerStore, ComposerSelection } from "./abstract_composer_store";
3333

3434
const CELL_DELETED_MESSAGE = _t("The cell you are trying to edit has been deleted.");
3535

@@ -201,39 +201,66 @@ export class CellComposerStore extends AbstractComposerStore {
201201
this.setContent("");
202202
}
203203

204-
protected getComposerContent(position: CellPosition): string {
204+
protected getComposerContent(
205+
position: CellPosition,
206+
selection?: ComposerSelection
207+
): { content: string; adjustedSelection?: ComposerSelection } {
205208
const locale = this.getters.getLocale();
206209
const cell = this.getters.getCell(position);
207210
if (cell?.isFormula) {
208211
const prettifiedContent = this.getPrettifiedFormula(cell);
209-
return localizeFormula(prettifiedContent, locale);
212+
// when a formula is prettified (multi lines, indented), adapt the cursor position
213+
// to take into account line breaks and tabs
214+
function adjustCursorIndex(targetIndex: number): number {
215+
let adjustedIndex = 0;
216+
let originalIndex = 0;
217+
while (originalIndex < targetIndex) {
218+
adjustedIndex++;
219+
const char = prettifiedContent[adjustedIndex];
220+
if (char !== "\n" && char !== "\t") {
221+
originalIndex++;
222+
}
223+
}
224+
return adjustedIndex;
225+
}
226+
let adjustedSelection = selection;
227+
if (selection) {
228+
adjustedSelection = {
229+
start: adjustCursorIndex(selection.start),
230+
end: adjustCursorIndex(selection.end),
231+
};
232+
}
233+
return {
234+
content: localizeFormula(prettifiedContent, locale),
235+
adjustedSelection,
236+
};
210237
}
211238
const spreader = this.model.getters.getArrayFormulaSpreadingOn(position);
212239
if (spreader) {
213-
return "";
240+
return { content: "" };
214241
}
215242
const { format, value, type, formattedValue } = this.getters.getEvaluatedCell(position);
216243
switch (type) {
217244
case CellValueType.empty:
218-
return "";
245+
return { content: "" };
219246
case CellValueType.text:
220247
case CellValueType.error:
221-
return value;
248+
return { content: value };
222249
case CellValueType.boolean:
223-
return formattedValue;
250+
return { content: formattedValue };
224251
case CellValueType.number:
225252
if (format && isDateTimeFormat(format)) {
226253
if (parseDateTime(formattedValue, locale) !== null) {
227254
// formatted string can be parsed again
228-
return formattedValue;
255+
return { content: formattedValue };
229256
}
230257
// display a simplified and parsable string otherwise
231258
const timeFormat = Number.isInteger(value)
232259
? locale.dateFormat
233260
: getDateTimeFormat(locale);
234-
return formatValue(value, { locale, format: timeFormat });
261+
return { content: formatValue(value, { locale, format: timeFormat }) };
235262
}
236-
return this.numberComposerContent(value, format, locale);
263+
return { content: this.numberComposerContent(value, format, locale) };
237264
}
238265
}
239266

src/components/composer/composer/composer.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { TextValueProvider } from "../autocomplete_dropdown/autocomplete_dropdow
2626
import { ContentEditableHelper } from "../content_editable_helper";
2727
import { FunctionDescriptionProvider } from "../formula_assistant/formula_assistant";
2828
import { SpeechBubble } from "../speech_bubble/speech_bubble";
29+
import { ComposerSelection } from "./abstract_composer_store";
2930
import { CellComposerStore } from "./cell_composer_store";
3031

3132
const functions = functionRegistry.content;
@@ -131,8 +132,8 @@ export interface CellComposerProps {
131132
inputStyle?: string;
132133
rect?: Rect;
133134
delimitation?: DOMDimension;
134-
onComposerContentFocused: () => void;
135-
onComposerCellFocused?: (content: String) => void;
135+
onComposerContentFocused: (selection: ComposerSelection) => void;
136+
onComposerCellFocused?: (content: string) => void;
136137
onInputContextMenu?: (event: MouseEvent) => void;
137138
isDefaultFocus?: boolean;
138139
composerStore: Store<CellComposerStore>;
@@ -600,11 +601,13 @@ export class Composer extends Component<CellComposerProps, SpreadsheetChildEnv>
600601
return;
601602
}
602603
const newSelection = this.contentHelper.getCurrentSelection();
603-
604604
this.props.composerStore.stopComposerRangeSelection();
605-
this.props.onComposerContentFocused();
605+
const isCurrentlyInactive = this.props.composerStore.editionMode === "inactive";
606+
this.props.onComposerContentFocused(newSelection);
607+
if (!isCurrentlyInactive) {
608+
this.props.composerStore.changeComposerCursorSelection(newSelection.start, newSelection.end);
609+
}
606610

607-
this.props.composerStore.changeComposerCursorSelection(newSelection.start, newSelection.end);
608611
this.processTokenAtCursor();
609612
}
610613

src/components/composer/grid_composer/grid_composer.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,10 @@ export class GridComposer extends Component<Props, SpreadsheetChildEnv> {
142142
},
143143
focus: this.focus,
144144
isDefaultFocus: true,
145-
onComposerContentFocused: () =>
145+
onComposerContentFocused: (selection) =>
146146
this.composerFocusStore.focusComposer(this.composerInterface, {
147147
focusMode: "contentFocus",
148+
selection,
148149
}),
149150
onComposerCellFocused: (content: string) =>
150151
this.composerFocusStore.focusComposer(this.composerInterface, {

src/components/composer/standalone_composer/standalone_composer_store.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export interface StandaloneComposerArgs {
2323
export class StandaloneComposerStore extends AbstractComposerStore {
2424
constructor(get: Get, private args: () => StandaloneComposerArgs) {
2525
super(get);
26-
this._currentContent = this.getComposerContent();
26+
this._currentContent = this.getComposerContent().content;
2727
}
2828

2929
protected getAutoCompleteProviders(): AutoCompleteProviderDefinition[] {
@@ -46,7 +46,7 @@ export class StandaloneComposerStore extends AbstractComposerStore {
4646
return res;
4747
}
4848

49-
protected getComposerContent(): string {
49+
protected getComposerContent() {
5050
let content = this._currentContent;
5151
if (this.editionMode === "inactive") {
5252
// References in the content might not be linked to the current active sheet
@@ -63,8 +63,9 @@ export class StandaloneComposerStore extends AbstractComposerStore {
6363
})
6464
.join("");
6565
}
66-
67-
return localizeContent(content, this.getters.getLocale());
66+
return {
67+
content: localizeContent(content, this.getters.getLocale()),
68+
};
6869
}
6970

7071
stopEdition() {

src/components/small_bottom_bar/small_bottom_bar.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,10 @@ export class SmallBottomBar extends Component<Props, SpreadsheetChildEnv> {
8888
},
8989
focus: this.focus,
9090
composerStore: this.composerStore,
91-
onComposerContentFocused: () =>
91+
onComposerContentFocused: (selection) =>
9292
this.composerFocusStore.focusComposer(this.composerInterface, {
9393
focusMode: "contentFocus",
94+
selection,
9495
}),
9596
isDefaultFocus: false,
9697
inputStyle: cssPropertiesToCss({

tests/test_helpers/helpers.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,9 +1071,9 @@ export class ComposerWrapper extends Component<ComposerWrapperProps, Spreadsheet
10711071
get composerProps(): CellComposerProps {
10721072
return {
10731073
...this.props.composerProps,
1074-
onComposerContentFocused: () => {
1074+
onComposerContentFocused: (selection) => {
10751075
this.state.focusComposer = "contentFocus";
1076-
this.setEdition({});
1076+
this.setEdition({ selection });
10771077
},
10781078
focus: this.state.focusComposer,
10791079
composerStore: this.composerStore,

tests/top_bar_component.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@ import {
2828
click,
2929
doubleClick,
3030
getElComputedStyle,
31+
getTextNodes,
3132
simulateClick,
3233
triggerMouseEvent,
3334
} from "./test_helpers/dom_helper";
3435
import { getBorder, getCell, getStyle, getTable } from "./test_helpers/getters_helpers";
3536
import {
3637
addToRegistry,
3738
getFigureIds,
39+
getInputSelection,
3840
getNode,
3941
mountComponent,
4042
mountSpreadsheet,
@@ -981,6 +983,34 @@ test("The composer helper should be closed on toggle topbar context menu", async
981983
expect(fixture.querySelectorAll(".o-composer-assistant")).toHaveLength(0);
982984
});
983985

986+
test("prettified formula should have the cursor", async () => {
987+
const { model } = await mountSpreadsheet();
988+
989+
// I'll set the cursor in between the fours
990+
const firstPart = "=SUM(11111111, 22222222, 33333333, 4444";
991+
const secondPart = "4444, 55555555, 66666666, 77777777, 88888888)";
992+
setCellContent(model, "A1", firstPart + secondPart);
993+
await nextTick();
994+
const composerEl: HTMLElement = document.querySelector(".o-spreadsheet-topbar .o-composer")!;
995+
composerEl.focus();
996+
triggerMouseEvent(composerEl, "pointerdown");
997+
const selection = document.getSelection()!;
998+
const range = document.createRange();
999+
selection.removeAllRanges();
1000+
selection.addRange(range);
1001+
const textNode = getTextNodes(composerEl)[0];
1002+
range.setStart(textNode, firstPart.length);
1003+
range.setEnd(textNode, firstPart.length);
1004+
1005+
await nextTick();
1006+
triggerMouseEvent(composerEl, "pointerup");
1007+
await nextTick();
1008+
triggerMouseEvent(composerEl, "click");
1009+
await nextTick();
1010+
expect(getInputSelection().anchorNodeText).toBe("44444444");
1011+
expect(getInputSelection().anchorOffset).toBe(4);
1012+
});
1013+
9841014
test("The menu items are orderer by their sequence", async () => {
9851015
addToRegistry(topbarMenuRegistry, "test", {
9861016
sequence: 1,

0 commit comments

Comments
 (0)