call windowManager.show() and windowManager.restore() together as an extension

[levkropp]

resolve #3667

windowManager.restore() calls gtk_window_deiconify as well as gtk_window_present https://docs.gtk.org/gtk3/method.Window.deiconify.html

In my testing, this has made it so "Open in Multipass" for an instance either

• opens the Multipass window (if keyboard focus is not stolen), or
• shows the " is ready" dialog if typing focus is on another app

100% of the time, and fixes the issues seen from #3667 without needing to fork the flutter window_manager plugin

[andrei-toterman]

Good that you found this method, @levkropp! But there are multiple places where we call show, and I think those might need to be replaced by restore as well :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.85%. Comparing base (d2d4a82) to head (418162b).
Report is 119 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3671      +/-   ##
==========================================
- Coverage   88.85%   88.85%   -0.01%     
==========================================
  Files         254      254              
  Lines       14261    14269       +8     
==========================================
+ Hits        12672    12679       +7     
- Misses       1589     1590       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[levkropp]

there are multiple places where we call show, and I think those might need to be replaced by restore as well :)

Good catch @andrei-toterman , thank you! I have replaced the other occurrence of show() in tray_menu for the "Toggle Window" button with restore() as well

The only other instance of windowManager.show() I see is in main.dart in the waitUntilReadyToShow method, but I don't believe this should be replaced since we're not in a minimized state at this point, and there is no need to have a call to deiconify

[andrei-toterman]

@levkropp check main.dart again ;)

[andrei-toterman]

Unfortunately, replacing show with restore breaks things on Windows, since the window no longer appears at all after being hidden. Looking at the window_manager source code, it seems that the Windows implementation of restore does not call some of the functions called by show. So there are several options. We could call show and restore together all the time. We could fork the plugin and make it do what we need in one function call. Or we could introduce a platform-specific method that calls the appropriate window_manager methods when needed i.e. show for Windows and restore for the rest. Opinions, @levkropp and @ricab?

[ricab]

Do you foresee any issues with just calling show + restore? Would it perhaps make things slow? If not, I guess that would be an easy fix. Otherwise, if we find that there is something wrong with the window_manager implementation that should be fixed, I would say fork+upstream. If it is not something wrong overall but just a specific need of ours, perhaps a platform method.

[andrei-toterman]

No, I don't think there would be any issues with calling both all the time, and that would indeed be an easy fix. Just wanted to hear other opinions :)

[andrei-toterman]

@levkropp I think we first need show and then restore. On Windows, restore has no effect until show is called (which might not be a problem, but better safe than sorry). Also, you could group those two method calls into a single function, so you don't have to actually call them together all the time.
You could look into creating an extension method for the WindowManager class called showAndRestore or something like that. Make sure the method is async and awaits both show and restore ;)
You can place that extension in the tray menu file, since that's the most relevant place I can think of...

[levkropp]

You can place that extension in the tray menu file, since that's the most relevant place I can think of...

Thank you for the feedback @andrei-toterman ! I like the idea of making an extension for window_manager to handle this as well

If we place the extension in the tray_menu file then I believe we'll have to import tray_menu.dart to access it if we want to use it for goToPrimary in main.dart

We could make a new .dart file to hold the extension, or perhaps not import the extension at all for the main.dart case since goToPrimary is the only place where restore() is needed

[andrei-toterman]

@levkropp we already import tray_menu.dart into main.dart since that's where we call setupTrayMenu which creates the tray menu itself, so that would not be an issue. But now that I look again, we have an extensions.dart file where there are various other extensions that couldn't find a better place. If you put it there, then we'd have to import that into main.dart, but that is also perfectly ok. Take your pick!

[andrei-toterman]

What is the purpose of this change? 'Toggle window' is supposed to hide the window if it's visible and show it if it's not.