Add-ons Manager global buttons (forward, backward, tools) should be contextually styled in Windows 7

VERIFIED FIXED in mozilla5

Status

()

Toolkit
Add-ons Manager
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: Boriss, Assigned: dao)

Tracking

({polish})

Trunk
mozilla5
All
Windows 7
polish
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(blocking2.0 .x+)

Details

(Whiteboard: [softblocker])

Attachments

(8 attachments, 7 obsolete attachments)

102.70 KB, image/png
Details
1.17 MB, application/x-zip-compressed
Details
8.82 KB, patch
mossop
: review+
Unfocused
: review-
Boriss
: ui-review-
Details | Diff | Splinter Review
352.14 KB, image/png
Details
645.78 KB, image/png
Details
1.00 KB, image/png
Details
62.79 KB, image/png
Details
7.56 KB, patch
Unfocused
: review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
Created attachment 501305 [details]
Mockup: intended design of add-ons manager global buttons

The buttons at the top of the add-on manager for global controls should be styled to look like a part of the in-content design rather than standard widgets inserted into the page.  Unfocused says this could possibly be achieved by making standard widgets semi-transparent.  If this doesn't work, new images should be added for the widgets.
(Reporter)

Updated

7 years ago
No longer blocks: 550048
No longer depends on: 520124, 586066, 554097, 562902, 562935, 585950, 590130, 601022
(Reporter)

Comment 1

7 years ago
Created attachment 501307 [details]
Mockup: intended design of add-ons manager global buttons
Attachment #501305 - Attachment is obsolete: true

Comment 2

7 years ago
See also bug 620963.
Keywords: meta → polish
(Reporter)

Updated

7 years ago
Blocks: 623250
Assignee: bmcbride → nobody

Updated

7 years ago
Assignee: nobody → tymerkaev
Status: NEW → ASSIGNED
(Reporter)

Updated

7 years ago
Keywords: uiwanted
(Reporter)

Updated

7 years ago
blocking2.0: --- → ?
Whiteboard: [softblocker]
Boriss: why'd you add uiwanted here?
blocking2.0: ? → final+
(Reporter)

Updated

7 years ago
Keywords: uiwanted
(Reporter)

Comment 4

7 years ago
Created attachment 504862 [details]
Graphics and icons: all widgets, color guide for OSX

At Azat's request, I'm attaching graphic files and guides for the add-on manager widgets.  The attachment has all of the graphic widgets for OSX and Windows, which are as follows:

- Windows right arrow
- Windows left arrow
- Windows magnifying glass
- OSX right arrow
- OSX left arrow
- OSX magnifying glass

It also includes a guide, as a .png and Photoshop file, which gives graphic values for OSX.  A similar guide is still needed for Windows, but I'm attaching that later because there's a file missing that Shorlander's looking for.  The OSX values are as follows:

OSX Button:

Drop shadow: white, 1px away, 25% opacity
Inner shadow: white, 2xp away, 25% opacity
Gradient: white, thickest at top, 45% opacity
Outline: #3c4961, 1px thick, 50% opacity

OSX Button pressed:

Drop shadow: white, 1px away, 25% opacity
Inner glow: #2c3647, 5px high, 60% opacity
Gradient: #2d3647, thickest at top, 60% opacity

OSX Search Box:

Drop shadow: white, 2px away, 25% opacity
Inner shadow: black, 1px away, 1px tall, 15% opacity
Gradient: black to white, black on top, 26% opacity
Stroke: #3c4961, 1px thick, 50% opacity
(Reporter)

Comment 5

