Fix MessageBox focus issues v3

[Miepee]

Description

Fixes #878, by using a custom MessageBox wrapper which in general makes the use of messageBoxes much cleaner.

Caveats

None

Notes

• Scripts have not been touched yet.
• Obsoletes Fix MessageBox focus issues v2 #936
• A few instances of messageBoxes have been kept as-is in order not to break stuff.

[github-actions]

Download the artifacts for this pull request here:

GUI:

• GUI-windows-latest-isBundled-false-isSingleFile-false
• GUI-windows-latest-isBundled-false-isSingleFile-true

CLI:

• CLI-macOS-latest-isBundled-false
• CLI-ubuntu-latest-isBundled-false
• CLI-windows-latest-isBundled-false

[VladiStep]

I suggest to return static MessageBox methods of MainWindow, but in separate file, and add static MainWindow.Instance property.

For example, MainWindow.ShowMessage() should call Instance.ShowMessage() with the parent arguments.
So, you can call MainWindow.ShowMessage() as before (without adding mainWindow for each class).

[VladiStep]

Hmm... actually, MainWindow.Instance also can be used as shorter replacement of Application.Current.MainWindow as MainWindow.
But that's should be done in another PR, as there's a lot places where it is used.

[Miepee]

I suggest to return static MessageBox methods of MainWindow, but in separate file, and add static MainWindow.Instance property.

IMO this complicates things, as you then have basically the same (extension) methods, but in multiple places, where the differences aren't fully clear. (Makes me wish tho that the "extend everything" feature would've been implemented...)
Since MainWindow.Instance is a full replacement for Application.Current.MainWindow as MainWindow, it's possible for classes which only use the main window for a few messagebox calls, to just use that directly, instead of defining it as a private property.

[VladiStep]

I suggest to return static MessageBox methods of MainWindow, but in separate file, and add static MainWindow.Instance property.

IMO this complicates things, as you then have basically the same (extension) methods, but in multiple places, where the differences aren't fully clear. (Makes me wish tho that the "extend everything" feature would've been implemented...) Since MainWindow.Instance is a full replacement for Application.Current.MainWindow as MainWindow, it's possible for classes which only use the main window for a few messagebox calls, to just use that directly, instead of defining it as a private property.

Is it really complicates that?
If some other class wants to show some message, then it should use MainWindow.ShowMessage() (without using mainWindow).
And if it's a Window, then it should use this.ShowMessage().

And each static MessageBox method will contain only one line of code, so adding that is not complicated.

[Miepee]

Is it really complicates that?

You have two classes, with the exact same names. From an outsider view, it's not fully clear why one should use MainWindow.ShowMessage() over MainWindow mw = MainWindow.Instance; mw.ShowMessage() or vice versa.
And from what I can recall, there also aren't a lot of classes, that'd benefit from such a change to begin with.

[VladiStep]

From an outsider view, it's not fully clear why one should use MainWindow.ShowMessage() over MainWindow mw = MainWindow.Instance; mw.ShowMessage() or vice versa.

To make it clear, all the classes except Window ones should use MainWindow.ShowMessage().
So, if someone would want to show a message within MainWindow, they will see that in the class, this.ShowMessage() is used, so they also use that.
And if it's other class, they will see MainWindow.ShowMessage(), and use that instead.
If they try to use this.ShowMessage() there, then it will show an error, and (hopefully) they will guess that it should be MainWindow.ShowMessage().

Also, if they decide to use MessageBox.Show() anyway, code reviewers should notice that and suggest to change that.

...there also aren't a lot of classes, that'd benefit from such a change to begin with.

That's for now - there can be more classes in the future.

This is only a suggestion, so if you don't want to do it, then you can not do that.
Then, I'll still approve it, since I don't have other suggestions/complaints.

[Miepee]

Yeah, it's a nice suggestion, but ATM I don't see the need for this, as well as in the near future.
Not to mention that "reference MainWindow's methods for anything outside of MainWindow, otherwise use the instance" isn't exactly that easy to get across.

[VladiStep]

Then, I'm approving it.