Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tooltips to connectdlg #3252

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pljones
Copy link
Collaborator

@pljones pljones commented Mar 28, 2024

Short description of changes

Add tool tips to the Connect Dialog to improve useability. I've aimed to keep the same translation strings as "What's this?" uses, as with the other dialogs.

CHANGELOG: Add tooltips to connectdlg

Context: Fixes an issue?

No.

Does this change need documentation? What needs to be documented and how?

No

Status of this Pull Request

Will need translations updated.

What is missing until this pull request can be merged?

Ready to merge.

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

@pljones pljones self-assigned this Mar 28, 2024
@pljones pljones added this to the Release 3.11.0 milestone Mar 28, 2024
@pljones pljones requested review from softins and ann0see March 30, 2024 08:58
src/connectdlg.cpp Outdated Show resolved Hide resolved
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch from 78ca923 to dc30efc Compare March 31, 2024 17:39
@@ -80,6 +86,11 @@ CConnectDlg::CConnectDlg ( CClientSettings* pNSetP, const bool bNewShowCompleteR
"<br>" + tr ( "Permanent servers (those that have been listed for longer than 48 hours) are shown in bold." ) +
"<br>" + tr ( "You can add custom directories in Advanced Settings." ) );

lvwServers->setToolTip ( "<b>" + tr ( "Server List" ) + ":</b> " +
tr ( "The list of servers registered with the selected directory. "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe even shorter: Just mention the double click to join a server. The other option doesn't really need to be explained and is slower.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server list: Double click a Server to connect.

@softins
Copy link
Member

softins commented Mar 31, 2024

Hmm, I've just tried this out, compiling on my RPi. I must admit I found the tooltips continually popping up in the connect window rather obtrusive, particularly the tooltip within the list of servers, which would pop up anywhere I stopped moving the pointer.

Not sure what to suggest, but to my mind, such tooltips are hand-holding for the unfamiliar user, but obtrusive to anyone who has become familiar with the program, which doesn't take long.

@ann0see
Copy link
Member

ann0see commented Apr 1, 2024

Agree. Other places are maybe not that obstructive. Would you be ok with a shorter version or would you still be bothered by it?

@pljones
Copy link
Collaborator Author

pljones commented Apr 1, 2024

The reason I did this is we have tooltips on all the other dialogs, with the text the same as the "What's this?" text - so I felt we should be consistent in our approach. If it's wrong / unhelpful / distracting here, why is it working elsewhere? Just because those other places aren't used so much? Is it distracting there when they are used?

Do people even think of the "What's this?" function? (It's not particularly common.)

I wonder if the tooltip delay can be made longer?

@softins
Copy link
Member

softins commented Apr 1, 2024

I haven't yet explored to see where else tooltips are used in Jamulus (I was going to do so as part of this review). But to my mind, "What's this?" and tooltips should provide different functions:

  • "What's this?" is a user-initiated action, and it's very appropriate for the pop-up to provide some detailed explanation or description, over several lines.
  • A tooltip pops up uninvited, and is intended to provide a short handy hint as to the identity or function of something that is otherwise non-obvious, such as an unlabelled icon. I don't feel that a box of explanation is appropriate for a tooltip, and it would rapidly become annoying.

I also wouldn't use a tooltip to describe a large area, such as the list of servers in the connect dialog. That was what I found the most annoying: the tooltip popping up every time I moved the mouse within the server list. And the tooltips for the Directory selector and filter box were also more explanatory than tooltippy to my mind, although definitely appropriate for "What's This?"

I understand the desire for consistency, but don't think it should trump benefit and usability. I've been struggling to understand who would benefit from the changes in this particular PR, and for how long.

@pljones
Copy link
Collaborator Author

pljones commented Apr 1, 2024

A tooltip pops up uninvited

To me, it's meant to be quasi-invited - only if I hover long enough for it to be obvious that I'm unsure what the control does were I to use it.

I'd agree they serve different purposes, though. I'd expect "What's this?" to redirect the user to the appropriate paragraph in the manual, to be honest, to avoid redundancy in documentation. The tooltip should be a "hint" as to usage. We don't follow this approach, though.

@ann0see
Copy link
Member

ann0see commented Apr 1, 2024

One of the users I work with really liked the hover feature to explain the usage - so it's valuable. Most likely what's this is much less often used.

@softins
Copy link
Member

softins commented Apr 1, 2024

One of the users I work with really liked the hover feature to explain the usage - so it's valuable.

I can understand this for someone who is new and learning to use Jamulus. But as an experienced user, I would at least want the ability to turn it off, particularly for the wordy ones like in this PR.

@pljones pljones modified the milestones: Release 3.11.0, Release 3.12.0 May 5, 2024
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch from dc30efc to bb96511 Compare May 6, 2024 18:52
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch 2 times, most recently from 0516c5a to 999016a Compare June 15, 2024 13:33
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch 2 times, most recently from f28a2f9 to ea87080 Compare June 30, 2024 08:37
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch from ea87080 to ea42fc6 Compare July 2, 2024 19:26
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch 2 times, most recently from e431c9b to ae65815 Compare July 26, 2024 15:52
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch 3 times, most recently from 5c68ef7 to 9e3ea0f Compare August 7, 2024 15:30
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch 2 times, most recently from 16d738c to ef2d6ca Compare August 31, 2024 14:22
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch from ef2d6ca to 6ae1d27 Compare September 13, 2024 20:05
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch 3 times, most recently from 0340692 to 1ea7d42 Compare September 22, 2024 17:12
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch from 1ea7d42 to 1e9dc29 Compare October 1, 2024 12:11
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch 2 times, most recently from d16ddc1 to c7f5f23 Compare October 29, 2024 18:13
@pljones pljones force-pushed the feature/add-tooltips-to-connectdlg branch from c7f5f23 to 885f04a Compare November 2, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

Successfully merging this pull request may close these issues.

3 participants