7 years ago
(In reply to comment #3)
> Boriss: why'd you add uiwanted here?

So I'd remember to attach the files that are now attached.  :)
Azat: Are you actively working on this? Or is it free for someone else to pick up?
(Reporter)

Comment 7

7 years ago
Created attachment 505296 [details]
Graphics and icons: all widgets, color guide for Windows

The attached guide has a .png and Photoshop file for the windows widgets.  I put the Windows widgets there also just to group everything.

Windows button:

Base color: #dfe8f3
Inner shadow: white, 1px thick, 25% opacity
Inner glow: white, 3px thick, 25% opacity
Gradient: Starts white, midway goes to #a6b8ce and shortly to #2d4e73, finishes 5% opacity, ok clearly text isn't the best way to explain a gradient
Stroke: #1f4064 gradient, 75% opacity at center and 50% at ends

Windows button pressed:

Base color: #dfe8f3
Inner shadow: #2c4767, 3px thick, 1px away, 20% opacity
Inner glow: #334a62, 3px thick, 20% opacity
Color overlay: #3d4c5c, 20% opacity
Gradient: white 50% at top, at center #a6b8ce at 25% and then #2d4e73 at 12%, at bottom #1f3754 at 5%
Stroke: #273544 gradient, 75% opacity at center and 50% at ends

Window search bar:

Base color: white
Inner shadow: black, 10% opacity, 2px away at 120°
Stroke: #252f3b gradient, full color to 70% opacity, 1px thick, 35% opacity overall
(Reporter)

Comment 8

7 years ago
(In reply to comment #6)
> Azat: Are you actively working on this? Or is it free for someone else to pick
> up?

I've emailed Azat with no response and have not seen him on IRC in a few days, so it's probably safe to assume he's not currently working on this bug.

Comment 9

7 years ago
I'm here (was busy few days, but I'm now working on this)
Any progress here Azat? Do you have everything you need?
(Reporter)

Comment 11

7 years ago
(In reply to comment #9)
> I'm here (was busy few days, but I'm now working on this)

Azat sent me a patch, which I'll go ahead and post to this bug.  I'm having some trouble running it, though.  Azat, could you possibly attach a few screenshots to this bug?
(Reporter)

Comment 12

7 years ago
Created attachment 509597 [details] [diff] [review]
WIP (osx only) v1
Created attachment 509598 [details]
Patch Screenshot

This is what I see with the applied patch.

Updated

7 years ago
Attachment #509597 - Attachment description: Azat Tymerkaev's patch version 1: contextual styling → WIP (osx only) v1
Attachment #509597 - Attachment is obsolete: true
(Reporter)

Comment 14

7 years ago
Created attachment 510756 [details]
Mockup: Tweaks needed on patch 509597

Getting (In reply to comment #13)
> Created attachment 509598 [details]
> Patch Screenshot
> 
> This is what I see with the applied patch.

Getting there, with a few issues that need tweaking:

* arrow glyphs should be centered in the back and forward buttons
* the outlines on all widgets are solidly black now, but in the mocks follow a gradient: #717c8e at the top, #758092 at the bottom
* Advanced button is too long
* Search bar is too shot - should be as tall as the other widgets (22px)
(Reporter)

Comment 15

7 years ago
Created attachment 510784 [details]
Mockup: Tweaks needed on patch 509597
Attachment #510756 - Attachment is obsolete: true
Depends on: 635675
I've stolen the Mac part of this and put a patch up in bug 635675. This bug can continue to track the implementation on Windows and Linux.
(Assignee)

Comment 17

7 years ago
I don't think this bug applies on Linux.
OS: All → Windows 7
(Reporter)

Updated

7 years ago
Summary: Add-ons Manager global buttons (forward, backward, tools) should be contextually styled → Add-ons Manager global buttons (forward, backward, tools) should be contextually styled in Windows 7
(Assignee)

Comment 18

7 years ago
Created attachment 515200 [details] [diff] [review]
WIP patch
Assignee: tymerkaev → dao
Attachment #504862 - Attachment is obsolete: true
Attachment #509598 - Attachment is obsolete: true
Attachment #510784 - Attachment is obsolete: true
(Assignee)

Comment 19

7 years ago
Created attachment 515338 [details] [diff] [review]
patch

I'll file a follow-up bug on the tools button's dropmarker icon
Attachment #515200 - Attachment is obsolete: true
Attachment #515338 - Flags: ui-review?(jboriss)
(Reporter)

Comment 20

7 years ago
(In reply to comment #19)
> Created attachment 515338 [details] [diff] [review]
> patch
> 
> I'll file a follow-up bug on the tools button's dropmarker icon

The tree's changed such that this patch can't be applied. Could you post a new one for the current tree?
(Reporter)

Comment 21

7 years ago
(In reply to comment #20)
> (In reply to comment #19)
> > Created attachment 515338 [details] [diff] [review]
> > patch
> > 
> > I'll file a follow-up bug on the tools button's dropmarker icon
> 
> The tree's changed such that this patch can't be applied. Could you post a new
> one for the current tree?

A screenshot of the patch applied would be much appreciated too.  :)
(Assignee)

Comment 22

7 years ago
Created attachment 515892 [details]
screenshot

The patch seems to apply just fine on tip. Make sure you're fully updated.
(Reporter)

Comment 23

7 years ago
Comment on attachment 515338 [details] [diff] [review]
patch

This patch looks great except for one item: the expansion arrow on the Advanced menu is too small and off center. 

It's currently not consistent with Windows nor with Firefox.

Since Firefox and Windows aren't even consistent with each other, this arrow should match the other arrows in Firefox near it (e.g. search bar arrow).  I'm attaching win7_arrow.png, which is the same size as the search bar arrow, to be replace the current smaller arrow.
Attachment #515338 - Flags: ui-review?(jboriss) → ui-review-
(Reporter)

Comment 24

7 years ago
Created attachment 516086 [details]
Diagram: The expansion arrow on the Advanced menu needs slight tweaking
(Reporter)

Comment 25

7 years ago
Created attachment 516087 [details]
win7_arrow.png, to replace current expansion arrow in Advanced Menu
(Assignee)

Comment 26

7 years ago
(In reply to comment #23)
> This patch looks great except for one item: the expansion arrow on the Advanced
> menu is too small and off center. 
> 
> It's currently not consistent with Windows nor with Firefox.

See comment 19. It actually is consistent with Firefox, but Firefox isn't consistent with Windows.
(Assignee)

Updated

7 years ago
Attachment #515338 - Flags: ui-review- → ui-review?(jboriss)
To me the border radius of the buttons looks 1 or 2 pixels bigger than in the mockups.
(Reporter)

Comment 28

7 years ago
(In reply to comment #26)
> (In reply to comment #23)
> > This patch looks great except for one item: the expansion arrow on the Advanced
> > menu is too small and off center. 
> > 
> > It's currently not consistent with Windows nor with Firefox.
> 
> See comment 19. It actually is consistent with Firefox, but Firefox isn't
> consistent with Windows.

If there's a followup bug on the dropmarker icon, then this r+'s easily
(Reporter)

Updated

7 years ago
Attachment #515338 - Flags: ui-review?(jboriss) → ui-review+
(Assignee)

Updated

7 years ago
Attachment #515338 - Flags: review?(bmcbride)
(Assignee)

Updated

7 years ago
Attachment #515338 - Flags: review?(dtownsend)
** PRODUCT DRIVERS PLEASE NOTE **

This bug is one of 7 automatically changed from blocking2.0:final+ to blocking2.0:.x during the endgame of Firefox 4 for the following reasons:

 - it was marked as a soft blocking issue without a requirement for beta coverage
blocking2.0: final+ → .x+
Comment on attachment 515338 [details] [diff] [review]
patch

>diff --git a/toolkit/themes/winstripe/mozapps/extensions/extensions.css b/toolkit/themes/winstripe/mozapps/extensions/extensions.css
>--- a/toolkit/themes/winstripe/mozapps/extensions/extensions.css
>+++ b/toolkit/themes/winstripe/mozapps/extensions/extensions.css
>@@ -30,16 +30,19 @@
>  * use your version of this file under the terms of the MPL, indicate your
>  * decision by deleting the provisions above and replace them with the notice
>  * and other provisions required by the GPL or the LGPL. If you do not delete
>  * the provisions above, a recipient may use your version of this file under
>  * the terms of any one of the MPL, the GPL or the LGPL.
>  *
>  * ***** END LICENSE BLOCK ***** */
> 
>+@namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
>+@namespace html url("http://www.w3.org/1999/xhtml");

Why are these necessary?
(Assignee)

Comment 31

7 years ago
They aren't necessary. It's cleaner to do this when dealing with multiple namespaces.
(In reply to comment #31)
> They aren't necessary. It's cleaner to do this when dealing with multiple
> namespaces.

I don't see any use of the html namespace anywhere though.
(Assignee)

Comment 33

7 years ago
Yeah, the patch actually removes the only HTML node targeting rule, so I could drop that namespace declaration.
Comment on attachment 515338 [details] [diff] [review]
patch

r=me with that dropped then
Attachment #515338 - Flags: review?(dtownsend)
Attachment #515338 - Flags: review?(bmcbride)
Attachment #515338 - Flags: review+
Why is this removing the styles for the .addon-control class? That's not mentioned anywhere in this bug.
(Reporter)

Comment 36

7 years ago
Comment on attachment 515338 [details] [diff] [review]
patch

Dropping r=me due to this patch removing the styles for the .addon-control class.  This bug should only effect the add-on toolbar.  Nice catch, Blair!
Attachment #515338 - Flags: ui-review+ → ui-review-
(Assignee)

Comment 37

7 years ago
Because boriss was saying "rather than standard widgets inserted into the page". The other buttons aren't however styled like standard widgets. Just like the global buttons, they copied the custom toolbarbutton styling from browser.css, which I don't believe was ever part of the plan.
(Reporter)

Comment 38

7 years ago
(In reply to comment #37)
> Because boriss was saying "rather than standard widgets inserted into the
> page". The other buttons aren't however styled like standard widgets. Just like
> the global buttons, they copied the custom toolbarbutton styling from
> browser.css, which I don't believe was ever part of the plan.

The top toolbar items are similar to the standard browser.css, but the widgets in list view are styled differently to match the add-ons manager, not system defaults.
(Assignee)

Comment 39

7 years ago
Not sure what you're saying. The global buttons and the buttons in the list view currently use the very same styling.
(Reporter)

Comment 40

7 years ago
Created attachment 518195 [details]
Diagram: Intended add-on manager widgets vs standard Windows 7 set

(In reply to comment #39)
> Not sure what you're saying. The global buttons and the buttons in the list
> view currently use the very same styling.

Hopefully the attached image clears it up - you're right, the intended styling for the list view buttons is like the global buttons
(Assignee)

Comment 41

7 years ago
I restored the .addon-control styling and copied over the textures from .header-button.

http://hg.mozilla.org/mozilla-central/rev/3971f2023772

filed a follow-up bug, too: bug 646419
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
Was surprised to see this landed - I was expecting another round of reviews, after the r- from Boriss and I.

Anyway, this seems to have removed the hover effect for all the buttons (the blue glow). Also, the header buttons don't seem to have enough padding, so they look squashed. Filed bug 646713.
(Assignee)

Comment 43

7 years ago
> Anyway, this seems to have removed the hover effect for all the buttons (the
> blue glow).

This was in accordance with to attachment 505296 [details].
Created attachment 523352 [details] [diff] [review]
final patch

This probably shouldn't have landed without a re-review. Boriss, Blair can you do a post review on the patch as it landed and we'll need to either backout or take care of any fixes.
Attachment #523352 - Flags: ui-review?(jboriss)
Attachment #523352 - Flags: review?(bmcbride)
Reopening until any issues are resolved.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 523352 [details] [diff] [review]
final patch

Sorry, I somehow missed this. Bug 646713 already covers everything - lets just get that fixed.
Attachment #523352 - Flags: ui-review?(jboriss)
Attachment #523352 - Flags: review?(bmcbride)
Attachment #523352 - Flags: review+
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
Blair, any changeset id?
(In reply to comment #47)
> Blair, any changeset id?

See comment 41.
Verified fixed with Mozilla/5.0 (Windows NT 6.1; rv:5.0a2) Gecko/20110501 Firefox/5.0a2

With the classic view on Windows 2000 the search field text box has an ugly 2px or so border. Is it something we want to fix? If that's the case I will file a new bug.
Status: RESOLVED → VERIFIED
Flags: in-testsuite-
Flags: in-litmus-
(In reply to comment #49)
> With the classic view on Windows 2000 the search field text box has an ugly 2px
> or so border. Is it something we want to fix? If that's the case I will file a
> new bug.

That seems to be a regression from bug 646419. I will file a new bug.
You need to log in before you can comment on or make changes to this bug.