Bookmarks Bar buttons should darken text on mousesdown

RESOLVED FIXED in Camino1.5

Status

Camino Graveyard
Bookmarks
--
enhancement
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: Andrew Collins, Assigned: Stuart Morgan)

Tracking

({fixed1.8.1.1})

unspecified
Camino1.5
PowerPC
Mac OS X
fixed1.8.1.1

Details

(Whiteboard: [Good First Bug])

Attachments

(1 attachment, 2 obsolete attachments)

v3
5.60 KB, patch
Simon Fraser
: review+
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b3) Gecko/20050708 Camino/0.9a1+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b3) Gecko/20050708 Camino/0.9a1+

Mouseovers in the Bookmarks Bar currently provide no visual feedback to the
user.  Mouse clicks only provide minimal visual feedback via the favicon or
folder icon.  Usability would be enhanced if the Bookmarks Bar provided more
feedback to the user by having a highlighted state for mouseovers and mouse clicks,

Reproducible: Always

Steps to Reproduce:
1.  Open Camino
2.  No step 2
i say WONTFIX, what say you simon?

Comment 2

12 years ago
I don't want mouseover (since toolbar buttons don't have that), but I agree the
click feedback could be more. Maybe darken the text like toolbar buttons do?

Comment 3

12 years ago
(In reply to comment #2)
> I don't want mouseover (since toolbar buttons don't have that), but I agree the
> click feedback could be more. Maybe darken the text like toolbar buttons do?

The toolbar buttons actually get a small shadow under their text when clicked.
If we were 10.3+, we could use NSShadow, but there are also some CoreGraphics
calls that can accomplish the same effect on 10.2.
Confirm this for improving shadows on BM bar text for clicks only?

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Text in Bookmarks Bar should have mouseover and mouseclick visual effects → Bookmarks Bar buttons should darken text on mousesdown
Blocks: 311840
Target Milestone: --- → Camino1.1

Comment 5

11 years ago
I agree with the original reporter: hover highlighting would be valuable for bookmark bar items.

Since favicons are only 16x16, there could be dozens of them, and they can move around at the user's whim, bookmark target acquisition is significantly more problematic than for the four standard toolbar buttons.  I think hover highlighting would be a definite usability improvement.

I like the way drop targets are highlighted when you drag a URL from the URL window to a folder in the bookmarks bar, although I'd imagine something more subtle for hover highlighting.

Updated

