Skip to content

Improve focus handling in Connect dialog - #3859

Open
softins wants to merge 3 commits into
jamulussoftware:mainfrom
softins:connectdlg-focus-server
Open

Improve focus handling in Connect dialog#3859
softins wants to merge 3 commits into
jamulussoftware:mainfrom
softins:connectdlg-focus-server

Conversation

@softins

@softins softins commented Aug 3, 2026

Copy link
Copy Markdown
Member

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

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@softins softins added the bug Something isn't working label Aug 3, 2026
@github-project-automation github-project-automation Bot moved this to Triage in Tracking Aug 3, 2026
@softins softins added the backport_required A change to main that needs fix on an existing release. label Aug 3, 2026
@softins softins added this to the Release 4.0.0 milestone Aug 3, 2026
Comment thread src/connectdlg.cpp
@github-project-automation github-project-automation Bot moved this from Triage to Waiting externally in Tracking Aug 3, 2026
@pljones

pljones commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

OK, here's an alternative suggestion.

When the connect dialog opens or on change of current Directory through the drop down, set "default Server" to the Directory. (Hm. Not sure on this -- if there's anything in the Server text field, that should still take precedence at this point, I think.)
When a Server in the server list is selected, set "default Server" to the selected Server.
When focus lands on the Server text field and there's a value in it, set "default Server" to that value.

When "Connect" is clicked, connect to "default Server". (Maybe that's not the best name for it, actually.)

(All of those are standard events from the widgets.)

@softins

softins commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

OK, here's an alternative suggestion.

Well, for what it's worth, I don't actually like this at all! Touché!

When the connect dialog opens or on change of current Directory through the drop down, set "default Server" to the Directory. (Hm. Not sure on this -- if there's anything in the Server text field, that should still take precedence at this point, I think.) When a Server in the server list is selected, set "default Server" to the selected Server. When focus lands on the Server text field and there's a value in it, set "default Server" to that value.

When "Connect" is clicked, connect to "default Server". (Maybe that's not the best name for it, actually.)

You probably mean "selectedServer" or "chosenServer", but I don't think this is the right approach anyway. It's certainly not DRY. We already have the information in the widgets.

Certainly, setting to the directory would be wrong if there is a value in the server address field. Many times I open the connect dialog, and immediately click Connect because the server I want is already in the server address field.

But to me it's still logical that if there is a selected item in the server list, clicking Connect should use it, even if the list doesn't have focus. Therefore we need a way to unselect it if the user wants to use a custom server address. That will have been exactly why Volker made the list item be deselected when any change was made to the server address field. I still maintain that it is logical actually to do that as soon as the server Address field gains focus, as that action indicates that server is the server of interest to the user, not the one that was selected in the list.

If your objection boils down to the fact that the selection in the list is forgotten when focussing on the server address field, this can be overcome by saving the identity of the selected item before clearing the selection. Then if and when the server list is again given focus, either by tabbing, or by clicking in an empty area of it (if it's not full), we see there is a saved QTreeWidgetItem* and make it the selection again.

I have added a commit which does exactly that, and have tested it. I really do hope that overcomes your objection.

@pljones

pljones commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Somehow, yes, that works better for me. I'm still not sure about what should happen on switching focus with tab.

I'd never really noticed the old behaviour -- I guess I'd not have a selected server list entry if I was entering text into the Server text box (Would editTextChanged trigger when the selection from the Server dropdown changed? Will FocusIn be triggered for that?)

@softins

softins commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Somehow, yes, that works better for me. I'm still not sure about what should happen on switching focus with tab.

Well at the moment, pressing tab while in a list of servers just moves to the next server in the list, and shift-tab moves to the previous server. Down-arrow and up-arrow to the same.

I think only the arrow keys should move up and down, and tab should move to the next widget (which is the Server Address box), without changing the selection.

I'm not sure how to fix that, so it would need investigation and a separate PR.

I'd never really noticed the old behaviour -- I guess I'd not have a selected server list entry if I was entering text into the Server text box

Yes, I think that's the case for most people, which would be why #3857 has only come to light now.

(Would editTextChanged trigger when the selection from the Server dropdown changed? Will FocusIn be triggered for that?)

I just checked that. In all versions up till now, editTextChanged was triggered when either the text in the box was edited or a new entry was selected from the dropdown. The attached slot cleared the selection in the server list.

In this PR, editTextChanged is no longer needed, since FocusIn is triggered as soon as the cursor keys or a mouse click selects the widget, whether that be to edit the text or to click on the drop-down arrow.

@softins
softins requested a review from dingodoppelt August 5, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport_required A change to main that needs fix on an existing release. bug Something isn't working

Projects

Status: Waiting externally

Development

Successfully merging this pull request may close these issues.

Connect button in connect dialogue doesn't connect to the expected server

2 participants