Tooltips for Download toolbar buttons should update to reflect the number of items selected

RESOLVED FIXED in Camino1.5

Status

Camino Graveyard
Downloading
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: froodian (Ian Leue), Assigned: froodian (Ian Leue))

Tracking

({fixed1.8.1.1, polish})

Trunk
Camino1.5
PowerPC
Mac OS X
fixed1.8.1.1, polish

Details

(Whiteboard: [Good First Bug] [String changes in comment 2])

Attachments

(1 attachment, 1 obsolete attachment)

8.16 KB, patch
Stuart Morgan
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
Currently we use "download(s)" and "items(s)" to talk about selected items in the download list.  Instead, the tooltips should reflect whether or not there are multiple items selected (so they'd update from "download" to "downloads" when you selected a second item).
(Assignee)

Updated

11 years ago
Whiteboard: [Good First Bug]
(Assignee)

Comment 1

11 years ago
Created attachment 247357 [details] [diff] [review]
Patch
Assignee: nobody → stridey
Status: NEW → ASSIGNED
Attachment #247357 - Flags: review?(bugzilla)
(Assignee)

Comment 2

11 years ago
This will need string changes such that the block for the toolbar items reads

"dlRemoveButtonTooltip" = "Remove selected item from the list";
"dlRemoveButtonTooltipPlural" = "Remove selected items from the list";
"dlRevealButtonTooltip" = "Show selected file in Finder";
"dlRevealButtonTooltipPlural" = "Show selected files in Finder";
"dlOpenButtonTooltip" = "Open selected file";
"dlOpenButtonTooltipPlural" = "Open selected files";
"dlCancelButtonTooltip" = "Cancel selected download";
"dlCancelButtonTooltipPlural" = "Cancel selected downloads";
"dlPauseButtonTooltip" = "Pause selected download";
"dlPauseButtonTooltipPlural" = "Pause selected downloads";
"dlResumeButtonTooltip" = "Resume selected download";
"dlResumeButtonTooltipPlural" = "Resume selected downloads";
"dlTrashButtonTooltip" = "Move selected file to Trash";
"dlTrashButtonTooltipPlural" = "Move selected files to Trash";
"dlCleanUpButtonTooltip" = "Remove all inactive items from the list";
Whiteboard: [Good First Bug] → [Good First Bug] [String changes in comment 2]

Comment 3

11 years ago
Comment on attachment 247357 [details] [diff] [review]
Patch

Looks reasonable. Didn't test it, though.
Attachment #247357 - Flags: review?(bugzilla) → review+
(Assignee)

Updated

11 years ago
Attachment #247357 - Flags: superreview?(stuart.morgan)

Comment 4

11 years ago
Comment on attachment 247357 [details] [diff] [review]
Patch

>+  if ([itemIdentifier isEqualToString:@"cleanupbutton"])
>+    toolTip = NSLocalizedString(@"dlCleanUpButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"removebutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlRemoveButtonTooltipPlural", nil) : NSLocalizedString(@"dlRemoveButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"cancelbutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlCancelButtonTooltipPlural", nil) : NSLocalizedString(@"dlCancelButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"revealbutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlRevealButtonTooltipPlural", nil) : NSLocalizedString(@"dlRevealButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"openbutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlOpenButtonTooltipPlural", nil) : NSLocalizedString(@"dlOpenButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"movetotrashbutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlTrashButtonTooltipPlural", nil) : NSLocalizedString(@"dlTrashButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"pauseresumebutton"]) {
>+    if ([self shouldAllowResumeAction])
>+      toolTip = plural ? NSLocalizedString(@"dlResumeButtonTooltipPlural", nil) : NSLocalizedString(@"dlResumeButtonTooltip", nil);
>+    else
>+      toolTip = plural ? NSLocalizedString(@"dlPauseButtonTooltipPlural", nil) : NSLocalizedString(@"dlPauseButtonTooltip", nil);
>+  }

Move the ternary inside the NSLocalizedString call.

>+  [theItem setToolTip:toolTip];

If somehow this list and the toolbar items got out of sync at all, this would be calling setToolTip: with an undefined argument and cause potentially hard-to-debug behavior.
Attachment #247357 - Flags: superreview?(stuart.morgan) → superreview-
(Assignee)

Comment 5

11 years ago
Created attachment 247415 [details] [diff] [review]
Patch

Is this nil assignment/check enough to ensure that we're never calling setToolTip when we shouldn't be?
Attachment #247357 - Attachment is obsolete: true
Attachment #247415 - Flags: superreview?(stuart.morgan)

Comment 6

11 years ago
Comment on attachment 247415 [details] [diff] [review]
Patch

>+  if (toolTip != nil)

if (toolTip)

sr=smorgan with that change.
Attachment #247415 - Flags: superreview?(stuart.morgan) → superreview+
(Assignee)

Comment 7

11 years ago
Chickened on 1.8branch and trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
Moving fixed "1.2" bugs to 1.1 where they were really fixed. Filter on CaminoFixed1.1 for bugmail purposes.
Target Milestone: Camino1.2 → Camino1.1
You need to log in before you can comment on or make changes to this bug.