Closed Bug 362541 Opened 18 years ago Closed 18 years ago

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

Categories

(Camino Graveyard :: Downloading, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Camino1.5

People

(Reporter: froodian, Assigned: froodian)

Details

(Keywords: fixed1.8.1.1, polish, Whiteboard: [Good First Bug] [String changes in comment 2])

Attachments

(1 file, 1 obsolete file)

8.16 KB, patch
stuart.morgan+bugzilla
: superreview+
Details | Diff | Splinter Review
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).
Whiteboard: [Good First Bug]
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → stridey
Status: NEW → ASSIGNED
Attachment #247357 - Flags: review?(bugzilla)
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 on attachment 247357 [details] [diff] [review]
Patch

Looks reasonable. Didn't test it, though.
Attachment #247357 - Flags: review?(bugzilla) → review+
Attachment #247357 - Flags: superreview?(stuart.morgan)
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-
Attached patch PatchSplinter Review
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 on attachment 247415 [details] [diff] [review]
Patch

>+  if (toolTip != nil)

if (toolTip)

sr=smorgan with that change.
Attachment #247415 - Flags: superreview?(stuart.morgan) → superreview+
Chickened on 1.8branch and trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 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.

Attachment

General

Created:
Updated:
Size: