[FIX] topbar: close font size editor dropdown and focus grid #7495
Conversation
|
|
tests/top_bar_component.test.ts
Outdated
| input.focus(); | ||
| await nextTick(); | ||
| expect(fixture.querySelector(".o-dropdown-content.o-text-options")).toBeTruthy(); | ||
| input.blur(); |
There was a problem hiding this comment.
I think jsdom doesn’t implement real Tab-based focus navigation, so we call blur() directly here to reliably trigger the blur handler in this test.
| const isOpen = this.dropdown.isOpen; | ||
| if (!isOpen) { | ||
| this.props.onToggle(); | ||
| this.inputRef.el!.focus(); |
There was a problem hiding this comment.
why remove this ? don't we want the input focused when clicking on the arrow ?
There was a problem hiding this comment.
Actually, if we keep the font editor input focused when clicking the arrow, the dropdown becomes impossible to close.
When we click the arrow to close it, the input first triggers a blur event (which closes the dropdown), and then the click handler on the arrow runs and opens it again. Since blur happens before click, the two handlers cancel each other out.
I tried the mousedown event as well (as it is called before the blur), but then we need to replace all the click events with mousedown in this component. I am not sure if that is good or not.
If we really want the focus on the font editor input, then I guess better to remove the blur handler and directly handle the Tab event inside the component? What do you think? Let me know if you have a better approach
There was a problem hiding this comment.
Oh yeah this sounds annoying ... maybe we could close the dropwdown on an external click event rather than on blur ?
There was a problem hiding this comment.
It’s already handled; I just also wanted it to close when we press Tab in the font size input, so focus returns to the grid, which selects the cell and moves to the right.
There was a problem hiding this comment.
So handling tab manually with a prevent default should do the trick I guess. Not that pretty but 🤷
There was a problem hiding this comment.
Yeah, I tried all the other approaches but couldn’t find anything better. All the solutions feel hacky (creating a boolean to skip the calls, applying mousedowns)
dhrp-odoo
force-pushed
the
17.0-tab-navigation-breaks-layout-dhrp
branch
from
b3b7064 to
d7c7704
Compare
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
dhrp-odoo
force-pushed
the
17.0-tab-navigation-breaks-layout-dhrp
branch
from
d7c7704 to
96be6b9
Compare
LucasLefevre
left a comment
LucasLefevre
left a comment
There was a problem hiding this comment.
Thanks :)
robodoo r+
4 participants
Description:
Pressing Tab in the font size editor moves focus to the hidden “add more rows” footer (the next focusable element in the DOM). This caused the browser to auto-scroll to the footer while the grid viewport state remained unchanged, making the footer appear to float over the grid.
Now, when the font size editor loses focus (via blur/Tab), it closes its dropdown and redirects focus back to the grid instead of the footer.
Task: 5263792
review checklist