search

sillsdev / l10nsharp

A .NET localization library for Windows Forms applications.
3 stars 10 forks source link

Added optional owner parameter to methods that show dialog boxes #100

Closed tombogle closed 2 years ago

tombogle commented 2 years ago

This causes them to be displayed centered on a parent window (and not appear off-screen).

This change is Reviewable

tombogle commented 2 years ago

src/L10NSharp/ILocalizationManager.cs line 95 at r1 (raw file):

Previously, andrew-polk wrote…
I don't know about this myself, but I thought you (or @ermshiperete?) told me that adding an optional parameter is still a breaking change. I thought I was told that you have to add an override to make it truly not breaking. But you can also just make it major change if you're trying to get this in before you publish 5.0 (since it will still just be 5.0).

Yes, that's true. I wondered about it when I went to push the change today (which I had left ready to push for some reason), but I was foolishly too lazy check.

tombogle commented 2 years ago

src/L10NSharp/UI/TipDialog.cs line 28 at r2 (raw file):

Previously, ermshiperete (Eberhard Beilharz) wrote…
Is this method expected to be called from clients, or should this be `internal` ?

I was thinking it should be public since the existing Show method was. I made this new method so as to avoid having the identical hard-coded string in multiple places. But now that I consider further, I guess it's probably not very likely that an application would want to show this particular tip in any other context. In actual fact, based on a search in Github, it does not appear that any of our projects use the TipDialog to display other tips at all. So probably we could just make the whole class internal. But I guess it's harmless and it does seem to be intended for other uses.