Closed Bug 554662 Opened 14 years ago Closed 14 years ago

Further Tweaks to Toolbar Buttons

Categories

(Firefox :: Theme, enhancement)

All
Windows 7
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3.7a4

People

(Reporter: shorlander, Assigned: shorlander)

References

Details

Attachments

(4 files, 4 obsolete files)

I did some further tweaking to the toolbar buttons and fields to bring them more inline with the designs.

* Darkened the Borders and Shadows
* Adjusted the Hues and Transparency on the background gradient
* Polished all the glyphs:
  - Changed the styling to an inset with glow
  - Made them all 20x20 to account for the outer glow
  - Tried to normalize the line width where applicable
  - Redid some to look nicer/less complicated

* Back Button Special Case
  - Remove the three stage border that works so well on the square buttons because it caused a visible stratification on the round back button
  - Recreated the border with -moz-box-shadow
Attachment #434552 - Flags: review?(dao)
Attached image Before and After
Blocks: 547371
Comment on attachment 434552 [details] [diff] [review]
Further Toolbar Tweaks for New Theme

> toolbar:not([iconsize="small"])[mode="icons"] #back-button {
>   -moz-border-radius: 100%;
>   padding: 0;
>-  width: 32px;
>-  height: 32px;
>+  width: 30px;
>+  height: 30px;
>   position: relative;
>   z-index: 1;
>-  margin-top: -3px;
>-  margin-bottom: -3px;
>-  -moz-image-region: rect(18px, 18px, 38px, 0);
>+  margin-top: -2px;
>+  margin-bottom: -2px;
>+  border: none;
>+  background: rgba(151,152,153,0.05)
>+              -moz-linear-gradient(top, rgba(251,252,253,.97), rgba(246,247,248,.5) 49%, 
>+                                        rgba(231,232,233,.45) 51%, rgba(225,226,229,.2));

You don't need to repeat the background color, since you didn't change it.

>+toolbar:not([iconsize="small"])[mode="icons"] #back-button:not([disabled="true"]):not([checked="true"]):not(:active):hover {
>+  background-color: hsla(190,60%,70%,.5);
>+  -moz-box-shadow: 0 0 0 1px rgba(255,255,255,.3) inset,
>+                   0 0 0 2px rgba(255,255,255,.1) inset,
>+                   0 0 5px hsl(190,90%,80%),
>+                   0 0 0 1px hsla(190,50%,40%,.8),
>+                   0 1px 0 0 rgba(0,0,0,.4),
>+                   0 1px 1px 0 rgba(0,0,0,.3),
>+                   1px 2px 1px 0 rgba(0,0,0,0.2);
>+  -moz-transition: background-color .5s ease-in;
>+}

