[FIX] figure: wrong focus change on figure unmount

[hokolomopo]

Description

When a figure is unmounted, the focus is moved to the default focusable element (the grid composer). But this change is done even if the figure wasn't focused. For example, the focus would be removed from the find & replace search search input when a collaborative user would delete a figure.

Task: 5154025

review checklist

• [ ] feature is organized in plugin, or UI components
• [ ] support of duplicate sheet (deep copy)
• [ ] in model/core: ranges are Range object, and can be adapted (adaptRanges)
• [ ] in model/UI: ranges are strings (to show the user)
• [ ] undo-able commands (uses this.history.update)
• [ ] multiuser-able commands (has inverse commands and transformations where needed)
• [ ] new/updated/removed commands are documented
• [ ] exportable in excel
• [ ] translations (_t("qmsdf %s", abc))
• [ ] unit tested
• [ ] clean commented code
• [ ] track breaking changes
• [ ] doc is rebuild (npm run doc)
• [ ] status is correct in Odoo

[robodoo]

[hokolomopo]

@robodoo fw=no

[robodoo]

Disabled forward-porting.

[rrahir]

It reminds me (or specifically the mentioning of F&R in the task) of https://github.com/odoo/o-spreadsheet/pull/2330/commits/9866576d761247f4529f02e19cd9f30d89926937

I think that:

• the condition set on the useEffect could/should not depend on the activeSheet but rather be checked all the time
• backporting the commit (withtout the activesheet condition) would achieve a similar result and at least would not require specific subcomponents to wonder about whether or not they should focus the grid. Wdyt?

[hokolomopo]

It reminds me (or specifically the mentioning of F&R in the task) of 9866576

I think that:

• the condition set on. the useEffect could/should not depend on the activeSheet but rather be checked all the time
• backporting the commit (withtout the activesheet condition) would achieve a similar result and at least would not require specific subcomponents to wonder about whether or not they should focus the grid. Wdyt?

@rrahir that's a good idea, it simplifies the handling of figures (there's no need for a onFigureDeleted at all). Unfortunately, it doesn't work for the GridAddRowsFooter, because blurring the input doesn't trigger a render, so no useEffect.

Still I made the changes for figures in the two PRs 🙂

[hokolomopo]

Knowing how fragile focus is and how easy it is to break it, I wouldn't merge this in 17.0, which is very stable now. Especially if it's only an issue in a collaborative context

Fine with me 🙂