#15 UI issues

[redtide]

• The main window position (and maybe also the size) is/are not correctly applied on the UI from the configuration file. Trying to debug seems like the data is restored but apparently SetSize(x, y, w, h) seems to do nothing.
  I also noticed that the position is not saved when moving the window before closing the application, I added a OnMove(wxMoveEvent&) and it works, but still doesn't help for the above issue.

• When using a dark theme it's hard to see the UI information, this could be solved by removing the hardcoded background color (#f4f2ef).

• The loops grid context menu appears in the wrong position.

IDK if some of these issues might be caused by the window manager, I'm using Archlinux, LXQt DE with Openbox WM.

[Lars Palo]

• Popup menu position corrected in r63.
• wxMoveEvent is only used by MSW so it's not really useful. I have another simple solution for that.
• I'll work on the background and test some things - other issues occur if I just remove it. Changing to a dark theme on Ubuntu 20.04 destroy the listctrl readability completely.

[redtide]

wxMoveEvent is only used by MSW so it's not really useful.

this doesn't sound to be true, I recall I used it in the past and as I said in the OP it worked, I'm using Archlinux, it saves the position when moved before closing, though it doesn't fix the original issue.

Changing to a dark theme on Ubuntu 20.04 destroy the listctrl readability completely.

Dark theme is something that doesn't work but not even a problem on Windows, at least up to 10, but it's used on Linux and macOS. Usually the solution is not to use custom colors and leave the native/default ones, except for some cases when using wx 3.0.x, 3.1 has some fixes for this, though it's still in development.
I wonder why a wxGrid was used instead wxListView for loops and cues,
Here a screenshot after removing hardcoded colors, so it seems there is some problem with some dark theme.

[Lars Palo]

this doesn't sound to be true

For the stable 3.0.5 it is, read the documentation. For dev. it's true for child windows under wxGTK, not top level. Anyway, there's a much easier fix for this that don't depend on any event detection.

wxGrid was selected because it's the natural choice. From the manual: wxGrid and its related classes are used for displaying and editing tabular data. The listctrl (which the listview uses in background) was selected for displaying the file list - and it has its own problems.

With a dark theme it looks like this with removed background in Ubuntu 20.04. To me it's not acceptable.

[redtide]

From your screenshot I guess that is the best you can do with 3.0.x, which lacks some fixes in 3.1.x for dark themes, in my case it works fine, probably the theme I'm using takes more the text color into account than others, IDK.

Anyway I tested your last changes in r64 and the colors and the popup menu are OK here, though the window position is not working, and I don't understand why, though I haven't dig into the issue much.

About the wxEVT_MOVE family looks like there were some error in documentation they fixed later: in the detailed general description they said that all wxEVT_MOVE events are wxMSW only but below they are 3 of 4 wxMSW, wxEVT_MOVE not, which I used and worked on 3.0.5 (Archlinux package, not self built) for the wxFrame, so as you and trunk docs say for the top level windows.

Meanwhile I notice some annoying warnings about unused variables and a missing curly brackets, if you want I could provide a patch? I forgot how to work with SVN ^^''
But I need to ask first: is there some reason to use FindWindow() over event.GetEventObject()?
And can we use static_cast instead C casts?

[Lars Palo]

I've fixed the compiler warnings for now.

With LA built on Ubuntu with wx 3.0.5 the window positioning and size is correctly stored and restored. I really suspect a wx issue/bug why it's not for you.

Using the ID with FindWindow() vs event.GetEventObject() is really only needed outside direct events (when handling more than one control) or if there possibly could be any confusion about which control should be dealt with. I don't really see any reason to worry about it though.

Changing the way of casting is certainly possible, though I really don't feel any pressing need for it either. Most of the reasons why it's necessary to cast to begin with is differences in underlying libs and how the type of wx controls are done. I've tried to make sure that the values should be valid at all times even without any casting, but... In the ideal world the different libs would have the same type for the same variable and no casting would be needed as one could have specified to wx what type is expected for the control - but we're not living in an ideal world.

[redtide]

I asked about static casts and if to use get event object to know if I could use them to make a patch to fix the warnings issue because of the unused event argument in event handlers.
static casts costs nothing since it's a compile time thing; c style casts are discouraged in c++, then using them for get event object, which I see is always same as the requested widget, would silent the unused argument warnings. Otherwise just remove the event argument from the function would do.