Improve focus handling in Connect dialog - #3859
Conversation
softins
added
the
backport_required
| if ( obj == cbxServerAddr && event->type() == QEvent::FocusIn ) | ||
| { | ||
| // remove selection in the server list (if any) | ||
| lvwServers->clearSelection(); |
There was a problem hiding this comment.
I don't actually like this.
The problem is that hitting connect when focus was in the Server text box should use that value.
Moving focus from the server list to any other field should have no effect on the selected server -- at that point, you have no idea of the user intent.
Why should moving to one field (Server) behave differently from another (Filter)?
There was a problem hiding this comment.
The problem is that hitting connect when focus was in the Server text box should use that value.
Exactly. That's what @dingodoppelt was saying in #3857, and what does not always happen currently.
Moving focus from the server list to any other field should have no effect on the selected server -- at that point, you have no idea of the user intent.
Except that if the field they have clicked on is the Server Address combo box, it's reasonable to assume that it contains the server they want to connect to.
Why should moving to one field (Server) behave differently from another (Filter)?
Because by moving to the Server Address field, the user is desiring to use its value. The connect button won't use that value if there is still a server selected in the list box.
Currently, the only ways to deselect the selected server in the server list are either to go to a different directory, or to edit or change the value in the Server Address field. Just selecting the Server Address field when it already contains the desired value from before does not currently allow its value to be used, if there is still a server selected in the list. This PR fixes that by interpreting putting focus on the Server Address field as intent that its value should be used when connecting, and therefore deselecting any server selected in the list.
If you need convincing, try this version out and see!
There was a problem hiding this comment.
I think it would be far better just to check focus correctly, rather than changing the natural behaviour of a widget (OK - whichever, it's changing Qt's implementation).
Being able to tab from the server list into the Server field would make more sense than this -- and that action shouldn't change the item selected. If you carry on tabbing back to the server list, the same server should be selected.
The change proposed here would break that. It's prioritising mouse navigation over keyboard navigation.
There was a problem hiding this comment.
I think it would be far better just to check focus correctly, rather than changing the natural behaviour of a widget (OK - whichever, it's changing Qt's implementation).
The problem is, by the time the Connect button click is being processed, I believe that button has focus, and you have lost the information about which widget had focus previously.
It's not "changing QT's implementation", it's making the perfectly reasonable UX design decision that the user, by choosing to select the Server Address box, is implying that that is now what they are interested in, and are no longer interested in the server they had previously clicked on in the list, so it can be deselected. That is already the behaviour if the user changes what is in the Server Address box. But they might not want to, if it already contains the desired server.
Being able to tab from the server list into the Server field would make more sense than this -- and that action shouldn't change the item selected. If you carry on tabbing back to the server list, the same server should be selected.
The Tab behaviour is a separate issue that warrants investigation and its own PR. Currently Tab moves between servers in the list instead of moving from the server list to the combo box.
The change proposed here would break that. It's prioritising mouse navigation over keyboard navigation.
No, the keyboard navigation is already broken. This PR doesn't change that, it just addresses and improves a specific shortcoming of the current UX design.
2 participants
Short description of changes
Deselects any selected server in the server list when the Server Address combo box gains focus. This ensures that clicking
Connect when the Server Address has focus will connect to the expected server, even if a server had previously been
selected in the server list.
CHANGELOG: Client: make sure Connect uses the expected server if Server Address box is selected.
Context: Fixes an issue?
Fixes #3857
Does this change need documentation? What needs to be documented and how?
No
Status of this Pull Request
Ready to merge. Tested on Linux and Windows..
What is missing until this pull request can be merged?
Nothing
Checklist