From d0a8860cb88bd148bbea3910d08548ba1ec5fc33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Rahir?= Date: Thu, 22 Dec 2022 14:05:51 +0100 Subject: [PATCH] [FIX] Composer: Persistent composition when starting the edition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When inputing a character in the grid, we capture the character and use it to start the edition, which opens a new composer and focus it. This change of focus closes the Input Method Editor if present. This typically affect people from Asia writing in their native language (chinese, japanese, etc) This revision addresses the issue by replacing the hidden input in the grid with the `GridComposer`. The later will therefore always be in the DOM, capture the characters with the IME and then change its position based on the edition mode. closes odoo/o-spreadsheet#3456 Task: 3685891 Signed-off-by: Lucas Lefèvre (lul) --- src/components/composer/composer/composer.ts | 93 ++++++++++----- src/components/composer/composer/composer.xml | 2 +- .../composer/grid_composer/grid_composer.ts | 107 ++++++++++-------- .../composer/grid_composer/grid_composer.xml | 9 +- src/components/grid/grid.ts | 30 ++--- src/components/grid/grid.xml | 27 ++--- src/components/spreadsheet/spreadsheet.ts | 9 +- src/helpers/focus_manager.ts | 11 ++ src/plugins/ui/edition.ts | 2 +- src/types/env.ts | 2 + .../__snapshots__/grid.test.ts.snap | 28 ++++- .../__snapshots__/spreadsheet.test.ts.snap | 28 ++++- tests/components/composer.test.ts | 57 +++++----- tests/components/figure.test.ts | 4 +- tests/components/formula_assistant.test.ts | 2 +- tests/components/grid.test.ts | 9 +- tests/components/overlay.test.ts | 13 ++- tests/components/side_panel.test.ts | 2 +- tests/components/spreadsheet.test.ts | 27 ++--- tests/test_helpers/helpers.ts | 16 ++- 20 files changed, 278 insertions(+), 200 deletions(-) create mode 100644 src/helpers/focus_manager.ts diff --git a/src/components/composer/composer/composer.ts b/src/components/composer/composer/composer.ts index 6203c60e26..6cb88ee523 100644 --- a/src/components/composer/composer/composer.ts +++ b/src/components/composer/composer/composer.ts @@ -1,9 +1,9 @@ -import { Component, onMounted, onPatched, onWillUnmount, useRef, useState } from "@odoo/owl"; +import { Component, onMounted, onPatched, useRef, useState } from "@odoo/owl"; import { ComponentsImportance, DEFAULT_FONT, SELECTION_BORDER_COLOR } from "../../../constants"; import { EnrichedToken } from "../../../formulas/index"; import { functionRegistry } from "../../../functions/index"; import { isEqual, rangeReference, splitReference, zoneToDimension } from "../../../helpers/index"; -import { ComposerSelection, SelectionIndicator } from "../../../plugins/ui/edition"; +import { SelectionIndicator } from "../../../plugins/ui/edition"; import { Color, DOMDimension, @@ -93,13 +93,14 @@ css/* scss */ ` } `; -interface Props { +export interface ComposerProps { inputStyle: string; rect?: Rect; delimitation?: DOMDimension; focus: "inactive" | "cellFocus" | "contentFocus"; - onComposerUnmounted?: () => void; - onComposerContentFocused: (selection: ComposerSelection) => void; + onComposerContentFocused: () => void; + onComposerCellFocused?: (content: String) => void; + isDefaultFocus?: boolean; } interface ComposerState { @@ -120,12 +121,13 @@ interface FunctionDescriptionState { argToFocus: number; } -export class Composer extends Component { +export class Composer extends Component { static template = "o-spreadsheet-Composer"; static components = { TextValueProvider, FunctionDescriptionProvider }; static defaultProps = { inputStyle: "", focus: "inactive", + isDefaultFocus: false, }; composerRef = useRef("o_composer"); @@ -195,15 +197,13 @@ export class Composer extends Component { setup() { onMounted(() => { const el = this.composerRef.el!; - + if (this.props.isDefaultFocus) { + this.env.focusableElement.setFocusableElement(el); + } this.contentHelper.updateEl(el); this.processContent(); }); - onWillUnmount(() => { - this.props.onComposerUnmounted?.(); - }); - onPatched(() => { if (!this.isKeyStillDown) { this.processContent(); @@ -216,7 +216,10 @@ export class Composer extends Component { // --------------------------------------------------------------------------- private processArrowKeys(ev: KeyboardEvent) { - if (this.env.model.getters.isSelectingForComposer()) { + if ( + this.env.model.getters.isSelectingForComposer() || + this.env.model.getters.getEditionMode() === "inactive" + ) { this.functionDescriptionState.showDescription = false; // Prevent the default content editable behavior which moves the cursor // but don't stop the event and let it bubble to the grid which will @@ -253,21 +256,22 @@ export class Composer extends Component { private processTabKey(ev: KeyboardEvent) { ev.preventDefault(); ev.stopPropagation(); - if (this.autoCompleteState.showProvider && this.autocompleteAPI) { - const autoCompleteValue = this.autocompleteAPI.getValueToFill(); - if (autoCompleteValue) { - this.autoComplete(autoCompleteValue); - return; + if (this.env.model.getters.getEditionMode() !== "inactive") { + if (this.autoCompleteState.showProvider && this.autocompleteAPI) { + const autoCompleteValue = this.autocompleteAPI.getValueToFill(); + if (autoCompleteValue) { + this.autoComplete(autoCompleteValue); + return; + } + } else { + // when completing with tab, if there is no value to complete, the active cell will be moved to the right. + // we can't let the model think that it is for a ref selection. + // todo: check if this can be removed someday + this.env.model.dispatch("STOP_COMPOSER_RANGE_SELECTION"); } - } else { - // when completing with tab, if there is no value to complete, the active cell will be moved to the right. - // we can't let the model think that it is for a ref selection. - // todo: check if this can be removed someday - this.env.model.dispatch("STOP_COMPOSER_RANGE_SELECTION"); + this.env.model.dispatch("STOP_EDITION"); } - const direction = ev.shiftKey ? "left" : "right"; - this.env.model.dispatch("STOP_EDITION"); this.env.model.selection.moveAnchorCell(direction, 1); } @@ -304,6 +308,9 @@ export class Composer extends Component { } onKeydown(ev: KeyboardEvent) { + if (this.env.model.getters.getEditionMode() === "inactive") { + return; + } let handler = this.keyMapping[ev.key]; if (handler) { handler.call(this, ev); @@ -314,24 +321,50 @@ export class Composer extends Component { } private updateCursorIfNeeded() { - if (!this.env.model.getters.isSelectingForComposer()) { + const moveCursor = + !this.env.model.getters.isSelectingForComposer() && + !(this.env.model.getters.getEditionMode() === "inactive"); + if (moveCursor) { const { start, end } = this.contentHelper.getCurrentSelection(); this.env.model.dispatch("CHANGE_COMPOSER_CURSOR_SELECTION", { start, end }); this.isKeyStillDown = true; } } + onPaste(ev: ClipboardEvent) { + if (this.env.model.getters.getEditionMode() !== "inactive") { + ev.stopPropagation(); + } + } + /* * Triggered automatically by the content-editable between the keydown and key up * */ - onInput() { - if (this.props.focus === "inactive" || !this.shouldProcessInputEvents) { + onInput(ev: InputEvent) { + if (!this.shouldProcessInputEvents) { return; } + if ( + ev.inputType === "insertFromPaste" && + this.env.model.getters.getEditionMode() === "inactive" + ) { + return; + } + ev.stopPropagation(); + let content: string; + + if (this.env.model.getters.getEditionMode() === "inactive") { + content = ev.data || ""; + } else { + const el = this.composerRef.el! as HTMLInputElement; + content = el.childNodes.length ? el.textContent! : ""; + } + if (this.props.focus === "inactive") { + return this.props.onComposerCellFocused?.(content); + } this.env.model.dispatch("STOP_COMPOSER_RANGE_SELECTION"); - const el = this.composerRef.el! as HTMLInputElement; this.env.model.dispatch("SET_CURRENT_CONTENT", { - content: el.childNodes.length ? el.textContent! : "", + content, selection: this.contentHelper.getCurrentSelection(), }); } @@ -395,8 +428,8 @@ export class Composer extends Component { const newSelection = this.contentHelper.getCurrentSelection(); this.env.model.dispatch("STOP_COMPOSER_RANGE_SELECTION"); + this.props.onComposerContentFocused(); if (this.props.focus === "inactive") { - this.props.onComposerContentFocused(newSelection); } this.env.model.dispatch("CHANGE_COMPOSER_CURSOR_SELECTION", newSelection); this.processTokenAtCursor(); diff --git a/src/components/composer/composer/composer.xml b/src/components/composer/composer/composer.xml index 66234a2bb4..f90d273171 100644 --- a/src/components/composer/composer/composer.xml +++ b/src/components/composer/composer/composer.xml @@ -14,7 +14,7 @@ t-on-keyup="onKeyup" t-on-click="onClick" t-on-blur="onBlur" - t-on-paste.stop="" + t-on-paste="onPaste" t-on-compositionstart="onCompositionStart" t-on-compositionend="onCompositionEnd" /> diff --git a/src/components/composer/grid_composer/grid_composer.ts b/src/components/composer/grid_composer/grid_composer.ts index 852ad481d7..55991b3ec7 100644 --- a/src/components/composer/grid_composer/grid_composer.ts +++ b/src/components/composer/grid_composer/grid_composer.ts @@ -1,19 +1,17 @@ -import { Component, onMounted, useRef, useState } from "@odoo/owl"; +import { Component, onWillUpdateProps } from "@odoo/owl"; import { ComponentsImportance, DEFAULT_CELL_HEIGHT, SELECTION_BORDER_COLOR, } from "../../../constants"; import { fontSizeMap } from "../../../fonts"; -import { DOMDimension, Rect, Ref, SpreadsheetChildEnv, Zone } from "../../../types/index"; +import { positionToZone } from "../../../helpers"; +import { Rect, SpreadsheetChildEnv } from "../../../types/index"; import { getTextDecoration } from "../../helpers"; import { css } from "../../helpers/css"; -import { Composer } from "../composer/composer"; +import { Composer, ComposerProps } from "../composer/composer"; import { Style } from "./../../../types/misc"; -const SCROLLBAR_WIDTH = 14; -const SCROLLBAR_HIGHT = 15; - const COMPOSER_BORDER_WIDTH = 3 * 0.4 * window.devicePixelRatio || 1; css/* scss */ ` div.o-grid-composer { @@ -24,15 +22,11 @@ css/* scss */ ` } `; -interface ComposerState { - rect: Rect | null; - delimitation: DOMDimension | null; -} - interface Props { focus: "inactive" | "cellFocus" | "contentFocus"; content: string; - onComposerUnmounted: () => void; + onComposerContentFocused: () => void; + onComposerCellFocused: () => void; } /** @@ -43,53 +37,59 @@ export class GridComposer extends Component { static template = "o-spreadsheet-GridComposer"; static components = { Composer }; - private gridComposerRef!: Ref; - - private zone!: Zone; - private rect!: Rect; + private rect: Rect | undefined = undefined; + private isEditing: boolean = false; - private composerState!: ComposerState; + get defaultRect() { + return { x: 0, y: 0, width: 0, height: 0 }; + } setup() { - this.gridComposerRef = useRef("gridComposer"); - this.composerState = useState({ - rect: null, - delimitation: null, - }); - const { col, row } = this.env.model.getters.getPosition(); - this.zone = this.env.model.getters.expandZone(this.env.model.getters.getActiveSheetId(), { - left: col, - right: col, - top: row, - bottom: row, - }); - this.rect = this.env.model.getters.getVisibleRect(this.zone); - onMounted(() => { - const el = this.gridComposerRef.el!; - - //TODO Should be more correct to have a props that give the parent's clientHeight and clientWidth - const maxHeight = el.parentElement!.clientHeight - this.rect.y - SCROLLBAR_HIGHT; - el.style.maxHeight = (maxHeight + "px") as string; - - const maxWidth = el.parentElement!.clientWidth - this.rect.x - SCROLLBAR_WIDTH; - el.style.maxWidth = (maxWidth + "px") as string; - - this.composerState.rect = { - x: this.rect.x, - y: this.rect.y, - width: el!.clientWidth, - height: el!.clientHeight, - }; - this.composerState.delimitation = { - width: el!.parentElement!.clientWidth, - height: el!.parentElement!.clientHeight, - }; + onWillUpdateProps(() => { + const isEditing = this.env.model.getters.getEditionMode() !== "inactive"; + if (this.isEditing !== isEditing) { + this.isEditing = isEditing; + if (!isEditing) { + this.rect = undefined; + this.env.focusableElement.focus(); + return; + } + const position = this.env.model.getters.getPosition(); + const zone = this.env.model.getters.expandZone( + this.env.model.getters.getActiveSheetId(), + positionToZone(position) + ); + this.rect = this.env.model.getters.getVisibleRect(zone); + } }); } + get composerProps(): ComposerProps { + const { width, height } = this.env.model.getters.getSheetViewDimensionWithHeaders(); + return { + rect: this.rect && { ...this.rect }, + delimitation: { + width, + height, + }, + inputStyle: this.composerStyle, + focus: this.props.focus, + isDefaultFocus: true, + onComposerContentFocused: this.props.onComposerContentFocused, + onComposerCellFocused: this.props.onComposerCellFocused, + }; + } + get containerStyle(): string { + if (this.env.model.getters.getEditionMode() === "inactive" || !this.rect) { + return ` + position: absolute; + z-index: -1000; + `; + } const isFormula = this.env.model.getters.getCurrentContent().startsWith("="); const cell = this.env.model.getters.getActiveCell(); + let style: Style = {}; if (cell) { const cellPosition = this.env.model.getters.getCellPosition(cell.id); @@ -119,6 +119,10 @@ export class GridComposer extends Component { if (!isFormula) { textAlign = style.align || cell?.defaultAlign || "left"; } + const sheetDimensions = this.env.model.getters.getSheetViewDimensionWithHeaders(); + + const maxWidth = sheetDimensions.width - this.rect.x; + const maxHeight = sheetDimensions.height - this.rect.y; return ` left: ${left - 1}px; @@ -126,6 +130,9 @@ export class GridComposer extends Component { min-width: ${width + 2}px; min-height: ${height + 1}px; + max-width: ${maxWidth}px; + max-height: ${maxHeight}px; + background: ${background}; color: ${color}; diff --git a/src/components/composer/grid_composer/grid_composer.xml b/src/components/composer/grid_composer/grid_composer.xml index 2ebea1b6bd..c19a904704 100644 --- a/src/components/composer/grid_composer/grid_composer.xml +++ b/src/components/composer/grid_composer/grid_composer.xml @@ -1,14 +1,7 @@
- +
diff --git a/src/components/grid/grid.ts b/src/components/grid/grid.ts index 8470910a0f..7da8442774 100644 --- a/src/components/grid/grid.ts +++ b/src/components/grid/grid.ts @@ -89,7 +89,6 @@ export class Grid extends Component { readonly HEADER_WIDTH = HEADER_WIDTH; private menuState!: MenuState; private gridRef!: Ref; - private hiddenInput!: Ref; onMouseWheel!: (ev: WheelEvent) => void; canvasPosition!: DOMCoordinates; @@ -102,20 +101,19 @@ export class Grid extends Component { menuItems: [], }); this.gridRef = useRef("grid"); - this.hiddenInput = useRef("hiddenInput"); this.canvasPosition = useAbsolutePosition(this.gridRef); this.hoveredCell = useState({ col: undefined, row: undefined }); useExternalListener(document.body, "cut", this.copy.bind(this, true)); useExternalListener(document.body, "copy", this.copy.bind(this, false)); useExternalListener(document.body, "paste", this.paste); - onMounted(() => this.focus()); - this.props.exposeFocus(() => this.focus()); + onMounted(() => this.focusDefaultElement()); + this.props.exposeFocus(() => this.focusDefaultElement()); useGridDrawing("canvas", this.env.model, () => this.env.model.getters.getSheetViewDimensionWithHeaders() ); useEffect( - () => this.focus(), + () => this.focusDefaultElement(), () => [this.env.model.getters.getActiveSheetId()] ); this.onMouseWheel = useWheelHandler((deltaX, deltaY) => { @@ -141,7 +139,7 @@ export class Grid extends Component { onClosePopover() { this.closeOpenedPopover(); - this.focus(); + this.focusDefaultElement(); } // this map will handle most of the actions that should happen on key down. The arrow keys are managed in the key @@ -269,12 +267,12 @@ export class Grid extends Component { PAGEUP: () => this.env.model.dispatch("SHIFT_VIEWPORT_UP"), }; - focus() { + focusDefaultElement() { if ( !this.env.model.getters.getSelectedFigureId() && this.env.model.getters.getEditionMode() === "inactive" ) { - this.hiddenInput.el?.focus(); + this.env.focusableElement.focus(); } } @@ -438,20 +436,6 @@ export class Grid extends Component { } } - onInput(ev: InputEvent) { - // the user meant to paste in the sheet, not open the composer with the pasted content - if (!ev.isComposing && ev.inputType === "insertFromPaste") { - return; - } - if (ev.data) { - // if the user types a character on the grid, it means he wants to start composing the selected cell with that - // character - ev.preventDefault(); - ev.stopPropagation(); - this.props.onGridComposerCellFocused(ev.data); - } - } - // --------------------------------------------------------------------------- // Context Menu // --------------------------------------------------------------------------- @@ -534,6 +518,6 @@ export class Grid extends Component { closeMenu() { this.menuState.isOpen = false; - this.focus(); + this.focusDefaultElement(); } } diff --git a/src/components/grid/grid.xml b/src/components/grid/grid.xml index 60ca26aa8b..7a2388ed59 100644 --- a/src/components/grid/grid.xml +++ b/src/components/grid/grid.xml @@ -4,9 +4,10 @@ class="o-grid" tabindex="-1" t-att-class="{'o-two-columns': !props.sidePanelIsOpen}" - t-on-click="focus" + t-on-click="focusDefaultElement" t-on-keydown="onKeydown" t-on-wheel="onMouseWheel" + t-on-contextmenu="onInputContextMenu" t-ref="grid"> - - - - - - - + this.render(true)); @@ -299,19 +301,20 @@ export class Spreadsheet extends Component void; clipboard: Clipboard; _t: TranslationFunction; + focusableElement: FocusableElement; } diff --git a/tests/components/__snapshots__/grid.test.ts.snap b/tests/components/__snapshots__/grid.test.ts.snap index acea758efe..86afe57065 100644 --- a/tests/components/__snapshots__/grid.test.ts.snap +++ b/tests/components/__snapshots__/grid.test.ts.snap @@ -51,11 +51,31 @@ exports[`Grid component simple rendering snapshot 1`] = ` /> +
+
+
+ +
+
-
+
+
+
+ +
+
- { jest.useFakeTimers(); ({ model, fixture } = await mountSpreadsheet()); - gridInputEl = fixture.querySelector(".o-grid>input")!; + gridInputEl = fixture.querySelector(".o-grid div.o-composer")!; }); describe("ranges and highlights", () => { @@ -377,7 +377,7 @@ describe("ranges and highlights", () => { await nextTick(); expect(composerEl.textContent).toBe("=C1"); - composerEl = await typeInComposerGrid("+B2"); + composerEl = await typeInComposerGrid("+B2", false); model.dispatch("START_CHANGE_HIGHLIGHT", { range: model.getters.getRangeDataFromXc(model.getters.getActiveSheetId(), "B1:B2"), }); @@ -429,7 +429,15 @@ describe("ranges and highlights", () => { }); describe("composer", () => { - test("starting the edition with enter, the composer should have the focus", async () => { + test("grid composer is not visible when not editing", async () => { + expect(model.getters.getEditionMode()).toBe("inactive"); + const gridComposerEl = fixture.querySelector(".o-grid-composer") as HTMLDivElement; + expect(gridComposerEl.style.zIndex).toBe("-1000"); + await startComposition(); + expect(gridComposerEl.style.zIndex).toBe(""); + }); + + test("starting the edition with enter, the grid composer should have the focus", async () => { await startComposition(); expect(model.getters.getEditionMode()).toBe("editing"); expect(model.getters.getPosition()).toEqual(toCartesian("A1")); @@ -448,6 +456,7 @@ describe("composer", () => { test("ArrowKeys will move to neighbour cell, if not in contentFocus mode (left/right)", async () => { composerEl = await startComposition("a"); + await nextTick(); expect(composerEl.textContent).toBe("a"); await keyDown("ArrowRight"); expect(getCellText(model, "A1")).toBe("a"); @@ -472,8 +481,8 @@ describe("composer", () => { test("Arrow keys will not move to neighbor cell when a formula", async () => { composerEl = await startComposition("="); - await typeInComposerGrid(`"`); - await typeInComposerGrid(`"`); + await typeInComposerGrid(`"`, false); + await typeInComposerGrid(`"`, false); expect(composerEl.textContent).toBe(`=""`); await keyDown("ArrowLeft"); expect(model.getters.getEditionMode()).not.toBe("inactive"); @@ -511,7 +520,7 @@ describe("composer", () => { composerEl.dispatchEvent(new Event("keyup")); await rightClickCell(model, "C8"); expect(getActiveXc(model)).toBe("C8"); - expect(fixture.querySelectorAll(".o-grid div.o-composer")).toHaveLength(0); + expect(model.getters.getEditionMode()).toBe("inactive"); }); test("type '=' in the sheet and select a cell", async () => { @@ -612,15 +621,13 @@ describe("composer", () => { setCellContent(model, "A1", "[label](http://odoo.com)"); await hoverCell(model, "A1", 400); expect(fixture.querySelector(".o-link-tool")).not.toBeNull(); - await startComposition(); await typeInComposerGrid(" updated"); await keyDown("Enter"); const cell = getCell(model, "A1") as LinkCell; - expect(cell.link.label).toBe("label updated"); expect(cell.link.url).toBe("http://odoo.com"); }); - test("Hitting enter on topbar composer will properly update it", async () => { + test("Hitting enter on topbar composer will properly update it and stop the edition", async () => { setCellContent(model, "A1", "I am Tabouret"); await clickCell(model, "A1"); const topbarComposerElement = fixture.querySelector( @@ -630,6 +637,8 @@ describe("composer", () => { await simulateClick(topbarComposerElement); await keyDown("Enter"); expect(topbarComposerElement.textContent).toBe(""); + expect(model.getters.getEditionMode()).toBe("inactive"); + expect(document.activeElement).toBe(fixture.querySelector(".o-grid div.o-composer")!); }); test("Losing focus on topbar composer will properly update it", async () => { @@ -730,7 +739,7 @@ describe("composer", () => { composerEl = await startComposition(); await typeInComposerGrid(matchingValue); await moveToStart(); - composerEl = await typeInComposerGrid(formula + ","); + composerEl = await typeInComposerGrid(formula + ",", false); expect(model.getters.getEditionMode()).toBe("selecting"); expect(composerEl.textContent).toBe(formula + "," + matchingValue); expect(cehMock.selectionState.isSelectingRange).toBeTruthy(); @@ -751,7 +760,7 @@ describe("composer", () => { composerEl = await startComposition(); await typeInComposerGrid(mismatchingValue); await moveToStart(); - await typeInComposerGrid(formula + ","); + await typeInComposerGrid(formula + ",", false); expect(model.getters.getEditionMode()).not.toBe("selecting"); expect(composerEl.textContent).toBe(formula + "," + mismatchingValue); } @@ -764,7 +773,7 @@ describe("composer", () => { await typeInComposerGrid(matchingValue); await moveToStart(); const formulaInput = formula + ", "; - composerEl = await typeInComposerGrid(formulaInput); + composerEl = await typeInComposerGrid(formulaInput, false); expect(model.getters.getEditionMode()).toBe("selecting"); expect(composerEl.textContent).toBe(formulaInput + matchingValue); expect(cehMock.selectionState.isSelectingRange).toBeTruthy(); @@ -778,7 +787,7 @@ describe("composer", () => { composerEl = await startComposition(); await typeInComposerGrid(mismatchingValue); await moveToStart(); - await typeInComposerGrid(formula + ", "); + await typeInComposerGrid(formula + ", ", false); expect(model.getters.getEditionMode()).not.toBe("selecting"); expect(composerEl.textContent).toBe(formula + ", " + mismatchingValue); expect(cehMock.selectionState.isSelectingRange).toBeFalsy(); @@ -791,7 +800,7 @@ describe("composer", () => { composerEl = await startComposition(); await typeInComposerGrid(" " + matchingValue); await moveToStart(); - composerEl = await typeInComposerGrid(formula + ","); + composerEl = await typeInComposerGrid(formula + ",", false); expect(model.getters.getEditionMode()).toBe("selecting"); expect(composerEl.textContent).toBe(formula + ", " + matchingValue); expect(cehMock.selectionState.isSelectingRange).toBeTruthy(); @@ -805,7 +814,7 @@ describe("composer", () => { composerEl = await startComposition(); await typeInComposerGrid(" " + mismatchingValue); await moveToStart(); - composerEl = await typeInComposerGrid(formula + ","); + composerEl = await typeInComposerGrid(formula + ",", false); expect(model.getters.getEditionMode()).not.toBe("selecting"); expect(cehMock.selectionState.isSelectingRange).toBeFalsy(); expect(composerEl.textContent).toBe(formula + ", " + mismatchingValue); @@ -955,17 +964,17 @@ describe("composer", () => { // Editing text await typeInComposerGrid("hey"); - expect(fixture.querySelector(".o-grid .o-composer")).toBeTruthy(); + expect(model.getters.getEditionMode()).not.toBe("inactive"); activateSheet(model, "42"); await nextTick(); - expect(fixture.querySelector(".o-grid .o-composer")).toBeFalsy(); + expect(model.getters.getEditionMode()).toBe("inactive"); // Editing formula await typeInComposerGrid("="); - expect(fixture.querySelector(".o-grid .o-composer")).toBeTruthy(); + expect(model.getters.getEditionMode()).not.toBe("inactive"); activateSheet(model, baseSheetId); await nextTick(); - expect(fixture.querySelector(".o-grid .o-composer")).toBeTruthy(); + expect(model.getters.getEditionMode()).not.toBe("inactive"); }); test("the composer should keep the focus after changing sheet", async () => { @@ -1204,14 +1213,6 @@ describe("composer", () => { await typeInComposerGrid("="); await rightClickCell(model, "C8"); expect(model.getters.getEditionMode()).toBe("inactive"); - expect(fixture.querySelectorAll(".o-grid div.o-composer")).toHaveLength(0); - }); - - test("The composer should be closed before selecting headers", async () => { - await typeInComposerGrid("Hello"); - expect(fixture.querySelectorAll(".o-grid div.o-composer")).toHaveLength(1); - await selectColumnByClicking(model, "C"); - expect(fixture.querySelectorAll(".o-grid div.o-composer")).toHaveLength(0); }); test("The content in the composer should be kept after selecting headers", async () => { @@ -1243,7 +1244,7 @@ describe("composer", () => { test("Add a character changing the edition mode to waitingForRangeSelection correctly renders the composer", async () => { await typeInComposerGrid("=sum(4"); expect(cehMock.selectionState.isSelectingRange).toBeFalsy(); - await typeInComposerGrid(","); + await typeInComposerGrid(",", false); expect(cehMock.selectionState.isSelectingRange).toBeTruthy(); }); diff --git a/tests/components/figure.test.ts b/tests/components/figure.test.ts index bccea3eb94..d826bb2d1a 100644 --- a/tests/components/figure.test.ts +++ b/tests/components/figure.test.ts @@ -106,7 +106,7 @@ describe("figures", () => { expect(document.activeElement).toBe(fixture.querySelector(".o-figure")); }); - test("deleting a figure focuses the grid hidden input", async () => { + test.skip("deleting a figure focuses the grid hidden input", async () => { createFigure(model); await nextTick(); const figure = fixture.querySelector(".o-figure")!; @@ -115,7 +115,7 @@ describe("figures", () => { figure.dispatchEvent(new KeyboardEvent("keydown", { key: "Delete" })); await nextTick(); expect(fixture.querySelector(".o-figure")).toBeNull(); - expect(document.activeElement).toBe(fixture.querySelector(".o-grid>input")); + expect(document.activeElement).toBe(fixture.querySelector(".o-grid div.o-composer")); }); test("deleting a figure doesn't delete selection", async () => { diff --git a/tests/components/formula_assistant.test.ts b/tests/components/formula_assistant.test.ts index 0c9b84d902..3c5084407f 100644 --- a/tests/components/formula_assistant.test.ts +++ b/tests/components/formula_assistant.test.ts @@ -303,7 +303,7 @@ describe("formula assistant", () => { test("=FUNC1(42 then add ',' focus index on 2nd arg", async () => { await typeInComposerGrid("=FUNC1(42"); - await typeInComposerGrid(","); + await typeInComposerGrid(",", false); expect( fixture.querySelectorAll(".o-formula-assistant-arg.o-formula-assistant-focus span")[0] .textContent diff --git a/tests/components/grid.test.ts b/tests/components/grid.test.ts index 3649f62577..b0f0264cf7 100644 --- a/tests/components/grid.test.ts +++ b/tests/components/grid.test.ts @@ -744,25 +744,26 @@ describe("Grid component", () => { ); expect(getCell(model, "C2")!.style).toEqual({ bold: true }); }); + test("closing contextmenu focuses the grid", async () => { await rightClickCell(model, "B2"); await simulateClick(".o-menu div[data-name='add_row_before']"); expect(fixture.querySelector(".o-menu div[data-name='add_row_before']")).toBeFalsy(); - expect(document.activeElement).toBe(fixture.querySelector(".o-grid>input")); + expect(document.activeElement).toBe(fixture.querySelector(".o-grid div.o-composer")); }); test("Duplicating sheet in the bottom bar focus the grid afterward", async () => { - expect(document.activeElement).toBe(fixture.querySelector(".o-grid>input")); + expect(document.activeElement).toBe(fixture.querySelector(".o-grid div.o-composer")); // open and close sheet context menu await simulateClick(".o-spreadsheet-bottom-bar .o-all-sheets .o-sheet-item .o-icon"); await simulateClick(".o-menu-item[title='Duplicate']"); - expect(document.activeElement).toBe(fixture.querySelector(".o-grid>input")); + expect(document.activeElement).toBe(fixture.querySelector(".o-grid div.o-composer")); }); test("Can open context menu with a keyboard input ", async () => { - const selector = ".o-grid>input"; + const selector = ".o-grid div.o-composer"; const target = document.querySelector(selector)! as HTMLElement; target.focus(); triggerMouseEvent(selector, "contextmenu", 0, 0, { button: 1, bubbles: true }); diff --git a/tests/components/overlay.test.ts b/tests/components/overlay.test.ts index 6011c7381a..048cc56ce5 100644 --- a/tests/components/overlay.test.ts +++ b/tests/components/overlay.test.ts @@ -26,8 +26,10 @@ import { triggerMouseEvent, } from "../test_helpers/dom_helper"; import { getActiveXc, getCell } from "../test_helpers/getters_helpers"; -import { mountSpreadsheet, nextTick } from "../test_helpers/helpers"; - +import { mountSpreadsheet, nextTick, typeInComposerGrid } from "../test_helpers/helpers"; +jest.mock("../../src/components/composer/content_editable_helper", () => + require("./__mocks__/content_editable_helper") +); let fixture: HTMLElement; let model: Model; @@ -211,6 +213,13 @@ describe("Resizer component", () => { expect(getActiveXc(model)).toBe("C1"); }); + test("The composer should be closed before selecting headers", async () => { + await typeInComposerGrid("Hello"); + expect(model.getters.getEditionMode()).not.toBe("inactive"); + await selectColumnByClicking(model, "C"); + expect(model.getters.getEditionMode()).toBe("inactive"); + }); + test("Can resize a column", async () => { await resizeColumn("C", 50); expect(model.getters.getColSize(model.getters.getActiveSheetId(), 1)).toBe( diff --git a/tests/components/side_panel.test.ts b/tests/components/side_panel.test.ts index 164f237a99..5d85f55422 100644 --- a/tests/components/side_panel.test.ts +++ b/tests/components/side_panel.test.ts @@ -135,6 +135,6 @@ describe("Side Panel", () => { await nextTick(); simulateClick(".o-sidePanelClose"); await nextTick(); - expect(document.activeElement).toBe(fixture.querySelector(".o-grid>input")); + expect(document.activeElement).toBe(fixture.querySelector(".o-grid div.o-composer")); }); }); diff --git a/tests/components/spreadsheet.test.ts b/tests/components/spreadsheet.test.ts index 7af221253d..e18de0967a 100644 --- a/tests/components/spreadsheet.test.ts +++ b/tests/components/spreadsheet.test.ts @@ -34,6 +34,7 @@ import { typeInComposerTopBar, } from "../test_helpers/helpers"; import { mockChart } from "./__mocks__/chart"; +import { ContentEditableHelper } from "./__mocks__/content_editable_helper"; jest.mock("../../src/components/composer/content_editable_helper", () => require("./__mocks__/content_editable_helper") @@ -68,17 +69,15 @@ describe("Simple Spreadsheet Component", () => { }); test("focus is properly set, initially and after switching sheet", async () => { - ({ model, parent, fixture } = await mountSpreadsheet({ - model: new Model({ sheets: [{ id: "sh1" }] }), - })); - // TODO check - expect(document.activeElement!.tagName).toEqual("INPUT"); + ({ model, parent, fixture } = await mountSpreadsheet()); + const defaultComposer = fixture.querySelector(".o-grid div.o-composer"); + expect(document.activeElement).toBe(defaultComposer); document.querySelector(".o-add-sheet")!.dispatchEvent(new Event("click")); await nextTick(); expect(document.querySelectorAll(".o-sheet").length).toBe(2); - expect(document.activeElement!.tagName).toEqual("INPUT"); + expect(document.activeElement).toBe(defaultComposer); await simulateClick(document.querySelectorAll(".o-sheet")[1]); - expect(document.activeElement!.tagName).toEqual("INPUT"); + expect(document.activeElement).toBe(defaultComposer); }); describe("Use of env in a function", () => { @@ -144,7 +143,10 @@ describe("Simple Spreadsheet Component", () => { })); await simulateClick(`div[title="Bold"]`); expect(document.activeElement).not.toBeNull(); - document.activeElement?.dispatchEvent(new InputEvent("input", { data: "d", bubbles: true })); + // @ts-ignore + const cehMock = window.mockContentHelper as ContentEditableHelper; + cehMock.insertText("d"); + document.activeElement!.dispatchEvent(new InputEvent("input", { data: "d", bubbles: true })); await nextTick(); expect(model.getters.getEditionMode()).toBe("editing"); expect(model.getters.getCurrentContent()).toBe("d"); @@ -321,11 +323,11 @@ describe("Simple Spreadsheet Component", () => { test("grid should regain focus after a topbar menu option is selected", async () => { ({ parent, fixture } = await mountSpreadsheet()); - expect(document.activeElement!.tagName).toEqual("INPUT"); + expect(document.activeElement!.classList).toContain("o-composer"); triggerMouseEvent(".o-topbar-menu[data-id='format']", "click"); await nextTick(); await simulateClick(".o-menu-item[title='Bold']"); - expect(document.activeElement!.tagName).toEqual("INPUT"); + expect(document.activeElement!.classList).toContain("o-composer"); }); }); @@ -376,7 +378,7 @@ describe("Composer interactions", () => { // Focus grid composer and type triggerMouseEvent(".o-grid .o-composer", "click"); await nextTick(); - await typeInComposerGrid("from grid"); + await typeInComposerGrid("from grid", false); expect(topBarComposer!.textContent).toBe("from topbarfrom grid"); expect(gridComposer!.textContent).toBe("from topbarfrom grid"); }); @@ -524,8 +526,7 @@ describe("Composer / selectionInput interactions", () => { // focus selection input await simulateClick(".o-selection-input input"); - - expect(fixture.querySelectorAll(".o-grid-composer")).toHaveLength(0); + expect(model.getters.getEditionMode()).toBe("inactive"); } ); diff --git a/tests/test_helpers/helpers.ts b/tests/test_helpers/helpers.ts index 157687e8b3..282b932d4e 100644 --- a/tests/test_helpers/helpers.ts +++ b/tests/test_helpers/helpers.ts @@ -3,6 +3,7 @@ import { ChartConfiguration } from "chart.js"; import format from "xml-formatter"; import { Spreadsheet, SpreadsheetProps } from "../../src/components/spreadsheet/spreadsheet"; import { functionRegistry } from "../../src/functions/index"; +import { FocusableElement } from "../../src/helpers/focus_manager"; import { toCartesian, toUnboundedZone, toXC, toZone } from "../../src/helpers/index"; import { Model } from "../../src/model"; import { MergePlugin } from "../../src/plugins/core/merge"; @@ -129,6 +130,7 @@ export function makeTestEnv(mockEnv: Partial = {}): Spreads (async () => { return [] as Currency[]; }), + focusableElement: new FocusableElement(), }; } @@ -388,7 +390,7 @@ async function typeInComposerHelper(selector: string, text: string, fromScratch: cehMock.insertText(text); composerEl.dispatchEvent(new Event("keydown", { bubbles: true })); await nextTick(); - composerEl.dispatchEvent(new Event("input", { bubbles: true })); + composerEl.dispatchEvent(new InputEvent("input", { data: text, bubbles: true })); await nextTick(); composerEl.dispatchEvent(new Event("keyup", { bubbles: true })); await nextTick(); @@ -396,7 +398,7 @@ async function typeInComposerHelper(selector: string, text: string, fromScratch: } export async function typeInComposerGrid(text: string, fromScratch: boolean = true) { - return await typeInComposerHelper(".o-grid .o-composer", text, fromScratch); + return await typeInComposerHelper(".o-grid div.o-composer", text, fromScratch); } export async function typeInComposerTopBar(text: string, fromScratch: boolean = true) { @@ -404,14 +406,16 @@ export async function typeInComposerTopBar(text: string, fromScratch: boolean = } export async function startGridComposition(key?: string) { + const gridComposerTarget = document.querySelector(".o-grid .o-composer")!; if (key) { - const gridInputEl = document.querySelector(".o-grid>input"); - gridInputEl!.dispatchEvent( + gridComposerTarget!.dispatchEvent( new InputEvent("input", { data: key, bubbles: true, cancelable: true }) ); } else { - const gridInputEl = document.querySelector(".o-grid"); - gridInputEl!.dispatchEvent( + gridComposerTarget!.dispatchEvent( + new KeyboardEvent("keydown", { key: "Escape", bubbles: true, cancelable: true }) + ); + gridComposerTarget!.dispatchEvent( new KeyboardEvent("keydown", { key: "Enter", bubbles: true, cancelable: true }) ); }