355323 - Fix followup issues with changing the color of the pop-up blocker bar

Status: RESOLVED FIXED

(Camino Graveyard :: Annoyance Blocking, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

These are issues identified in the review comments for bug 331331 (which we all agreed would be addressed in a follow-up bug):

* Aqua buttons look out-of-place on the curvy background
* Focus rings are very difficult to see on the dark black background
* The close button's pressed state seems indistinguishable from the hover state
* kreeger apparently landed the wrong version of the new popup_blocked_icon.tif warning icon 
* cvs remove popup-blocked.png

I expect we'll see some feedback about the new color/look soon, too ;)  

We'll also have to deal with the issue of tiling once the blocker is able to wrap properly (bug 341967), so a static image background may not be the best way to go here....

Comment 1

[Jon Hicks]

* Aqua buttons look out-of-place on the curvy background

Change to flatter style buttons? Ooh I can hear the sound of air being sucked through teeth! This option was based on the bar that appears in the finder when you have CD or DVD inserted to burn. It does use the more recent aqua style of flatter buttons (http://www.guilford.edu/content/images/admin/helpsheets/its/TigerBurning-6ClickbBurnButtonNoClickScreen.JPG)

However, the issue of the focus ring suggests to me that we should consider another of the options: http://www.hicksdesign.co.uk/clients/camino/PopUpBlocker2.zip

Regarding tiling - can the image be anchored to either center or bottom as in CSS? If so, then all we need to do is create an image that has the background colour at either the top or both top and bottom.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

We discussed the issues relating to "Aqua buttons vs. curved glass bg", indistinguishable close button and tiling problems with using images at this morning's meeting. 

We decided we need to implement a lighter gradient (similar to [1], and do so in code, to solve the buttons/tiling issues.  We're also going to go back to using the standard grey close buttons for discoverability and consistency with the rest of the app.

Jon, could you give us the top and bottom color values from that blue gradient? [1]  Also, for ease and to lessen confusion, could you attach the proper popup_blocked_icon.tif here?

[1] http://adggda.com/~ss/camino/mockups/popupblocker2-toned_down_blue.jpg

Comment 3

[Samuel Sidler (old account; do not CC)]

Not blocking 1.1a1, but we'd definitely take a patch.

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Code-wise, what we need here is a patch that will draw a gradient for the bar, using the colors from Jon's "toned-down blue", and which doesn't make assumptions about the height of the bar.

This bug is blocking making the bar text wrap properly (bug 341967, which has a patch that's almost ready).

Comment 5

[Jon Hicks]

Attachment: Pop up blocked icon (checked in on trunk and MOZILLA_1_8_BRANCH)

The top of the gradient (lighter colour) is #4471C3 (RGB 68,113,195)

The bottom is #29529E (RGB 41,82,158)

Pop up blocked icon attached!

Comment 6

[Sean Murphy]

Taking, as per the 2006-11-18 Status Meeting.  (Thanks kreeger)

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Not blocking a2.

Comment 8

[froodian (Ian Leue)]

Comment on attachment 244876 [details]
Pop up blocked icon (checked in on trunk and MOZILLA_1_8_BRANCH)

cvs removed the old old popup blocked icon and checked the new one in on trunk and MOZILLA_1_8_BRANCH.  Thanks as always, Jon.

Comment 9

[Stuart Morgan]

Since we decided to stick with black, and already have the code-gradient, is this bug still tracking anything?

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

> * The close button's pressed state seems indistinguishable from the hover state

still bothers me, at the very least (I've never been fond of the non-standard colored close button, and Sam or Max and I had a long debate about it in the last month or so).

Comment 11

[Stuart Morgan]

Ah, I remembered us deciding to use the standard close box several meetings back, so I guess I thought that part was already done. Am I remembering correctly? If so we just need a simple patch.

Comment 12

[Jon Hicks]

Sorry folks, please remind me if there are still outstanding things I need to do on this bug

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #11)
> Ah, I remembered us deciding to use the standard close box several meetings
> back, so I guess I thought that part was already done. Am I remembering
> correctly? If so we just need a simple patch.

I have that same recollection (of course), and the 10-04 notes state that (as reflected by comment 2 above).  I went through later logs, and while we re-re-revisited the bar color, we never revisited the close button discussion, so I believe it stands, and we just need a simple patch :)

Jon, I think we're done with you on this bug, and we thank you deeply for your work on the UI and imagery for this feature :D

Comment 14

[Sean Murphy]

The close button's rollover behavior is broken at the moment.  The RollerImageButton object enables mouse tracking for itself during its initWithFrame method.  Since the close popup button is already instantiated by Interface Builder, initWithFrame: is never called at runtime.

We can take the default setup code from initWithFrame and break it out into a separate method, and then also call this from awakeFromNib.  The only case where an object receives both initWithFrame and awakeFromNib messages is when a CustomView placeholder is used in Interface Builder.

If changing RolloverImageButton makes sense, I'll fix that and switch these images around to the standard style.

Comment 15

[Sean Murphy]

Attachment: hover fixed + tab style close images

Fixes the issue with RolloverImageButton's mouse tracking being disabled by default when it is instantiated within Interface Builder.  The close button images have been changed to use the lighter tab-style ones.

I attached a composite screenshot of the two styles during each state (normal, hover, pressed) as they look on the popup blocker, for comparison.  I'll admit that I prefer the darker images (the ones we're using now).  The tab style images appear to take on a sort of oval shape when placed on the dark background.   Also, the normal (non-hover or pressed) state of the tab style looks out of place on the blocker, and you can barely make out the "x" on it.

The nib also defines an image/alternateImage, but the setImage:/setAlternateImage: code in BrowserWrapper.mm overrides the images set within IB.  After we decide for certain which images to use I'll submit a new nib to match them, just in case those lines are ever removed from BW.mm.

Comment 16

[Sean Murphy]

Attachment: visual of the competing styles

Comment 17

[Stuart Morgan]

Yeah, the grey looks pretty bad. Maybe we should see if we can just make the normal state of a black version stand out more.

Comment 18

[Stuart Morgan]

Comment on attachment 253635 [details] [diff] [review]
hover fixed + tab style close images

>+- (void)setupDefaults;

This is an internal method; put it in a private category in the implementation file.

>+  mImage = nil;
>+  mHoverImage = nil;

Remove these entirely, as they aren't necessary.

Lets assume for the moment that we won't be using the tab close buttons, so you can leave the image name changes out of the next round.

Comment 19

[Sean Murphy]

Attachment: hover fix only [checked in]

(In reply to comment #18)
> This is an internal method; put it in a private category in the implementation
> file.

Duh, sorry I missed that.

> >+  mImage = nil;
> >+  mHoverImage = nil;
> 
> Remove these entirely, as they aren't necessary.

Yeah, instance variables are zeroed out upon allocation, but this was in the original code and I didn't want to remove the lines in case they were there for readability or to explicitly indicate the initial values.

I made some additional style changes in attempt the RolloverImageButton code more consistent with the project.

Comment 20

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 253950 [details] [diff] [review]
hover fix only [checked in]

sr=pink

Comment 21

[Stuart Morgan]

Comment on attachment 253950 [details] [diff] [review]
hover fix only [checked in]

Patch checked in; now we just need to figure out the art.

Comment 22

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I think that the "distortion" look on the grey close buttons can be fixed with a little pixel-pushing (and I think that both grey or black look better on the blue bar; why is it again that we changed our minds again and decided not to go back to blue like we decided at the meeting in October [comment 4]?)

http://www.ardisson.org/smokey/moz/blue-w-black.png
http://www.ardisson.org/smokey/moz/blue-w-grey.png

Comment 23

[Stuart Morgan]

(In reply to comment #22)
> why is it again that we changed our minds again and decided not to go
> back to blue like we decided at the meeting in October [comment 4]?)

Blue = good
Popups = bad
I believe my comment at the meeting where we ditched blue was: "If we ever needed a notification bar that said 'This page is totally awesome', the blue would be perfect."

Comment 24

[Stuart Morgan]

Attachment: new button images

New close button images that Cole made for me, with fatter X (and a height matching the buttons it's next to). There was general agreement in channel that this helps the visibility issue.

Comment 25

[Stuart Morgan]

Attachment: new nib

The nib to go with the above, since the button is now bigger than the frame given to it in the current nib. I also tweaked the horizontal spacing a bit so it doesn't look so off-center.

Comment 26

[Stuart Morgan]

Attachment: centering adjustment

And lastly, Cole pointed out that the aqua buttons appear to sit too high. Since aqua buttons always look slightly high in their frame, we are better of doing a floor than a ceiling when centering the buttons.

Comment 27

[Stuart Morgan]

Checked in on trunk and MOZILLA_1_8_BRANCH.