Closed Bug 623199 Opened 14 years ago Closed 13 years ago

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

Categories

(Toolkit :: Add-ons Manager, defect)

All
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla5
Tracking Status
blocking2.0 --- .x+

People

(Reporter: jboriss, Assigned: dao)

References

Details

(Keywords: polish, Whiteboard: [softblocker])

Attachments

(8 files, 7 obsolete files)

102.70 KB, image/png
Details
1.17 MB, application/x-zip-compressed
Details
8.82 KB, patch
mossop
: review+
Unfocused
: review-
jboriss
: 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
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.
No longer blocks: 550048
No longer depends on: 520124, 586066, 554097, 562902, 562935, 585950, 590130, 601022
See also bug 620963.
Keywords: metapolish
Blocks: 623250
Assignee: bmcbride → nobody
Assignee: nobody → tymerkaev
Status: NEW → ASSIGNED
Keywords: uiwanted
blocking2.0: --- → ?
Whiteboard: [softblocker]
Boriss: why'd you add uiwanted here?
blocking2.0: ? → final+
Keywords: uiwanted
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
(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?
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
(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.
I'm here (was busy few days, but I'm now working on this)
Any progress here Azat? Do you have everything you need?
(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?
Attached patch WIP (osx only) v1 (obsolete) — Splinter Review
Attached image Patch Screenshot (obsolete) —
This is what I see with the applied patch.
Attachment #509597 - Attachment description: Azat Tymerkaev's patch version 1: contextual styling → WIP (osx only) v1
Attachment #509597 - Attachment is obsolete: true
Attached image Mockup: Tweaks needed on patch 509597 (obsolete) —
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)
Attached image Mockup: Tweaks needed on patch 509597 (obsolete) —
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.
I don't think this bug applies on Linux.
OS: All → Windows 7
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
Attached patch WIP patch (obsolete) — Splinter Review
Assignee: tymerkaev → dao
Attachment #504862 - Attachment is obsolete: true
Attachment #509598 - Attachment is obsolete: true
Attachment #510784 - Attachment is obsolete: true
Attached patch patchSplinter Review
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)
(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?
(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.  :)
Attached image screenshot
The patch seems to apply just fine on tip. Make sure you're fully updated.
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-
(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.
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.
(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
Attachment #515338 - Flags: ui-review?(jboriss) → ui-review+
Attachment #515338 - Flags: review?(bmcbride)
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?
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.
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.
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-
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.
(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.
Not sure what you're saying. The global buttons and the buttons in the list view currently use the very same styling.
(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
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
Closed: 13 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.
> 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].
Attached patch final patchSplinter Review
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
Closed: 13 years ago13 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.

Attachment

General

Created:
Updated:
Size: