Skip to content

Commit 86b983f

Browse files
dhrp-odoodhrp-odoo
committed
[FIX] topbar: close font size dropdown and keep focus on the grid
Pressing Tab in the font size editor could move focus to the hidden “add more rows” footer. The browser then auto-scrolled to that footer while the grid viewport state stayed unchanged, making the footer look like it was floating above the grid. We now handle Tab on the font size input: we prevent the default navigation, close the dropdown, and redirect focus back to the grid composer instead of the footer. This keeps the layout stable when tabbing from the toolbar. Alternative approaches considered but discarded: - Closing on blur and refocusing the grid: clicking the arrow caused the input to blur first, so the blur handler closed the dropdown and the arrow click immediately reopened it, making it impossible to close the dropdown. - Using mousedown to distinguish internal clicks: this fired before selecting an item in the dropdown, so the dropdown closed too early and the item click was never applied. Task: 5263792 X-original-commit: 99a94ac
1 parent 62b85c8 commit 86b983f

File tree

6 files changed

+289
-1
lines changed

6 files changed

+289
-1
lines changed

‎src/components/font_size_editor/font_size_editor.ts‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,35 @@
1+
<<<<<<< 62b85c879351bdec7be5164365be9dcb86c47ba9
12
import { FONT_SIZES } from "@odoo/o-spreadsheet-engine/constants";
23
import { SpreadsheetChildEnv } from "@odoo/o-spreadsheet-engine/types/spreadsheet_env";
34
import { Component } from "@odoo/owl";
45
import { NumberEditor } from "../number_editor/number_editor";
6+
||||||| 641d1923a1b5601da2192cd01f8b4905d31a5aa7
7+
import { Component, useExternalListener, useRef, useState } from "@odoo/owl";
8+
import { FONT_SIZES } from "../../constants";
9+
import { clip } from "../../helpers/index";
10+
import { SpreadsheetChildEnv } from "../../types/index";
11+
import { css } from "../helpers/css";
12+
import { isChildEvent } from "../helpers/dom_helpers";
13+
import { Popover, PopoverProps } from "../popover";
14+
15+
interface State {
16+
isOpen: boolean;
17+
}
18+
=======
19+
import { Component, useExternalListener, useRef, useState } from "@odoo/owl";
20+
import { FONT_SIZES } from "../../constants";
21+
import { clip } from "../../helpers/index";
22+
import { Store, useStore } from "../../store_engine";
23+
import { DOMFocusableElementStore } from "../../stores/DOM_focus_store";
24+
import { SpreadsheetChildEnv } from "../../types/index";
25+
import { css } from "../helpers/css";
26+
import { isChildEvent } from "../helpers/dom_helpers";
27+
import { Popover, PopoverProps } from "../popover";
28+
29+
interface State {
30+
isOpen: boolean;
31+
}
32+
>>>>>>> 96be6b93372a0086042fc38a96cfa426d9fee592
533

