Ensure a server is always selected when list gains focus - #3872
Conversation
This fixes keyboard navigation when the list only contains one server.
|
Very nice, haven't come across that bug myself, yet, but the fix sounds very sensible. |
dingodoppelt
left a comment
There was a problem hiding this comment.
I just tested it and it works nicely :) Thank you!
| } | ||
| else if ( lvwServers->topLevelItemCount() > 0 ) | ||
| { | ||
| // no saved server: select first item in list for keyboard navigation |
There was a problem hiding this comment.
Shouldn't this select the Directory by default? The default in Jamulus has been that hitting enter with nothing selected connects to the Directory.
There was a problem hiding this comment.
(Although, if the filter is in used, the Directory might not be visible -- still the default, even in that case, would have been for enter to join the Directory.)
There was a problem hiding this comment.
That is very old behaviour. If there is anything in the Server Address combo box, then Enter will connect to that server.
In order to test your assertion, I needed to clear my whole list of custom servers to make the combo box empty.
Having done that, I selected a directory (e.g. private.jamulus.io or one of my own), and without clicking on a server in the list I pressed Enter. To my surprise, it connected to the Any Genre 1 directory server! That was the case whichever directory I had selected. I think that is the behaviour you are remembering, and in fact I would argue that behaviour is now obsolete.
So I don't think there is much utility in making the directory the one that is selected, as it might be some way down the list, if its ping is a lot greater than some of its registered servers.
This PR is just to make keyboard navigation work consistently when tabbing into the server list, and the first item in the list is the natural one to start with if you are going to cursor down the list.
There was a problem hiding this comment.
To my surprise, it connected to the Any Genre 1 directory server! That was the case whichever directory I had selected. I think that is the behaviour you are remembering, and in fact I would argue that behaviour is now obsolete.
That seems very weird. For any user not living near Frankfurt that server seems like a bad choice.
There was a problem hiding this comment.
That behaviour dates from b63a625 in 2015, and has been functionally unchanged since.
In CClientDlg::OnConnectDlgAccepted():
Lines 760 to 768 in a24e09c
I think it's time we didn't do that, but instead either ignored the Connect button if a server isn't selected, or displayed a message box. But that's for a separate PR.
There was a problem hiding this comment.
Oh... I didn't realise it was still DEFAULT_SERVER_ADDRESS -- I thought when the custom Directories got added it got updated to the currently selected Directory, as a fallback in cases where nothing was selected.
Anyway, I agree, given the current version isn't adequate, anything is probably an improvement.
This fixes keyboard navigation when the list only contains one server.
Short description of changes
This fixes keyboard navigation when the list only contains one server. When the server list gains focus,
if there was a saved server from previously being in focus, that saved server is restored. In this PR,
if there was no saved server, the first item in the list will be selected, so that keyboard navigation
will work properly.
No changelog entry needed, as this is just a bugfix for #3859
CHANGELOG: SKIP
Context: Fixes an issue?
Fixes #3871
Does this change need documentation? What needs to be documented and how?
No
Status of this Pull Request
Ready
What is missing until this pull request can be merged?
Nothing
Checklist