Closed Bug 881073 Opened 11 years ago Closed 10 years ago

Remove "Click here to..." clutter from message list column header tooltips ("Click here to sort by recipient" etc.) for ux-consistency with other tooltips

Categories

(Thunderbird :: Folder and Message Lists, defect)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 29.0

People

(Reporter: thomas8, Assigned: zach.x.nickell)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Keywords: ux-consistency, Whiteboard: [good first bug])

Attachments

(1 file, 3 obsolete files)

+++ This bug was initially created as a clone of Bug #522886 +++

Can we clean up those tooltips to get rid of that useless "Click here to" part of message list column header tooltips like:

"Click here to sort by recipient" etc., so that they become just plain vanilla
"Sort by recipient"?

Think about it, the "Click here to" part is just a nuisance:

a) no informational value at all: we could add that to any tooltip in the whole UI: "Click here to get new messages", "Click here to print this message" and so on... so what's the point of having it here? (ux-consistency)

b) "Click here to" just distracts from the real action which is "Sort by ..." (so it actually makes it harder to parse the tooltip for the relevant info)

c) It's common knowledge that tables can be resorted by clicking their headers; users who don't have that knowledge are unlikely to hover the column header anyway ("Click here to" won't help them to find this click target)

d) iow, tooltip will only show for users who have already found the click target (column header), and then we already have plenty of ux-feedback that you can "click here": column header is highlighted on hover, and the very fact that we show a tooltip with an action verb like "Sort by..." *always* implies that this an UI element where clicking will trigger that action...

(N.B. we've tried that road of being over-explicit in our UI before in old quick search where we had "Filter for To", "Filter for CC" etc. on the dropdown, which was equally useless when there's a clear context of magnifier icon and a filter searchbox... Alas, localizations didn't even bother to pick that up, and it's all gone now...)
(In reply to Blake Winton (:bwinton) from comment #14)
> We should probably remove the "Click here to" from the tooltips, but I don't
> particularly care whether you do it here, or make Thomas file a new bug.  ;)

That looks like an ui-review+ for addressing this little nit.
Whiteboard: [good first bug][has ui-review+]
(In reply to Thomas D. from comment #1)
> (In reply to Blake Winton (:bwinton) from comment #14)

That's bug 522886 comment 14, sorry

> > We should probably remove the "Click here to" from the tooltips, but I don't
> > particularly care whether you do it here, or make Thomas file a new bug.  ;)
> 
> That looks like an ui-review+ for addressing this little nit.
Whiteboard: [good first bug][has ui-review+] → [good first bug][has ui-review+][string change only]
Whiteboard: [good first bug][has ui-review+][string change only] → [good first bug][string change only][has ui-review+]
Assignee: nobody → syshagarwal
Attachment #810826 - Flags: review?(bwinton)
Comment on attachment 810826 [details] [diff] [review]
This patch removes the column header tooltips

Review of attachment 810826 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Zach,

thank you for the patch!  I have one fairly minor comment below, but I'm confident that you can fix the stuff I mentioned, and the next version of the patch should pass review without any problems.  :)

::: mail/locales/en-US/chrome/messenger/messenger.dtd
@@ -660,5 @@
>  <!ENTITY idColumn.label "Order Received">
>  <!ENTITY attachmentColumn.label "Attachments">
>  
> -<!-- Thread Pane Tooltips -->
> -<!ENTITY columnChooser.tooltip "Click to select columns to display">

It looks like we're still using some of these in mail/base/content/SearchDialog.xul and suite/mailnews/threadPane.xul, so we probably don't want to remove them from this file, but…

The ui-review was for removing the "Click to" at the start of each message.  I don't think we should remove the tooltips entirely, since the extra reinforcement of explaining what the control will do is relatively small, and is likely to help people who are exploring the UI, particularly for columns where it's not entirely clear what the icon represents.

So, if you wanted to change just the strings, and leave the tooltips, I think that would be a better solution here.
Attachment #810826 - Flags: review?(bwinton) → review-
Sorry about that last patch! I took the word 'remove' and ran with it a bit too far.
Attachment #810942 - Flags: review?(bwinton)
Attachment #810826 - Attachment is obsolete: true
Comment on attachment 810942 [details] [diff] [review]
This patch removes "Click to" from the column header tooltips

Review of attachment 810942 [details] [diff] [review]:
-----------------------------------------------------------------

Unfortunately we always have to change the localization keys when the text change. Otherwise localizers can't keep up :(

(So e.g. columnChooser.tooltip needs to change to columnChooser2.tooltip or something, and all the places it's used need to also be updated.)
Attachment #810942 - Flags: review?(bwinton) → review-
Assignee: syshagarwal → zach.x.nickell
Hi Zach, thanks for the work. But next time when you work on a bug that is already assigned to somebody, please first ask if it is OK to take it or there is a reason it is not already being worked on. This time Suyash didn't start to work on this because it is waiting on bug 522886. Checking this patch in first will break his patch in 522886. But fortunately it is not that fatal this time so I think Suyash will be OK if you to proceed with your patch.
I emailed Suyash first before I started submitting anything. Sorry about that, I hope I'm not causing too much trouble. This is my first time in open source, so I'm a bit new. I really appreciate how helpful you guys are!
(In reply to Zach Nickell from comment #8)
> I emailed Suyash first before I started submitting anything. Sorry about
> that, I hope I'm not causing too much trouble. This is my first time in open
> source, so I'm a bit new. I really appreciate how helpful you guys are!

Ya, its okay :) as aceman sir said above.
And congrats you are about to fix your first bug :)
Attachment #810942 - Attachment is obsolete: true
Attachment #811268 - Flags: review?(mkmelin+mozilla)
Attachment #811268 - Flags: review?(bwinton)
Comment on attachment 811268 [details] [diff] [review]
Removes "Click to" from the column header tooltips, changes associated localization keys

Review of attachment 811268 [details] [diff] [review]:
-----------------------------------------------------------------

Alright, I've got some minor things to fix in this patch, but I'm going to say r=me, because I think you can fix them without me re-reviewing the changes.  :)