11 years ago
Whiteboard: [Good First Bug]
(In reply to comment #3)
> The toolbar buttons actually get a small shadow under their text when clicked.
> If we were 10.3+, we could use NSShadow, but there are also some CoreGraphics
> calls that can accomplish the same effect on 10.2.

Hey hey... we're 10.3+ now. Make it happen, yo.

Comment 7

11 years ago
->aschulm per IRC, since he likes this stuff.
Assignee: mikepinkerton → aschulm

Updated

11 years ago
Blocks: 340099
QA Contact: bookmarks
(Assignee)

Comment 8

11 years ago
Created attachment 243067 [details] [diff] [review]
fix

Swaps in a modified attributedTitle during highlighted drawing; this lets us get the shadow without having to custom-draw the whole cell.
Assignee: aschulm → stuart.morgan
Status: NEW → ASSIGNED
Attachment #243067 - Flags: review?
(Assignee)

Comment 9

11 years ago
Created attachment 243123 [details] [diff] [review]
with SDK workaround

Should actually build with the current setup.
Attachment #243067 - Attachment is obsolete: true
Attachment #243123 - Flags: review?
Attachment #243067 - Flags: review?

Comment 10

11 years ago
Comment on attachment 243067 [details] [diff] [review]
fix


>Index: src/extensions/DraggableImageAndTextCell.mm
>===================================================================


>+- (void)dealloc
>+{
>+  [mSavedStandardTitle release];
>+}

Missing [super dealloc];


>Index: src/browser/BrowserWindowController.mm
>===================================================================
>RCS file: /cvsroot/mozilla/camino/src/browser/BrowserWindowController.mm,v
>retrieving revision 1.281
>diff -u -8 -r1.281 BrowserWindowController.mm
>--- src/browser/BrowserWindowController.mm	11 Oct 2006 16:42:29 -0000	1.281
>+++ src/browser/BrowserWindowController.mm	22 Oct 2006 05:25:31 -0000
>@@ -1133,16 +1133,17 @@
>     NSButton* button = [self createToolbarPopupButton:toolbarItem];
>     [toolbarItem setLabel:NSLocalizedString(@"Back", nil)];
>     [toolbarItem setPaletteLabel:NSLocalizedString(@"Go Back", nil)];
> 
>     NSSize size = NSMakeSize(32., 32.);
>     NSImage* icon = [NSImage imageNamed:@"back"];
>     [icon setScalesWhenResized:YES];
>     [button setImage:icon];
>+    [button setTitle:nil];
>     
>     [toolbarItem setView:button];
>     [toolbarItem setMinSize:size];
>     [toolbarItem setMaxSize:size];
>     
>     [button setTarget:self];
>     [button setAction:@selector(back:)];
>     [toolbarItem setTarget:self];
>@@ -1170,16 +1171,17 @@
>     NSButton* button = [self createToolbarPopupButton:toolbarItem];
>     [toolbarItem setLabel:NSLocalizedString(@"Forward", nil)];
>     [toolbarItem setPaletteLabel:NSLocalizedString(@"Go Forward", nil)];
> 
>     NSSize size = NSMakeSize(32., 32.);
>     NSImage* icon = [NSImage imageNamed:@"forward"];
>     [icon setScalesWhenResized:YES];
>     [button setImage:icon];
>+    [button setTitle:nil];

Why do you need these changes?
Attachment #243067 - Flags: review-
(Assignee)

Comment 11

11 years ago
Created attachment 243284 [details] [diff] [review]
v3

reply to comment #10)
> Missing [super dealloc];

Whoops.

> Why do you need these changes?

AppKit weirdness.  The forward and back toolbar button cells are DraggableImageAndTextCells, and although their imagePositions are set to NSImageOnly, as soon as you call setAttributedTitle: (even with just [self attributedTitle]) it starts drawing the title over the buttons in the toolbar.  The buttons come with a default title of "Button", so I needed to remove it.  That wasn't a good place for the fix though; I've moved it into the init of the helper method that sets them up.
Attachment #243123 - Attachment is obsolete: true
Attachment #243284 - Flags: review?(sfraser_bugs)
Attachment #243123 - Flags: review?
(Assignee)

Updated

11 years ago
No longer blocks: 340099
(Assignee)

Updated

11 years ago
Attachment #243284 - Flags: review?(sfraser_bugs) → review?

Updated

11 years ago
Attachment #243284 - Flags: review? → review+
(Assignee)

Updated

11 years ago
Attachment #243284 - Flags: superreview?(mikepinkerton)
Comment on attachment 243284 [details] [diff] [review]
v3

+#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_3
+@class NSShadow;
+#endif

still needed after the SDK switch?

sr=pink
Attachment #243284 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 13

11 years ago
Checked in on trunk and MOZILLA_1_8_BRANCH. I left the SDK thing since branch still hasn't switched and it'll be easier to remove all that junk later if it's not different on branch and trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
(Assignee)

Comment 14

11 years ago
Clearly I shouldn't try to outsmart the 10.2.8 SDK, as it seems to be smarter than I am.  I could have sworn this was compiled against it, but apparently not. Backed out on branch until the SDK changeover.

I removed the VERSION_MAX_ALLOWED on the trunk since there will be no need for it to exist anywhere.
Status: RESOLVED → REOPENED
Keywords: fixed1.8.1.1
Resolution: FIXED → ---

Updated

11 years ago
Depends on: 359218

Updated

11 years ago
No longer blocks: 311840
(Assignee)

Comment 15

11 years ago
Checked in on MOZILLA_1_8_BRANCH.
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Keywords: fixed1.8.1.1
You need to log in before you can comment on or make changes to this bug.