634
interface Props {
735
currentFontSize: number;
@@ -26,5 +54,163 @@ export class FontSizeEditor extends Component<Props, SpreadsheetChildEnv> {
2654
onFocusInput: () => {},
2755
};
2856

57+
<<<<<<< 62b85c879351bdec7be5164365be9dcb86c47ba9
2958
fontSizes: number[] = FONT_SIZES;
59+
||||||| 641d1923a1b5601da2192cd01f8b4905d31a5aa7
60+
static components = { Popover };
61+
fontSizes = FONT_SIZES;
62+
63+
dropdown: State = useState({ isOpen: false });
64+
65+
private inputRef = useRef("inputFontSize");
66+
private rootEditorRef = useRef("FontSizeEditor");
67+
private fontSizeListRef = useRef("fontSizeList");
68+
69+
setup() {
70+
useExternalListener(window, "click", this.onExternalClick, { capture: true });
71+
}
72+
73+
get popoverProps(): PopoverProps {
74+
const { x, y, width, height } = this.rootEditorRef.el!.getBoundingClientRect();
75+
return {
76+
anchorRect: { x, y, width, height },
77+
positioning: "bottom-left",
78+
verticalOffset: 0,
79+
};
80+
}
81+
82+
onExternalClick(ev: MouseEvent) {
83+
if (!isChildEvent(this.fontSizeListRef.el!, ev) && !isChildEvent(this.rootEditorRef.el!, ev)) {
84+
this.closeFontList();
85+
}
86+
}
87+
88+
toggleFontList() {
89+
const isOpen = this.dropdown.isOpen;
90+
if (!isOpen) {
91+
this.props.onToggle?.();
92+
this.inputRef.el!.focus();
93+
} else {
94+
this.closeFontList();
95+
}
96+
}
97+
98+
closeFontList() {
99+
this.dropdown.isOpen = false;
100+
}
101+
102+
private setSize(fontSizeStr: string) {
103+
const fontSize = clip(Math.floor(parseFloat(fontSizeStr)), 1, 400);
104+
this.props.onFontSizeChanged(fontSize);
105+
this.closeFontList();
106+
}
107+
108+
setSizeFromInput(ev: InputEvent) {
109+
this.setSize((ev.target as HTMLInputElement).value);
110+
}
111+
112+
setSizeFromList(fontSizeStr: string) {
113+
this.setSize(fontSizeStr);
114+
}
115+
116+
onInputFocused(ev: InputEvent) {
117+
this.dropdown.isOpen = true;
118+
(ev.target as HTMLInputElement).select();
119+
}
120+
121+
onInputKeydown(ev: KeyboardEvent) {
122+
if (ev.key === "Enter" || ev.key === "Escape") {
123+
this.closeFontList();
124+
const target = ev.target as HTMLInputElement;
125+
// In the case of a ESCAPE key, we get the previous font size back
126+
if (ev.key === "Escape") {
127+
target.value = `${this.props.currentFontSize}`;
128+
}
129+
this.props.onToggle?.();
130+
}
131+
}
132+
=======
133+
static components = { Popover };
134+
fontSizes = FONT_SIZES;
135+
136+
dropdown: State = useState({ isOpen: false });
137+
138+
private inputRef = useRef("inputFontSize");
139+
private rootEditorRef = useRef("FontSizeEditor");
140+
private fontSizeListRef = useRef("fontSizeList");
141+
142+
private DOMFocusableElementStore!: Store<DOMFocusableElementStore>;
143+
144+
setup() {
145+
this.DOMFocusableElementStore = useStore(DOMFocusableElementStore);
146+
useExternalListener(window, "click", this.onExternalClick, { capture: true });
147+
}
148+
149+
get popoverProps(): PopoverProps {
150+
const { x, y, width, height } = this.rootEditorRef.el!.getBoundingClientRect();
151+
return {
152+
anchorRect: { x, y, width, height },
153+
positioning: "bottom-left",
154+
verticalOffset: 0,
155+
};
156+
}
157+
158+
onExternalClick(ev: MouseEvent) {
159+
if (!isChildEvent(this.fontSizeListRef.el!, ev) && !isChildEvent(this.rootEditorRef.el!, ev)) {
160+
this.closeFontList();
161+
}
162+
}
163+
164+
toggleFontList() {
165+
const isOpen = this.dropdown.isOpen;
166+
if (!isOpen) {
167+
this.props.onToggle?.();
168+
this.inputRef.el!.focus();
169+
} else {
170+
this.closeFontList();
171+
}
172+
}
173+
174+
closeFontList() {
175+
this.dropdown.isOpen = false;
176+
}
177+
178+
private setSize(fontSizeStr: string) {
179+
const fontSize = clip(Math.floor(parseFloat(fontSizeStr)), 1, 400);
180+
this.props.onFontSizeChanged(fontSize);
181+
this.closeFontList();
182+
}
183+
184+
setSizeFromInput(ev: InputEvent) {
185+
this.setSize((ev.target as HTMLInputElement).value);
186+
}
187+
188+
setSizeFromList(fontSizeStr: string) {
189+
this.setSize(fontSizeStr);
190+
}
191+
192+
onInputFocused(ev: InputEvent) {
193+
this.dropdown.isOpen = true;
194+
(ev.target as HTMLInputElement).select();
195+
}
196+
197+
onInputKeydown(ev: KeyboardEvent) {
198+
if (ev.key === "Enter" || ev.key === "Escape") {
199+
this.closeFontList();
200+
const target = ev.target as HTMLInputElement;
201+
// In the case of a ESCAPE key, we get the previous font size back
202+
if (ev.key === "Escape") {
203+
target.value = `${this.props.currentFontSize}`;
204+
}
205+
this.props.onToggle?.();
206+
}
207+
if (ev.key === "Tab") {
208+
ev.preventDefault();
209+
ev.stopPropagation();
210+
this.closeFontList();
211+
this.DOMFocusableElementStore.focus();
212+
return;
213+
}
214+
}
215+
>>>>>>> 96be6b93372a0086042fc38a96cfa426d9fee592
30216
}

