support selection and activation in tree controls

[oltolm]

The first commit focuses the selected item, otherwise if you switch focus with tab and use arrow keys it jumps to the the top item.

Second commit adds support for activation in addition to selection. Activation happens with ENTER and double click. This makes keyboard navigation easier and fixes some problems like the inability to select an already selected item and the need to manually focus the HTML window with tab after selecting an item.

[rzvncj]

I'm not sure I completely understand what this is trying to fix, but two observations to begin with.

First, this introduces compile-time warnings which need resolved:

In file included from chmindexpanel.cpp:22:
./chmindexpanel.h: In constructor ‘CHMIndexPanel::CHMIndexPanel(wxWindow*, CHMHtmlNotebook*)’:
./chmindexpanel.h:78:22: warning: ‘CHMIndexPanel::_nbhtml’ will be initialized after [-Wreorder]
   78 |     CHMHtmlNotebook* _nbhtml;
      |                      ^~~~~~~
chmindexpanel.cpp:26:106: warning:   base ‘wxPanel’ [-Wreorder]
   26 | CHMIndexPanel::CHMIndexPanel(wxWindow* parent, CHMHtmlNotebook* nbhtml) : _nbhtml(nbhtml), wxPanel(parent)
      |                                                                                                          ^
chmindexpanel.cpp:26:1: warning:   when initialized here [-Wreorder]
   26 | CHMIndexPanel::CHMIndexPanel(wxWindow* parent, CHMHtmlNotebook* nbhtml) : _nbhtml(nbhtml), wxPanel(parent)
      | ^~~~~~~~~~~~~

But more importantly, this PR breaks existing good behavior: with the current code, if I open a CHM file, navigate to Index and start typing, if any of the list items is a (partial) match I can just press Enter and the associated page is displayed.

With your patch, Enter doesn't work anymore.

[rzvncj]

There's also random whitespace introduced. Please run clang-format -i on your changes before a commit.

[rzvncj]

I would also appreciate it if you could split these into more commits (or even more pull requests), to make reviews and bisection easier.

For example, I could see a separate commit here just doing clean-up work (removing unused headers + switching to wxIMPLEMENT_APP()).

It's easy to consider that obvious when doing the coding, but when you're a reviewer precision and the least amount of noise matters.

Thanks!

[oltolm]

I fixed the problems and added a commit that makes "Enter" focus the HTML window. I wanted to submit it in a later PR, but it probably makes sense in this one.

I can try to explain what I am trying to fix. Here are the use cases.

First use case.

1. User types in the index text box.
2. User presses "Tab" to focus the index list.
3. User presses "Up" or "Down" and the focus jumps to the top of the list.

Second use case.

1. User types in the index text box.
2. User presses "Tab" to focus the index list.
3. User presses "Enter" and nothing happens. If the user clicks or double clicks the selected entry also nothing happens.

Third use case.

1. User types in the index text box.
2. User presses "Enter" and the entry is displayed in the HTML window.
3. User tries to navigate using the keyboard and nothing happens because the HTML window is not selected.

I also plan to submit a PR which make Up and Down keys work in the text box and move the focus to the index list.

[rzvncj]

Thanks! Sorry the review is taking a while, I've been (and still am) extremely busy and it'll take a while yet until I can get to it.

[oltolm]

No problem.

[rzvncj]

I understand that xCHM currently does not focus the displayed HTML on pressing Enter (your 3rd use case). That's arguably useful, but you can also just press Tab one more time after that. Having the focus stay on the index list has the benefit of being able to navigate it with the keyboard as well.

But I can't reproduce your 1st use case on my Arch Linux machine with wxWidgets 3.2.6. The up and down buttons can navigate the index list and the focus does not jump anywhere. What is your setup?

The 2nd case I still don't understand. What is supposed to happen when you press Enter? Just pressing up or down automatically displays the page related to the list item. There's nothing else that a double-click or Enter should do.

Here is a short screencast I've recorded. I'm just typing something in, Tab to switch focus to the list, then up and down. Nothing jumps, and when an item is focused the link associated with it is simply displayed without Enter or double-clicks:

Screencast.From.2024-12-10.21-18-44.mp4

[rzvncj]

One other confusing aspect of this PR is that the subject refers to tree controls, and yet all the enumerated use cases concern the index list control.

[oltolm]

Yes it's the list control and not the tree control.

I tested it on Windows. I know it's not supported, so if it's working for you I'm OK with closing the PR.

[rzvncj]

I'm just trying to understand it so far. I remember a long time ago I used to do Windows builds (with MSVC) and I don't recall problems with the application.

It's possible that in the meantime problems appeared with the Windows build, but I'd like to be able to reproduce them somehow before a potential merge (and I no longer have a Windows build environment, so ideally with some type of Linux build).

Are you using MSVC? Cygwin? Something else?

[oltolm]

I uses MSYS2. Actually the mingw64 environment, which means that it's a pure windows application and not emulated like Cygwin. I have a CMakeLists.txt in my fork for ease of building with CMake.