Close only “child” window (e.g. Preview) inside Browser on Cmd+W

[beyondcompute]

Currently if we open Preview (for example) while being in the Browse mode, and then we try to close the Preview by pressing Cmd+W, both the Preview and the “parent” Browse window get closed. This feels counter-intuitive as Cmd+W should only close the “current window”.

Before

Before.mov

After

After.mov

It is interesting that Qt does not do ”the right thing” out of the box. 📦

[dae]

This is an improvement, though there's still an issue not caused by this PR: if the user switches to the main window then back to preview, the shortcut stops working, as the main window's menu is still active.

Given #3905 as well, it's got me wondering whether we'd be better off with a global shortcut handler that does something like what you've done here, so we don't need to manually add the shortcuts to each different dialog. WDYT?

[beyondcompute]

This is an improvement, though there's still an issue not caused by this PR: if the user switches to the main window then back to preview, the shortcut stops working, as the main window's menu is still active.

Given #3905 as well, it's got me wondering whether we'd be better off with a global shortcut handler that does something like what you've done here, so we don't need to manually add the shortcuts to each different dialog. WDYT?

Yeah, it is an interesting issue with switching windows. I wonder, does Qt actually offer some pattern on how to manage those handlers.

As a side note, I tried “simply closing the Browse window if it’s active and do nothing otherwise (hoping that a sub-dialog has its own handler)” approach, and that does not work. Even if I install the handler to a sub-window, it apparently gets intercepted by the Browse window. 🙈

The global approach obviously sounds better! I suppose, I’ll try to at least research into that direction. But I am not sure that I won’t miss some important aspects of Anki’s architecture or Qt’s approaches.

[dae]

It's a learning experience for all of us :-)

[dae]

I noticed you updated this - does that mean you'd prefer to merge this in as-is for now?

[beyondcompute]

@dae, yes, I think I’d prefer that.

1. Thanks for merging the other PR as well! 🙏
2. I just remembered yesterday to go and prune my branches and decided to rebase “just in case”. Of course, it’s your call if this is OK to be merged.
3. I quickly tried some things regarding the global keyboard shortcut handling. But nothing worked so far. It seems also that Anki has a custom mechanism for opening dialog windows… I haven’t spent that much time on the issue but in any case, I am not sure if I would be able to confidently test a larger refactoring if that were to take place.

[beyondcompute]

Actually #3885 could affect how close actions work. 🤔

[dae]

Not all of our windows have access to a menubar, and I don't think we'll be moving to a unified bar in the near future, so a global shortcut is more useful.