Thanks, and congratulations!
Blake.

::: mail/locales/en-US/chrome/messenger/SearchDialog.dtd
@@ +59,5 @@
>  <!ENTITY idColumn.label "Order Received">
>  
>  <!-- Thread Pane Tooltips -->
> +<!ENTITY columnChooser2.tooltip "Select columns to display">
> +<!ENTITY threadColumn2.tooltip "Display message threads"> 

I think we can take this opportunity to remove the trailing space from this line.

::: mail/locales/en-US/chrome/messenger/messenger.dtd
@@ +660,5 @@
>  <!ENTITY idColumn.label "Order Received">
>  <!ENTITY attachmentColumn.label "Attachments">
>  
>  <!-- Thread Pane Tooltips -->
> +<!ENTITY columnChooser2.tooltip "Select columns to display">

Something's gone slightly wrong with this part of the patch…  ;)

::: mailnews/base/src/nsMessenger.cpp
@@ +1,4 @@
>  /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

I would probably leave this blank line in, just because it's not hurting anything, and makes the patch a little harder to read.

::: suite/locales/en-US/chrome/mailnews/threadpane.dtd
@@ +23,5 @@
>  <!ENTITY idColumn.label "Order Received">
>  
>  <!--Tooltips-->
> +<!ENTITY columnChooser2.tooltip "Select columns to display">
> +<!ENTITY threadColumn2.tooltip "Display message threads"> 

Let's remove the blank space at the end of this line, too.

Oh, and since you're changing files in the suite/ directory, we'll need to get a SeaMonkey reviewer to check it over, too.  Using my secret powers, I'll pick Neil!  ;)
Attachment #811268 - Flags: review?(neil)
Attachment #811268 - Flags: review?(bwinton)
Attachment #811268 - Flags: review+
(In reply to Blake Winton (:bwinton) from comment #11)
> Comment on attachment 811268 [details] [diff] [review]
> Removes "Click to" from the column header tooltips, changes associated
> localization keys
> 
> Review of attachment 811268 [details] [diff] [review]:
> -----------------------------------------------------------------

Thanks! I can fix that. :)
I was reading mailnews/base/src/nsMessenger.cpp and must have removed the line by mistake. My apologies! I'll have that corrected as well.

One small question: Since this attachment has 'review+' and pending 'review?' flags on it, how should I go about making changes on this patch? Should I attach a new patch with these corrections?
Hi Zach, thanks for picking up this little bug which I reported :)

(In reply to Zach Nickell from comment #12)
> One small question: Since this attachment has 'review+' and pending
> 'review?' flags on it, how should I go about making changes on this patch?
> Should I attach a new patch with these corrections?

Yes please. Just set review?neil@httl.net on your new patch, and remove it from the old one if you can. You should make reference to Blake's comment 11 in the accompanying comment. You might choose to NOT obsolete your previous patch yet so that people can see the review+ flag there until it will reappear on your new patch.
Comment on attachment 811268 [details] [diff] [review]
Removes "Click to" from the column header tooltips, changes associated localization keys

Review of attachment 811268 [details] [diff] [review]:
-----------------------------------------------------------------

bwinton already reviewed
Attachment #811268 - Flags: review?(mkmelin+mozilla)
Corrections made based on Blake's comment 11
Attachment #811563 - Flags: review?(neil)
Attachment #811268 - Flags: review?(neil)
Attachment #811563 - Flags: review?(neil) → review?(mnyromyr)
Comment on attachment 811563 [details] [diff] [review]
Removes "Click to" from the column header tooltips, changes associated localization keys  v2

Ian might be able to review if Karsten hasn't got time...
Attachment #811563 - Flags: review?(iann_bugzilla)
Comment on attachment 811563 [details] [diff] [review]
Removes "Click to" from the column header tooltips, changes associated localization keys  v2

r=me
Attachment #811563 - Flags: review?(mnyromyr)
Attachment #811563 - Flags: review?(iann_bugzilla)
Attachment #811563 - Flags: review+
Whiteboard: [good first bug][string change only][has ui-review+] → [good first bug]
Status: NEW → ASSIGNED
Attachment #811268 - Attachment is obsolete: true
https://hg.mozilla.org/comm-central/rev/3201941f3f95
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
There is similar cleanup possible in the Calendar at http://mxr.mozilla.org/comm-central/source/calendar/locales/en-US/chrome/calendar/calendar.dtd . Florian, please file the bug if you think it should be done there to.
Flags: needinfo?(florian)
Sorry, I meant Fallen for the Calendar :)
Flags: needinfo?(florian) → needinfo?(philipp)
Filed bug 983969 with patch, thanks for the hint.
Flags: needinfo?(philipp)
You need to log in before you can comment on or make changes to this bug.