‎src/components/font_size_editor/font_size_editor.xml‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<templates>
22
<t t-name="o-spreadsheet-FontSizeEditor">
3+
<<<<<<< 62b85c879351bdec7be5164365be9dcb86c47ba9
34
<NumberEditor
45
currentValue="props.currentFontSize"
56
onValueChange="props.onFontSizeChanged"
@@ -11,5 +12,78 @@
1112
valueList="fontSizes"
1213
title.translate="Font Size"
1314
/>
15+
||||||| 641d1923a1b5601da2192cd01f8b4905d31a5aa7
16+
<div class="o-dropdown" t-ref="FontSizeEditor">
17+
<div
18+
class=" o-font-size-editor d-flex align-items-center"
19+
t-att-class="props.class"
20+
title="Font Size"
21+
t-on-click="this.toggleFontList">
22+
<input
23+
type="number"
24+
min="1"
25+
max="400"
26+
class="o-font-size o-number-input bg-transparent border-0"
27+
t-on-keydown="onInputKeydown"
28+
t-on-wheel.prevent.stop=""
29+
t-on-click.stop="props.onFocusInput"
30+
t-on-focus.stop="onInputFocused"
31+
t-att-value="props.currentFontSize"
32+
t-on-change="setSizeFromInput"
33+
t-ref="inputFontSize"
34+
/>
35+
<span>
36+
<t t-call="o-spreadsheet-Icon.CARET_DOWN"/>
37+
</span>
38+
</div>
39+
<Popover t-if="dropdown.isOpen" t-props="popoverProps">
40+
<div class="o-text-options bg-white" t-on-click.stop="" t-ref="fontSizeList">
41+
<t t-foreach="fontSizes" t-as="fontSize" t-key="fontSize">
42+
<div
43+
t-esc="fontSize"
44+
t-att-data-size="fontSize"
45+
t-on-click="() => this.setSizeFromList(fontSize)"
46+
/>
47+
</t>
48+
</div>
49+
</Popover>
50+
</div>
51+
=======
52+
<div class="o-dropdown" t-ref="FontSizeEditor">
53+
<div
54+
class=" o-font-size-editor d-flex align-items-center"
55+
t-att-class="props.class"
56+
title="Font Size"
57+
t-on-click.stop="this.toggleFontList">
58+
<input
59+
type="number"
60+
min="1"
61+
max="400"
62+
class="o-font-size o-number-input bg-transparent border-0"
63+
t-on-keydown="onInputKeydown"
64+
t-on-wheel.prevent.stop=""
65+
t-on-click.stop="props.onFocusInput"
66+
t-on-focus.stop="onInputFocused"
67+
t-att-value="props.currentFontSize"
68+
t-on-change="setSizeFromInput"
69+
t-ref="inputFontSize"
70+
/>
71+
<span>
72+
<t t-call="o-spreadsheet-Icon.CARET_DOWN"/>
73+
</span>
74+
</div>
75+
<Popover t-if="dropdown.isOpen" t-props="popoverProps">
76+
<div class="o-text-options bg-white" t-on-click.stop="" t-ref="fontSizeList">
77+
<t t-foreach="fontSizes" t-as="fontSize" t-key="fontSize">
78+
<div
79+
t-esc="fontSize"
80+
t-att-data-size="fontSize"
81+
t-on-click="() => this.setSizeFromList(fontSize)"
82+
/>
83+
</t>
84+
</div>
85+
</Popover>
86+
</div>
87+
>>>>>>> 96be6b93372a0086042fc38a96cfa426d9fee592
1488
</t>
1589
</templates>

‎src/components/grid_add_rows_footer/grid_add_rows_footer.xml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
<button
88
t-on-click="onConfirm"
99
t-att-disabled="state.errorFlag"
10-
class="o-button flex-grow-0 me-2">
10+
class="o-button flex-grow-0 me-2"
11+
tabindex="-1">
1112
Add
1213
</button>
1314
<input
@@ -19,6 +20,7 @@
1920
t-on-keydown.stop="onKeydown"
2021
t-on-pointerdown.stop=""
2122
t-on-input.stop="onInput"
23+
tabindex="-1"
2224
/>
2325
<span>more rows at the bottom</span>
2426
<ValidationMessages t-if="state.errorFlag" messages="errorMessages" msgType="'error'"/>

‎tests/grid/__snapshots__/grid_component.test.ts.snap‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@ exports[`Grid component simple rendering snapshot 1`] = `
2929
>
3030
<button
3131
class="o-button flex-grow-0 me-2"
32+
tabindex="-1"
3233
>
3334
Add
3435
</button>
3536
<input
3637
class="o-grid-add-rows-input o-input mt-0 me-2"
38+
tabindex="-1"
3739
type="text"
3840
value="100"
3941
/>

‎tests/spreadsheet/__snapshots__/spreadsheet_component.test.ts.snap‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,11 +835,13 @@ exports[`Simple Spreadsheet Component simple rendering snapshot 1`] = `
835835
>
836836
<button
837837
class="o-button flex-grow-0 me-2"
838+
tabindex="-1"
838839
>
839840
Add
840841
</button>
841842
<input
842843
class="o-grid-add-rows-input o-input mt-0 me-2"
844+
tabindex="-1"
843845
type="text"
844846
value="100"
845847
/>
@@ -1842,11 +1844,13 @@ exports[`components take the small screen into account 1`] = `
18421844
>
18431845
<button
18441846
class="o-button flex-grow-0 me-2"
1847+
tabindex="-1"
18451848
>
18461849
Add
18471850
</button>
18481851
<input
18491852
class="o-grid-add-rows-input o-input mt-0 me-2"
1853+
tabindex="-1"
18501854
type="text"
18511855
value="100"
18521856
/>

‎tests/top_bar_component.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
doubleClick,
3131
getElComputedStyle,
3232
getTextNodes,
33+
keyDown,
3334
simulateClick,
3435
triggerMouseEvent,
3536
} from "./test_helpers/dom_helper";
@@ -451,6 +452,25 @@ describe("TopBar component", () => {
451452
expect(getStyle(model, "A1").fontSize).toBe(8);
452453
});
453454

455+
test("Tab from font size editor closes the dropdown and moves focus to grid", async () => {
456+
const { fixture } = await mountSpreadsheet();
457+
const input = fixture.querySelector("input.o-font-size") as HTMLInputElement;
458+
input.focus();
459+
await nextTick();
460+
expect(fixture.querySelector(".o-popover .o-text-options")).toBeTruthy();
461+
await keyDown({ key: "Tab" });
462+
expect(fixture.querySelector(".o-popover .o-text-options")).toBeFalsy();
463+
const composerEl = fixture.querySelector<HTMLElement>(".o-grid-composer .o-composer")!;
464+
expect(document.activeElement).toBe(composerEl);
465+
});
466+
467+
test("Clicking the font size dropdown arrow focuses the input", async () => {
468+
const { fixture } = await mountSpreadsheet();
469+
const input = fixture.querySelector("input.o-font-size") as HTMLInputElement;
470+
await click(fixture, ".o-font-size-editor .o-icon");
471+
expect(document.activeElement).toBe(input);
472+
});
473+
454474
test("prevents default behavior of mouse wheel event on font size input", async () => {
455475
await mountParent();
456476
const fontSizeInput = fixture.querySelector("input.o-font-size") as HTMLInputElement;

0 commit comments

Comments
 (0)