Re: [lmi] Using picker controls for paths

[Greg Chicares]

On 2016-06-04 15:40, Vadim Zeitlin wrote:
> On Wed, 1 Jun 2016 17:54:43 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2016-05-31 20:15, Vadim Zeitlin wrote:
> GC>
> GC> [...]
> GC> >  I don't have anything to add to this list, I just wanted to say that it
> GC> > would be better to choose wxFilePickerCtrl and wxDirPickerCtrl for
> GC> > default_input_filename_ and print_directory_ respectively, instead of 
> plain
> GC> > wxTextCtrl.
> GC> 
> GC> Sounds like a good idea. Since I'm not familiar with those controls,
> GC> could I prevail upon you to add them to lmi's 'transferor.?pp'?
> 
>  I've done this now, please see the simple changes at
> 
>       https://github.com/vadz/lmi/pull/39

Pushed to the "central git repository" if I may call it that.

> GC> (I'm pretty sure we want wxFLP_FILE_MUST_EXIST,
> 
>  I did use it for both controls. Notice that it is still possible to enter
> non-existent path by typing it in manually, but IMHO it's a feature and not
> a bug: if a user _really_ wants to use a file/directory which doesn't exist
> (yet?), we shouldn't prevent him from doing it, the MUST_EXIST flags are
> helpful hints rather than mandatory policy.

Okay. Let me also mention that it honors the value in XML even if no such
file exists. That is, my 'configurable_settings.xml' says:
  <default_input_filename>C:/etc/opt/lmi/default.ill</default_input_filename>
and the file-picker control correspondingly contains
  C:/etc/opt/lmi/default.ill
although no such file exists (as I can see in the file-picker dialog).

If I click "Open" in the pop-up dialog, I get a warning. If I dismiss that
warning and click "Cancel", then my (nonexistent) selection is unchanged.

I consider these behaviors natural and correct.

> GC> but I'm not sure exactly how, say, GetPath() and GetFileName() differ.)
> 
>  They only differ by the type of their return value, the first one returns
> a string while the second one returns a wxFileName (~~ boost::path) object.
> 
>  Actually the first version of my patch uses SetFileName() with the
> following comment:

[...elided: what's in the online repository is the ultimate version...]

> I.e. the idea was that the default path "C:/etc/opt/lmi/default.ill" would
> be shown to the user as more usual "C:\etc\opt\lmi\default.ill". And this
> worked without any problem until I started using the directory picker for
> the print directory as well. At which point I started getting warnings
> about "Contents of more than one control changed" whenever the preferences
> dialog was opened because both the file and directory pickets were changed
> due to the normalization performed by SetFileName()/SetDirName()
> respectively.

Curious. I guess normalizing one control normalized the other. However...

>  I didn't see any simple way to avoid this warning, so, finally, I've just
> abandoned the idea of normalizing the paths when showing them and the final
> version simply shows them as they're, which works.

Okay, that way just works. Sounds good to me.

> Maybe you don't consider
> normalizing the paths be a good idea anyhow, of course, but if you do, I
> could try looking more into making this work without disabling/removing the
> check for the number of changes controls in MvcController::UponUpdateUI().
> Should I do this?

IMO, no. Users these days might not even recognize either directory
separator, '/' or the benighted '\'. They just use directory-picker
controls, which--deliberately, it would seem--show no separator. I don't
think I can even type a directory separator on my company laptop: it
locks me out of CMD.EXE, and this '\' thing always was really a CMD.EXE
and COMMAND.COM oddity. [I used to set SWITCHAR...but when I search for
that online now, I get Swedish results.]

Oh, wait--sure, our end users type directory separators all the time:
  http://www.freedos.org/kernel/config.txt
Typing this into my browser:
  www.freedos.org\kernel\config.txt
teleports me to my ISP's 404 redirector. Thus, lmi works navigates
directories the way a browser does, so this is perfect already.

>  In addition to this, there are two minor cosmetic issues with the dialog
> now. The first one is that the pickers "..." buttons are too close
> together, I would add some vertical separation between these (and hence
> also the other, for consistency) rows. How much of it is needed is a rather
> subjective question, so I think it might be best if you just did whatever
> you consider to be aesthetically pleasing there. But if you'd prefer, I
> could do it too, of course.

Ummm...I'm using 192 DPI, and the ellipsis buttons are enormous. It would
be nice to change wx to make them high-DPI-aware. I don't see any issue
with vertical separation; let's see what Kim says, because her office
computer has the same configuration as a typical end user.

>  The second point is worse, although it only appears when themes are
> enabled, meaning that you won't see it by default. However most (all?)
> users will and if you enable themes, you can see that the background
> between the pickers text and button is grey and not the same white as the
> notebook background around them. I'm not sure why does this happen yet, but
> I'm almost certain it's a bug in wxMSW. And this means that it can only be
> fixed in wxMSW itself.

Fixing it in wxMSW sounds like the right thing to do.

> However a possible workaround in lmi could would be
> to explicitly set the pickers background to white, and, to ensure that this
> looks correctly in all themes and not just the default one, also set the
> white background for the panel itself. This is not ideal because it still
> will override the theme chosen by the user and will look strange when
> themes are disabled

That sounds like a poor use of our limited resources.

Let me mention another oddity that I see. When I click the ellipsis to open
the dir-picker control, I see some furious paint activity. At first, I see
"Desktop" at the top of the directory tree, and a very long vertical
scrollbar (because there's not much to scroll at first). Then directories
are expanded, the tree scrolls, and the scrollbar resizes. My impression
is that paint events weren't suspended when they ought to have been. The
"File | Open" dialog doesn't have this issue.

If convenient, it might be good to block intermediate repainting in wxMSW.
Otherwise, I guess it's not a big problem--perhaps I see it only because
I'm running with emulated graphics in a VM, backed up by a $20 graphics
card.