This needs to be adjusted for bug 553673.
(In reply to comment #0)
>   - Made them all 20x20 to account for the outer glow

I'd prefer to keep the size as it is, as I don't think the extra glow is actually visible...
Comment on attachment 434552 [details] [diff] [review]
Further Toolbar Tweaks for New Theme

>diff --git a/browser/themes/winstripe/browser/Toolbar.png b/browser/themes/winstripe/browser/Toolbar.png

Why did you move the big back icon on one row with the other icons? I'd prefer to keep it in a separate row, as it could be (and actually is, in the current Toolbar.png) larger than the other icons. We wouldn't want to change the other icons' regions whenever we touch that special back icon.

>-  background: rgba(85%,85%,85%,.1)
>-              -moz-linear-gradient(top, rgba(255,255,255,.9), rgba(255,255,255,.45) 48%,
>-                                        rgba(90%,90%,90%,.4) 52%, rgba(90%,90%,90%,.2));
>+  background: rgba(151,152,153,.05)
>+              -moz-linear-gradient(top, rgba(251,252,253,.95), rgba(246,247,248,.47) 49%, 
>+                                        rgba(231,232,233,.45) 51%, rgba(225,226,229,.3));

Please remove "top," while you're at it. It's the default behavior, doesn't need to be specified.
Comment on attachment 434552 [details] [diff] [review]
Further Toolbar Tweaks for New Theme

You also need to fix up #download-monitor if you want to change the sizes and regions (again, I think you shouldn't; I can't tell the difference in the screenshot).
Attachment #434552 - Flags: review?(dao) → review-
Attached image Glyph Close-up
(In reply to comment #3)
> (In reply to comment #0)
> >   - Made them all 20x20 to account for the outer glow
> 
> I'd prefer to keep the size as it is, as I don't think the extra glow is
> actually visible...

The outer glow is 2px and the glyph is 16px so the end size is 20x20. I could reduce the glow to 1px to get to 18x18 but I don't think it looks as nice. Admittedly it's hard to tell the difference on the lighter color backgrounds it's more for when the background is darker and for the possibility of using the glyphs outside of the buttons.

Is there some reason they can't be 20x20 instead of 18x18?


(In reply to comment #4)
> (From update of attachment 434552 [details] [diff] [review])
> >diff --git a/browser/themes/winstripe/browser/Toolbar.png b/browser/themes/winstripe/browser/Toolbar.png
> 
> Why did you move the big back icon on one row with the other icons? I'd prefer
> to keep it in a separate row, as it could be (and actually is, in the current
> Toolbar.png) larger than the other icons. We wouldn't want to change the other
> icons' regions whenever we touch that special back icon.

I can move it back. It just fit within the 20x20 size so I put them all on one line.
(In reply to comment #6)
> The outer glow is 2px and the glyph is 16px so the end size is 20x20. I could
> reduce the glow to 1px to get to 18x18 but I don't think it looks as nice.
> Admittedly it's hard to tell the difference on the lighter color backgrounds
> it's more for when the background is darker and for the possibility of using
> the glyphs outside of the buttons.

Yeah, I could see the full glow in an image editor, but not at 100% and with the button background. I don't think this needs to be optimized for the possibility of using the icons without the button background right now.

> Is there some reason they can't be 20x20 instead of 18x18?

It just complicates things. We'd want to limit extension icons to 18px at most, as they don't have the built in padding.
Returned the glyphs to previous format and addressed other comments.

(In reply to comment #2)
> You don't need to repeat the background color, since you didn't change it.

The alpha values are actually slightly different. The bigger button wasn't as opaque as the smaller buttons. Maybe because of the gradient stretching?
Attachment #434552 - Attachment is obsolete: true
Attachment #434909 - Flags: review?(dao)
Comment on attachment 434909 [details] [diff] [review]
Further Toolbar Tweaks for New Theme - 02

It looks like you somehow included the patch from bug 553673. You need to update your tree instead (merging browser.css automatically, as the first patch applies on mozilla-central).
Attachment #434909 - Flags: review?(dao)
(In reply to comment #8)
> (In reply to comment #2)
> > You don't need to repeat the background color, since you didn't change it.
> 
> The alpha values are actually slightly different.

The alpha values of the background-image (the gradient), yes, but I was referring to the background-color, rgba(151,152,153,.05).
(In reply to comment #9)
> (From update of attachment 434909 [details] [diff] [review])
> It looks like you somehow included the patch from bug 553673. You need to
> update your tree instead (merging browser.css automatically, as the first patch
> applies on mozilla-central).

Oops! This should be correct. Thanks!
Attachment #434909 - Attachment is obsolete: true
Attachment #434978 - Flags: review?(dao)
Comment on attachment 434978 [details] [diff] [review]
Further Toolbar Tweaks for New Theme - 02

> toolbar:not([iconsize="small"])[mode="icons"] #back-button {
>   -moz-border-radius: 100%;
>   padding: 0;
>-  width: 32px;
>-  height: 32px;
>+  width: 30px;
>+  height: 30px;
>   position: relative;
>   z-index: 1;
>-  margin-top: -3px;
>-  margin-bottom: -3px;
>-  -moz-image-region: rect(18px, 18px, 38px, 0);
>+  margin-top: -2px;
>+  margin-bottom: -2px;
>+  border: none;
>+  background: -moz-linear-gradient(rgba(251,252,253,.97), rgba(246,247,248,.5) 49%, 
>+                                   rgba(231,232,233,.45) 51%, rgba(225,226,229,.2));

s/background/background-image/

>+toolbar:not([iconsize="small"])[mode="icons"] #back-button:not([disabled="true"]):not([checked="true"]):not(:active):hover {
>+  background-color: hsla(190,60%,70%,.5);

Redundant declaration (no change from .toolbarbutton-1:not([disabled="true"]):not([checked="true"]):not(:active):hover).

>+  -moz-transition: background-color .4s ease-in,
>+                   -moz-box-shadow .3s ease-in;

This look redundant too.

>+toolbar:not([iconsize="small"])[mode="icons"] #back-button:not([disabled="true"]):hover:active {
>+  background-color: transparent;
...
>+  text-shadow: none;

Ditto.

> toolbar:not([iconsize="small"])[mode="icons"] #back-button > .toolbarbutton-icon {
>+	padding-top: 1px;

Why is this needed?

nit: use spaces, not tab.
Comment on attachment 434978 [details] [diff] [review]
Further Toolbar Tweaks for New Theme - 02

>+toolbar:not([iconsize="small"])[mode="icons"] #back-button:not([disabled="true"]):not([checked="true"]):not(:active):hover {
>+  background-color: hsla(190,60%,70%,.5);
>+  -moz-box-shadow: 0 0 0 1px rgba(255,255,255,.3) inset,
>+                   0 0 0 2px rgba(255,255,255,.1) inset,
>+                   0 0 0 1px hsla(190,50%,40%,.3),
>+                   0 1px 0 0 rgba(0,0,0,.4),
>+                   0 1px 1px 0 rgba(0,0,0,.3),
>+                   1px 2px 1px 0 rgba(0,0,0,0.2),
>+                   0 0 5px hsl(190,90%,80%);

You need to make sure that the spread radii compensate the fake border... For instance, 0 0 5px hsl(190,90%,80%) needs to be 0 0 5px 1px hsl(190,90%,80%) instead. Also please drop the trailing 0 if you don't want a spread radius.
(In reply to comment #12)
> > toolbar:not([iconsize="small"])[mode="icons"] #back-button > .toolbarbutton-icon {
> >+	padding-top: 1px;
> 
> Why is this needed?

The icon was pushed up 1px without it. Is there a better way to align it?

> nit: use spaces, not tab.

Thanks, I have bad whitespace habits :)
Attachment #434978 - Attachment is obsolete: true
Attachment #435184 - Flags: review?(dao)
Attachment #434978 - Flags: review?(dao)
(In reply to comment #14)
> > > toolbar:not([iconsize="small"])[mode="icons"] #back-button > .toolbarbutton-icon {
> > >+	padding-top: 1px;
> > 
> > Why is this needed?
> 
> The icon was pushed up 1px without it. Is there a better way to align it?

It should be centered by default, so my question really was: Why is the icon pushed down in the first place?
(In reply to comment #15)
> It should be centered by default, so my question really was: Why is the icon
> pushed down in the first place?

The image itself is centered in the image area so I am not sure. I will double check and see if I did something weird.
Comment on attachment 435184 [details] [diff] [review]
Further Toolbar Tweaks for New Theme - 03

>+toolbar:not([iconsize="small"])[mode="icons"] #back-button:not([disabled="true"]):hover:active {
>+  background-color: transparent;
>+  -moz-box-shadow: 0 0 9px rgba(0,0,0,.4) inset,
>+                   0 0 3px rgba(0,0,0,.4) inset,
>+                   0 0 0 1px rgba(0,0,0,.65),
>+                   0 2px 0 rgba(255,255,255,.4);
> }

background-color: transparent; is also redundant, I think.
(In reply to comment #17)
> background-color: transparent; is also redundant, I think.

Thanks, missed that one.

(In reply to comment #16)
> (In reply to comment #15)
> > It should be centered by default, so my question really was: Why is the icon
> > pushed down in the first place?
> 
> The image itself is centered in the image area so I am not sure. I will double
> check and see if I did something weird.

I figured it out. I screwed up the image-region coordinates.
Attachment #435184 - Attachment is obsolete: true
Attachment #435190 - Flags: review?(dao)
Attachment #435184 - Flags: review?(dao)
Attachment #435190 - Flags: review?(dao) → review+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/82d527d82812
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Summary: Further Tweaks to Toolbar Buttons/Fields → Further Tweaks to Toolbar Buttons
Target Milestone: --- → Firefox 3.7a4
Looks like I got a jagged and no longer round back button now.
Could you attach a screenshot ?
Attached image Jagged Back Button
(In reply to comment #22)
> Created an attachment (id=435515) [details]
> Jagged Back Button

This happens only when you have Direct2D enabled. I suggest creating a bug for this that blocks Bug 527707. Unless somebody already did.
Depends on: 556210
I have a question, why did all the browsers all of a sudden you the word back from their browsers. The back button is the most frequently used button in the browser so I think it should be the biggest button. Having that big button is easier and faster to click. In addition I use a tablet PC, so it is much easier and faster to click that button was bigger.
I noticed that a lot of are trying to remove or minimize a lot of controls to increase screen real estate for content instead of the controls. I would rather have more controls that are very easy to access and more content space because it is very easy to scroll.
And since monitors are increasing the resolution, I think those extra pixels can be used for controls.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: