Skip to content

Commit

Permalink
[FIX] pivot: mouse events do not work in pivot text input
Browse files Browse the repository at this point in the history
We added a behaviour to select the whole text of the measure name input
when the input is focused. We select the whole input text on `focus`
event, but we had to stop/prevent the `mouseup` events, otherwise
the event would sometime unselect the text.

It turns out (surprise) that entirely disabling `mouseup` event on the
input was a bad idea: it prevented the user from moving the cursor
inside the input when all the text was selected.

This commit changes the way we handle all this:

- if the input is not focused, we disable the `mousedown` event on it.
This prevent the `focus` event from firing.
- if the input is not focused and we catch a `mouseup` event on the
input,  we select the whole text and focus the input manually.
- if the input is focused, we do not touch the events and let the
browser handle them.

closes #5415

Task: 4350439
X-original-commit: 85aaab6
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
  • Loading branch information
hokolomopo committed Jan 7, 2025
1 parent 5f27a7e commit a7552fd
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 14 deletions.
24 changes: 15 additions & 9 deletions src/components/text_input/text_input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,21 @@ export class TextInput extends Component<Props, SpreadsheetChildEnv> {
this.inputRef.el?.blur();
}

focusInputAndSelectContent() {
const inputEl = this.inputRef.el;
if (!inputEl) return;
onMouseDown(ev: MouseEvent) {
// Stop the event if the input is not focused, we handle everything in onMouseUp
if (ev.target !== document.activeElement) {
ev.preventDefault();
ev.stopPropagation();
}
}

// The onFocus event selects all text in the input.
// The subsequent mouseup event can deselect this text,
// so t-on-mouseup.prevent.stop is used to prevent this
// default behavior and preserve the selection.
inputEl.focus();
inputEl.select();
onMouseUp(ev: MouseEvent) {
const target = ev.target as HTMLInputElement;
if (target !== document.activeElement) {
target.focus();
target.select();
ev.preventDefault();
ev.stopPropagation();
}
}
}
4 changes: 2 additions & 2 deletions src/components/text_input/text_input.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
t-att-placeholder="props.placeholder"
t-att-value="props.value"
t-on-change="save"
t-on-focus="focusInputAndSelectContent"
t-on-blur="save"
t-on-mouseup.prevent.stop=""
t-on-pointerdown="onMouseDown"
t-on-pointerup="onMouseUp"
t-on-keydown="onKeyDown"
/>
</div>
Expand Down
11 changes: 8 additions & 3 deletions tests/components/text_input.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { Component, xml } from "@odoo/owl";
import { SpreadsheetChildEnv } from "../../src";
import { TextInput } from "../../src/components/text_input/text_input";
import { click, keyDown, setInputValueAndTrigger } from "../test_helpers/dom_helper";
import {
click,
keyDown,
setInputValueAndTrigger,
triggerMouseEvent,
} from "../test_helpers/dom_helper";
import { mountComponent } from "../test_helpers/helpers";

let fixture: HTMLElement;
Expand Down Expand Up @@ -64,9 +69,9 @@ describe("TextInput", () => {
expect(onChange).toHaveBeenCalledWith("world");
});

test("selects input content upon focus", async () => {
test("selects input content upon mouseup", async () => {
await mountTextInput({ value: "hello", onChange: () => {} });
fixture.querySelector("input")!.focus();
triggerMouseEvent("input", "pointerup");
expect(fixture.querySelector("input")!.selectionStart).toEqual(0);
expect(fixture.querySelector("input")!.selectionEnd).toEqual(5);
});
Expand Down

0 comments on commit a7552fd

Please sign in to comment.