Make toolbar buttons borderless in the default state

RESOLVED FIXED in Firefox 14

Status

()

Firefox
Theme
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

({addon-compat})

Trunk
Firefox 14
All
Windows 7
addon-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Assignee)

Description

5 years ago
I'll lay some groundwork here, we'll probably want to do additional polishing in bug 734373.
(Assignee)

Comment 1

5 years ago
Created attachment 605777 [details] [diff] [review]
WIP patch

This may be completely busted, I haven't tested it yet.
(Assignee)

Comment 2

5 years ago
Created attachment 605905 [details] [diff] [review]
WIP patch v2
Attachment #605777 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Duplicate of this bug: 675595
(Assignee)

Comment 4

5 years ago
Created attachment 606020 [details] [diff] [review]
patch
Attachment #605905 - Attachment is obsolete: true
Attachment #606020 - Flags: review?(shorlander)
(Assignee)

Comment 5

5 years ago
Created attachment 606178 [details] [diff] [review]
patch v2

fixed a few glitches I discovered
Attachment #606020 - Attachment is obsolete: true
Attachment #606020 - Flags: review?(shorlander)
Attachment #606178 - Flags: review?(shorlander)
(Assignee)

Comment 6

5 years ago
Created attachment 606235 [details] [diff] [review]
patch v3

... and due to the previous changes, the svg mask needed another update ...
Attachment #606178 - Attachment is obsolete: true
Attachment #606178 - Flags: review?(shorlander)
Attachment #606235 - Flags: review?(shorlander)
This is mostly for not showing a button in the default state and extending the hit box right?

So I shouldn't worry too much about padding, style, etc.?

Comment 8

5 years ago
Can you also apply this style in small icons mode and to the buttons of the bookmarks toolbar in order to have more consistency?
In small icon mode buttons are already borderless.

Comment 10

5 years ago
yes, but they use -moz-appearance:toolbarbutton as well as the buttons of the bookmarks toolbar so they look different when you hover and press them
I filed Bug 734326 for getting the bookmarks appearance updated :)
(In reply to Valerio from comment #10)
> yes, but they use -moz-appearance:toolbarbutton as well as the buttons of
> the bookmarks toolbar so they look different when you hover and press them

Yes, part of the Australis work is going to be gaining consistency everywhere.

Comment 13

5 years ago
I filed Bug 736179 for update the Small Icons mode
(Assignee)

Comment 14

5 years ago
(In reply to Stephen Horlander from comment #7)
> This is mostly for not showing a button in the default state and extending
> the hit box right?
> 
> So I shouldn't worry too much about padding, style, etc.?

yep
(Assignee)

Comment 15

5 years ago
Stephen, ping?
(In reply to Dão Gottwald [:dao] from comment #15)
> Stephen, ping?

Sorry for the delay, traveling wrecked my schedule. Looking at this now.
Comment on attachment 606235 [details] [diff] [review]
patch v3

Review of attachment 606235 [details] [diff] [review]:
-----------------------------------------------------------------

> +@navbarLargeIcons@ .toolbarbutton-1[type=menu] > .toolbarbutton-icon {
> +  -moz-padding-end: 17px;
> +}
> +
> +@navbarLargeIcons@ .toolbarbutton-1 > .toolbarbutton-menu-dropmarker {
> +  -moz-margin-start: -15px;
> +}

This adds the padding to #feed-button and separated forward and back buttons even though the dropdown is hidden.

Also the button affordance on the forward button is missing between transitions: http://cl.ly/0q1u0v3C0X3t0J011E3f

Otherwise it looks great!
Attachment #606235 - Flags: review?(shorlander) → review-
(Assignee)

Comment 18

5 years ago
Created attachment 607918 [details] [diff] [review]
patch v4
Attachment #606235 - Attachment is obsolete: true
(Assignee)

Comment 19

5 years ago
Comment on attachment 607918 [details] [diff] [review]
patch v4

I *think* this should do it. Being on Linux, I haven't tested this yet :/
Attachment #607918 - Flags: review?(shorlander)
(Assignee)

Comment 20

5 years ago
I tested this now, the two issues are fixed.
Comment on attachment 607918 [details] [diff] [review]
patch v4

Review of attachment 607918 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Dão Gottwald [:dao] from comment #20)
> I tested this now, the two issues are fixed.

Yes, thank you! :)


> +@navbarLargeIcons@ .toolbarbutton-1[type=menu] {
> +  padding-left: 5px;
> +  padding-right: 5px;
> +}

This is throwing off the spacing for those three special-case menu buttons (http://cl.ly/112c003t0O1f0Y3J2d45). r+ with that.

Thanks!
Attachment #607918 - Flags: review?(shorlander) → review+
(Assignee)

Comment 22

5 years ago
> > +@navbarLargeIcons@ .toolbarbutton-1[type=menu] {
> > +  padding-left: 5px;
> > +  padding-right: 5px;
> > +}
> 
> This is throwing off the spacing for those three special-case menu buttons
> (http://cl.ly/112c003t0O1f0Y3J2d45). r+ with that.

added :not(#back-button):not(#forward-button):not(#feed-button)

http://hg.mozilla.org/mozilla-central/rev/e9938aab62e2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
(Assignee)

Updated

5 years ago
Blocks: 700246
This makes certain custom (non Firefox) toolbar buttons with type as menu act weird.
They don't have any hover state and the drop-marker which should appear to right of text (when no image is displayed) appears behind the text in the center of the button.
Should I file a new bug.
This is related to this bug only right ?
(Assignee)

Comment 24

5 years ago
(In reply to Girish Sharma from comment #23)
> Should I file a new bug.

Yes please.

> This is related to this bug only right ?

Chances are it is.
Depends on: 738304
Depends on: 738320
Depends on: 738548
Don't forget about addon-compat ;)

Jorge announced a margin/width/height "trick" he lifted from Firefox 4 sources at the time:
http://blog.mozilla.com/addons/2010/12/02/toolbar-buttons-in-firefox-4/
(scroll down to the Windows section)
If authors implemented that, addons-mxr indeed seems to list a few instances of (popular) add-ons implementing this, then the hover frame will get too big. It's not a big deal, but it is not pretty either, especially with type=menu buttons.
Examples (my own extensions): DownThemAll! and MinTrayR
Keywords: addon-compat

Comment 26

5 years ago
(In reply to Nils Maier [:nmaier] from comment #25)
> Don't forget about addon-compat ;)
> 
> Jorge announced a margin/width/height "trick" he lifted from Firefox 4
> sources at the time:
> http://blog.mozilla.com/addons/2010/12/02/toolbar-buttons-in-firefox-4/
> (scroll down to the Windows section)
> If authors implemented that, addons-mxr indeed seems to list a few instances
> of (popular) add-ons implementing this, then the hover frame will get too
> big. It's not a big deal, but it is not pretty either, especially with
> type=menu buttons.
> Examples (my own extensions): DownThemAll! and MinTrayR
That issue was brought up in bug 738320 which was 'Marked Invalid' since that issue supposedly can be corrected by the extension developer's end, but it was never stated exactly how. So you may want to post a bug comment on bug 738320 and ask them to be more specific on that issue.

Updated

5 years ago
Depends on: 747690
(Assignee)

Updated

5 years ago
Depends on: 761990

Updated

5 years ago
Depends on: 775191
You need to log in before you can comment on or make changes to this bug.