Closed
Bug 601022
Opened 14 years ago
Closed 14 years ago
Add final visual style and graphics to add-ons manager
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
VERIFIED
FIXED
mozilla2.0b8
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: jboriss, Assigned: Unfocused)
References
Details
Attachments
(9 files, 6 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
dao
:
review+
mossop
:
feedback+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details |
This bug will track the addition of the final colors, textures, gradients, and files to the add-ons manager for Firefox 4.0.
Foreground color and graphics:
• See attachment: "Foreground color and graphics"
• All files: http://people.mozilla.com/~jboriss/bugs/addons_manager_styling/foreground_colors_and_graphics.zip
Background color and graphics:
• See attachment for OSX: "OSX Background Color and Graphics"
• See attachment for Windows 7: "Windows 7 Background Color and Graphics"
• All files: http://people.mozilla.com/~jboriss/bugs/addons_manager_styling/background_colors_and_graphics.zip
Reporter | ||
Comment 1•14 years ago
|
||
The images I marked above as attachments are too big and make Bugzilla's head explode, so I threw them online:
Foreground color and graphics:
• http://people.mozilla.com/~jboriss/bugs/addons_manager_styling/foreground_color_and_graphics.png
OSX background color and graphics:
• http://people.mozilla.com/~jboriss/bugs/addons_manager_styling/osx_background_color_and_graphics.png
• Windows 7 background color and graphics:
http://people.mozilla.com/~jboriss/bugs/addons_manager_styling/windows_7_background_color_and_graphics.png
OS: Mac OS X → Windows 7
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Comment 2•14 years ago
|
||
All these blockers are tied together, submit patches however is best.
At present, in default theme color scheme used by Add-ons is not matching that of default theme. It looks odd(or not formal) when you see current Addon screen, when user use windows classic theme.
http://people.mozilla.com/~jboriss/bugs/addons_manager_styling/foreground_color_and_graphics.png
also not matching with windows classic theme.
Same issue is there for Tabcandy too
I have created Bug 601305
I also have another suggestion - bug 601306
RFE: Need an option to Hide disabled extension and plugin
Depends on: 601305
Assignee | ||
Comment 4•14 years ago
|
||
(In reply to comment #3)
> At present, in default theme color scheme used by Add-ons is not matching that
> of default theme.
That's because it's not finished yet, thus we have this bug ;)
> It looks odd(or not formal) when you see current Addon
> screen, when user use windows classic theme.
>
> http://people.mozilla.com/~jboriss/bugs/addons_manager_styling/foreground_color_and_graphics.png
> also not matching with windows classic theme.
See bug 563565 for using system-colors when appropriate.
> I also have another suggestion - bug 601306
> RFE: Need an option to Hide disabled extension and plugin
I don't see how that's related to this bug.
Assignee | ||
Comment 5•14 years ago
|
||
Windows only. This makes some changes to the structure of the XUL/XBL, so the OSX/Linux themes probably look a bit uglier with this applied.
Notes:
* Still need to make the aero-style light beams only apply when on Vista/7, when aero-glass (compositor) is enabled.
* The icon of a disabled addon is greyscale in the list view, but not in the detail view. For some reason, trying that just makes it not show at all. There's a couple of SVG bugs that could cause this (bug 577824, bug 590434), but I gave up spending time on it since its not a big thing (its mostly important for list view).
* For consistency, I moved the search-loading indicator to be in the search view.
* The styling of the loading indicators for search and details will probably pick up the style used in bug 601143, rather than whats in this patch (although they're similar).
* Waiting on some graphics from Boriss - gear icon, and updated red stripes.
* Waiting on feedback from Boriss about what to do about integration with the page's browser tab. The patch currently has a gradient at the top, which fades out the texture/color/etc, to blend with the tab's background color.
Things that weren't possible:
* Inset shadow in the viewport and selected category item. I couldn't find any acceptable way to cut out the shadow where the category meets the view port - and any hacks I tried worked acceptably.
Attachment #482194 -
Flags: review?(dtownsend)
Assignee | ||
Updated•14 years ago
|
Attachment #482194 -
Attachment description: WiP v1 → WiP v1 (Windows-only)
Comment 6•14 years ago
|
||
Can we get a screenshot with the WIP applied?
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #6)
> Can we get a screenshot with the WIP applied?
Oh, yes - knew I forgot something!
Assignee | ||
Updated•14 years ago
|
Attachment #482399 -
Attachment description: WiP v1 screenshot (Win7) → WiP v1 list-view screenshot (Win7)
Assignee | ||
Comment 8•14 years ago
|
||
Assignee | ||
Comment 9•14 years ago
|
||
Couple of other things to note:
* The patch purposefully goes against what bug 563565 proposes. Its overrides all system colors, so the interface should look (mostly) the same on any Windows theme. This guarantees that the interface will always be useable on any theme - which is quite a challenge when using system colors, since everything has custom styles (the best we can do is mix-and-match system colors and custom colors, which doesn't work so well in practice). Already discussed this with Boriss and Mossop - putting it here for future reference.
* If the window is small (800px wide, or less), I've made it so the categories list collapses to just icons. This makes the view-port much bigger on smaller screens, where the categories list was (needlessly) taking up half the screen real-estate. That solves one very common issue with using the EM on small screens. Going to need to add a tooltip though (which will be just the category name, so no string changes needed).
Assignee | ||
Updated•14 years ago
|
OS: Windows 7 → All
Hardware: x86 → All
Comment 10•14 years ago
|
||
Do you know how it looks in Windows XP? It still is the most used operating system... The gradient that connects the tab with the page probably looks out of place then?
As a test in my CSS skills, I decided to recreate the mockups locally. I think reducing the size (font and icon size) of the categories looks a lot more mature. It also puts more focus where it should be, on the page contents.
Comment 11•14 years ago
|
||
Comment on attachment 482194 [details] [diff] [review]
WiP v1 (Windows-only)
This looks good, one correction that needs to be made however I'd really like it if we could get dao or Ryan to find the time to look over the styles, they have way more knowledge with CSS than me.
>diff --git a/toolkit/mozapps/extensions/content/extensions.js b/toolkit/mozapps/extensions/content/extensions.js
> var sortService = Cc["@mozilla.org/xul/xul-sort-service;1"].
> getService(Ci.nsIXULSortService);
> sortService.sort(this._listBox, aSortBy, hints);
>
>+ var item = this._listBox.querySelector("richlistitem[remote='true'][first]");
>+ if (item)
>+ item.remoteItems.remoteAttribute("first");
>+ item = this._listBox.querySelector("richlistitem[remote='true'][last]");
>+ if (item)
>+ item.remoteItems.remoteAttribute("last");
These look broken to me.
>+ var items = this._listBox.querySelectorAll("richlistitem[remote='true']");
>+ if (items.length > 0) {
>+ items[0].setAttribute("first", true);
>+ items[items.length - 1].setAttribute("last", true);
>+ }
Is there really not a nicer way to do this? :(
>diff --git a/toolkit/themes/winstripe/mozapps/extensions/extensions.css b/toolkit/themes/winstripe/mozapps/extensions/extensions.css
>-#category-search > .category-icon {
>+#category-search .category-icon {
Any reason for dropping all the >'s? It makes the lookups a bit quicker I believe.
Attachment #482194 -
Flags: review?(dtownsend) → review-
Assignee | ||
Comment 12•14 years ago
|
||
(In reply to comment #11)
> >-#category-search > .category-icon {
> >+#category-search .category-icon {
>
> Any reason for dropping all the >'s? It makes the lookups a bit quicker I
> believe.
Opps - that was left-over from some ill-fated experiments.
Assignee | ||
Comment 13•14 years ago
|
||
(In reply to comment #10)
> Do you know how it looks in Windows XP? It still is the most used operating
> system... The gradient that connects the tab with the page probably looks out
> of place then?
Sadly, I don't have a XP install to test on. Will see what I can do.
> I think
> reducing the size (font and icon size) of the categories looks a lot more
> mature. It also puts more focus where it should be, on the page contents.
Boriss: thoughts?
Comment 14•14 years ago
|
||
(In reply to comment #13)
> (In reply to comment #10)
> > Do you know how it looks in Windows XP? It still is the most used operating
> > system... The gradient that connects the tab with the page probably looks out
> > of place then?
>
> Sadly, I don't have a XP install to test on. Will see what I can do.
Lets start a tryserver build so everyone with an XP machine/vm can test it. If it's not ready for yet, we should wait until remaining questions are solved. I could also ask for feedback in our nightly tester list.
Comment 15•14 years ago
|
||
(In reply to comment #13)
> (In reply to comment #10)
> > Do you know how it looks in Windows XP? It still is the most used operating
> > system... The gradient that connects the tab with the page probably looks out
> > of place then?
>
> Sadly, I don't have a XP install to test on. Will see what I can do.
FYI, you might also be interested in bug 543910 then.
Assignee | ||
Comment 16•14 years ago
|
||
(In reply to comment #11)
> >+ var item = this._listBox.querySelector("richlistitem[remote='true'][first]");
> >+ if (item)
> >+ item.remoteItems.remoteAttribute("first");
> >+ item = this._listBox.querySelector("richlistitem[remote='true'][last]");
> >+ if (item)
> >+ item.remoteItems.remoteAttribute("last");
>
> These look broken to me.
How so?
> >+ var items = this._listBox.querySelectorAll("richlistitem[remote='true']");
> >+ if (items.length > 0) {
> >+ items[0].setAttribute("first", true);
> >+ items[items.length - 1].setAttribute("last", true);
> >+ }
>
> Is there really not a nicer way to do this? :(
Sadly, no :( Unless we split out local and remote results into their own richlistboxes (which I originally tried, but discovered half the search view would need rewritten). That code is solving the same issue we had with the rows in the details view (bug 592708).
Assignee | ||
Comment 17•14 years ago
|
||
(In reply to comment #15)
> FYI, you might also be interested in bug 543910 then.
I saw that - not sure if its needed for this or not. I'm now changing the background based on whether its running with aero (-moz-windows-compositor), the classic theme (-moz-windows-classic), and a catch-all for anything else - it seems to work really well for the various theming options in Windows 7. I'm hoping to get Windows XP running in a VM this week to play around with it.
Assignee | ||
Comment 18•14 years ago
|
||
Various tweaks, changes, and additions to the Windows theme.
Linux theme now has no hard-coded colors, other than colors that overlay ontop of system colors (eg, notification gradients). Also added icons to buttons, which only show up if GTK is configured to show icons for buttons. Tested with various themes - light and dark.
Attachment #483960 -
Flags: feedback?(dtownsend)
Assignee | ||
Updated•14 years ago
|
Attachment #482194 -
Attachment is obsolete: true
Assignee | ||
Comment 19•14 years ago
|
||
Assignee | ||
Comment 20•14 years ago
|
||
Assignee | ||
Comment 21•14 years ago
|
||
It's more obvious here that there's not a seemless transition between the tab and the content, when the navigation bar is hidden.
Comment 22•14 years ago
|
||
In the Windows theme, is the button theme exclusive to the add-ons manager? It's just that I've seen the being used in other mockups.
Comment 23•14 years ago
|
||
(In reply to comment #16)
> (In reply to comment #11)
> > >+ var item = this._listBox.querySelector("richlistitem[remote='true'][first]");
> > >+ if (item)
> > >+ item.remoteItems.remoteAttribute("first");
> > >+ item = this._listBox.querySelector("richlistitem[remote='true'][last]");
> > >+ if (item)
> > >+ item.remoteItems.remoteAttribute("last");
> >
> > These look broken to me.
>
> How so?
I don't now what remoteItems or remoteAttribute are, they don't exist anywhere in our codebase. I suspect you just want item.removeAttribute in both cases.
Assignee | ||
Comment 24•14 years ago
|
||
(In reply to comment #23)
> > How so?
>
> I don't now what remoteItems or remoteAttribute are, they don't exist anywhere
> in our codebase. I suspect you just want item.removeAttribute in both cases.
Oh, wow, I didn't see those typos :\ Kinda surprised that didn't blow up more obviously.
Assignee | ||
Comment 25•14 years ago
|
||
(In reply to comment #22)
> In the Windows theme, is the button theme exclusive to the add-ons manager?
> It's just that I've seen the being used in other mockups.
The buttons in the header are almost exactly the same as Firefox's toolbar buttons (minus the drop shadow). The other buttons are also very similar, with some additional tweaks - eg, the corners aren't as rounded, there's additional padding on either side of the button text.
Assignee | ||
Comment 26•14 years ago
|
||
Attachment #483960 -
Attachment is obsolete: true
Attachment #484540 -
Flags: feedback?(dtownsend)
Attachment #483960 -
Flags: feedback?(dtownsend)
Assignee | ||
Updated•14 years ago
|
Attachment #484540 -
Flags: review?(dao)
Comment 27•14 years ago
|
||
Comment on attachment 484540 [details] [diff] [review]
WiP v3 (Windows and Linux)
Looks fine from my point of view, leaving the nitty gritty theme reviewing to dao though
Attachment #484540 -
Flags: feedback?(dtownsend) → feedback+
Updated•14 years ago
|
Updated•14 years ago
|
Comment 28•14 years ago
|
||
Comment on attachment 484540 [details] [diff] [review]
WiP v3 (Windows and Linux)
global nit: replace 0xp and 0em with 0
Links in selected list items are invisible over here (Linux). Should add color: inherit; there.
> <xul:vbox anonid="relnotes-container" class="relnotes-container">
> <xul:label class="relnotes-header" value="&addon.releaseNotes.label;"/>
> <xul:label anonid="relnotes-loading" value="&addon.loadingReleaseNotes.label;"/>
> <xul:label anonid="relnotes-error" hidden="true"
> value="&addon.errorLoadingReleaseNotes.label;"/>
>- <xul:vbox anonid="relnotes"/>
>+ <xul:vbox anonid="relnotes" class="relnotes"/>
> </xul:vbox>
indentation is off
>--- a/toolkit/mozapps/extensions/content/extensions.xul
>+++ b/toolkit/mozapps/extensions/content/extensions.xul
>@@ -56,17 +56,17 @@
>
> <xhtml:link rel="shortcut icon"
> href="chrome://mozapps/skin/extensions/extensionGeneric-16.png"/>
>
> <script type="application/javascript"
> src="chrome://mozapps/content/extensions/extensions.js"/>
> <script type="application/javascript"
> src="chrome://global/content/contentAreaUtils.js"/>
>-
>+
bogus whitespace change
>+.loading > image {
>+ list-style-image: url("chrome://global/skin/icons/loading_16.png");
>+}
Can you reduce the selector to just ".loading"? The image node should automatically pick up the list-style-image, I think.
>+/* Maximize the size of the viewport when the window is small */
>+@media all and (max-width: 800px) {
>+ .category-name {
>+ display: none;
>+ }
>+}
Looks like the categories should have tooltips for that case.
>+.addon-view[active="false"]:not([selected]) .name-container,
>+.addon-view[active="false"]:not([selected]) .creator,
>+.addon-view[active="false"]:not([selected]) .description,
>+.addon-view[active="false"]:not([selected]) .date-updated {
>+ color: GrayText;
> }
This looks like it should just be:
.addon-view[active="false"]:not([selected]) {
color: GrayText;
}
>-.warning, .pending, .error, .info {
>+.warning, .pending, .error {
> -moz-margin-start: 48px;
> }
> .download-progress .pause, .download-progress .cancel {
>+.download-progress .pause:hover, .download-progress .cancel:hover {
[etc.]
nit: line break after commas in selectors
>--- a/toolkit/themes/winstripe/mozapps/extensions/extensions.css
>+++ b/toolkit/themes/winstripe/mozapps/extensions/extensions.css
> #addons-page {
>- background-color: #c5ccd7;
> -moz-appearance: none;
>- margin: 20px;
>+ background-color: #CCD9EA;
>+ background-image: /* Fade-out texture at the top */
>+ -moz-linear-gradient(top, rgb(231, 237, 346), rgba(231, 237, 346, 0) 156px),
>+ /* Texture */
>+ url("chrome://mozapps/skin/extensions/background-texture.png");
>+ background-repeat: repeat;
>+ padding: 18px;
>+ color: #000;
>+}
>+@media all and (-moz-windows-classic) {
>+ #addons-page {
>+ background-color: -moz-dialog;
>+ background-image: /* Fade-out texture at the top, and blend with browser tab */
>+ -moz-linear-gradient(rgba(255, 255, 255, 0.5), rgba(255, 255, 255, 0) 30%),
>+ /* Texture */
>+ url("chrome://mozapps/skin/extensions/background-texture.png");
>+ }
> }
Why is classic special-cased here? If it's because the colors might not fit, then the logic should be reversed, i.e. the bottom part should be used unconditionally and the top part should be used depending on -moz-windows-default-theme.
>+@media all and (-moz-windows-compositor) {
Are you sure you want to exclude aero basic here?
> #back-btn:-moz-locale-dir(ltr),
> #forward-btn:-moz-locale-dir(rtl) {
> -moz-image-region: rect(0, 18px, 18px, 0);
>+ -moz-border-radius-topright: 0px;
>+ -moz-border-radius-bottomright: 0px;
Use the non-prefixed border-radius properties (applies elsewhere too).
>+ border
> }
typo
>+.addon-view[active="false"] .name-container,
>+.addon-view[active="false"] .creator,
>+.addon-view[active="false"] .description,
>+.addon-view[active="false"] .date-updated {
>+ color: #888A8B;
>+}
Same suggestion as for gnomestripe:
.addon-view[active="false"] {
color: #888A8B;
}
>+.header-button {
>+ -moz-appearance: none;
>+ margin: 0px;
>+ background: rgba(151,152,153,.05)
>+ -moz-linear-gradient(rgba(251, 252, 253, 0.95), rgba(246, 247, 248, 0.47) 49%,
>+ rgba(231, 232, 233, 0.45) 51%, rgba(225, 226, 229, 0.3));
>+ background-clip: padding-box;
>+ border-radius: 4.5px;
>+ border: 1px solid;
>+ border-color: rgba(0, 0, 0, 0.12) rgba(0, 0, 0, 0.19) rgba(0, 0, 0, 0.38);
>+ box-shadow: 0 0 0 1px rgba(255, 255, 255, 0.3) inset,
>+ 0 0 0 2px rgba(255, 255, 255, 0.1) inset;
>+}
>+
>+.header-button[disabled="true"] {
>+ -moz-border-top-colors: rgba(0, 0, 0, 0.12) !important;
>+ -moz-border-left-colors: rgba(0, 0, 0, 0.19) !important;
>+ -moz-border-right-colors: rgba(0, 0, 0, 0.19) !important;
>+ -moz-border-bottom-colors: rgba(0, 0, 0, 0.38) !important;
>+ opacity: 0.8;
>+}
Why are you switching to -moz-border-*-colors all of a sudden?
Assignee | ||
Comment 29•14 years ago
|
||
(In reply to comment #28)
> >+.loading > image {
> >+ list-style-image: url("chrome://global/skin/icons/loading_16.png");
> >+}
>
> Can you reduce the selector to just ".loading"? The image node should
> automatically pick up the list-style-image, I think.
Yes, that worked.
> >+/* Maximize the size of the viewport when the window is small */
> >+@media all and (max-width: 800px) {
> >+ .category-name {
> >+ display: none;
> >+ }
> >+}
>
> Looks like the categories should have tooltips for that case.
I had originally been planning on doing that in a separate bug, but I've done it here instead.
> >+@media all and (-moz-windows-classic) {
> >+ #addons-page {
> >+ background-color: -moz-dialog;
> >+ background-image: /* Fade-out texture at the top, and blend with browser tab */
> >+ -moz-linear-gradient(rgba(255, 255, 255, 0.5), rgba(255, 255, 255, 0) 30%),
> >+ /* Texture */
> >+ url("chrome://mozapps/skin/extensions/background-texture.png");
> >+ }
> > }
>
> Why is classic special-cased here? If it's because the colors might not fit,
> then the logic should be reversed, i.e. the bottom part should be used
> unconditionally and the top part should be used depending on
> -moz-windows-default-theme.
Ok, done.
> >+@media all and (-moz-windows-compositor) {
>
> Are you sure you want to exclude aero basic here?
Yes - the additional gradients in the backgrond should only apply when Aero glass is enabled (I had already asked Boriss about that).
> >+.header-button[disabled="true"] {
> >+ -moz-border-top-colors: rgba(0, 0, 0, 0.12) !important;
> >+ -moz-border-left-colors: rgba(0, 0, 0, 0.19) !important;
> >+ -moz-border-right-colors: rgba(0, 0, 0, 0.19) !important;
> >+ -moz-border-bottom-colors: rgba(0, 0, 0, 0.38) !important;
> >+ opacity: 0.8;
> >+}
>
> Why are you switching to -moz-border-*-colors all of a sudden?
Because I need to override the style set in button.css - which in this case uses -moz-border-*-colors with the !important flag. Using just border-color doesn't seem to override that.
Comment 30•14 years ago
|
||
Comment on attachment 484540 [details] [diff] [review]
WiP v3 (Windows and Linux)
I'd like to see a new patch.
Attachment #484540 -
Flags: review?(dao) → review-
Assignee | ||
Comment 31•14 years ago
|
||
* Review comments addressed
* Fixes for Windows XP
* EULA dialog updated
* About dialog updated
* Re-did the alert boxes so they're all uniform and generally fit in better
* Numerous small tweaks and fixes everywhere
I might yet tweak the download progress widget. It doesn't fit in with the rest of the theme as nicely as I want.
Although the OSX theme isn't done in this patch, I've made sure that it isn't broken by the various XUL changes. Assuming there are no major things left to fix in the Windows theme, I'll start adapting it to OSX. If the tree unfreezes before the OSX theme is finished, it might be nice to land Windows/Linux ahead of OSX.
Attachment #484540 -
Attachment is obsolete: true
Attachment #487325 -
Flags: review?(dao)
Assignee | ||
Updated•14 years ago
|
Attachment #487325 -
Attachment is patch: true
Attachment #487325 -
Attachment mime type: application/octet-stream → text/plain
Attachment #487325 -
Flags: review?(dtownsend)
Comment 32•14 years ago
|
||
Comment on attachment 487325 [details] [diff] [review]
WiP v4 (Windows and Linux)
>diff --git a/toolkit/mozapps/extensions/content/about.js b/toolkit/mozapps/extensions/content/about.js
> function init() {
> var addon = window.arguments[0];
> var extensionsStrings = document.getElementById("extensionsStrings");
>
>+ var dialog = document.getElementById("genericAbout");
>+ dialog.setAttribute("addontype", addon.type);
Just use document.documentElement.setAttribute(...
>diff --git a/toolkit/mozapps/extensions/content/eula.js b/toolkit/mozapps/extensions/content/eula.js
> function Startup() {
> var bundle = document.getElementById("extensionsStrings");
>- var label = document.createTextNode(bundle.getFormattedString("eulaHeader", [window.arguments[0].name]));
>+ var addon = window.arguments[0].addon;
>+
>+ document.getElementById("eula-dialog").setAttribute("addontype", addon.type);
Again just document.documentElement seems fine.
>diff --git a/toolkit/mozapps/extensions/content/extensions.xul b/toolkit/mozapps/extensions/content/extensions.xul
> <!-- category list -->
> <richlistbox id="categories" persist="last-selected">
> <richlistitem id="category-search" value="addons://search/"
> class="category"
>- name="&view.search.label;" disabled="true"/>
>+ name="&view.search.label;"
>+ tooltiptext="&view.search.label;" disabled="true"/>
Is there much point in having the same text as label and tooltip?
>diff --git a/toolkit/mozapps/extensions/test/browser/browser_searching.js b/toolkit/mozapps/extensions/test/browser/browser_searching.js
>+ // Check the "first" and "last" attributes are set correctly
>+ for (let i = 0; i < actualResults.length; i++) {
>+ if (i == 0) {
>+ is(actualResults[0].item.hasAttribute("first"), true,
>+ "First item should have 'first' attribute set");
>+ is(actualResults[0].item.hasAttribute("last"), false,
>+ "First item should not have 'last' attribute set");
>+ } else if (i == (actualResults.length - 1)) {
>+ is(actualResults[actualResults.length - 1].item.hasAttribute("first"), false,
>+ "Last item should not have 'first' attribute set");
>+ is(actualResults[actualResults.length - 1].item.hasAttribute("last"), true,
>+ "Last item should have 'last' attribute set");
>+ } else {
>+ is(actualResults[i].item.hasAttribute("first"), false,
>+ "First item should not have 'first' attribute set");
>+ is(actualResults[i].item.hasAttribute("last"), false,
>+ "First item should not have 'last' attribute set");
s/First item/item i/
>diff --git a/toolkit/themes/winstripe/mozapps/extensions/eula.css b/toolkit/themes/winstripe/mozapps/extensions/eula.css
>--- a/toolkit/themes/winstripe/mozapps/extensions/eula.css
>+++ b/toolkit/themes/winstripe/mozapps/extensions/eula.css
>@@ -1,17 +1,54 @@
>+/*
>+ @media all and (-moz-windows-compositor) {
>+ dialog {
>+ -moz-appearance: -moz-win-borderless-glass;
>+ background: transparent;
>+ padding: 0;
>+ }
>+
>+ #content {
>+ border: 1px solid rgb(40%, 40%, 40%);
>+ background: -moz-Dialog;
>+ }
>+}
>+*/
Why is this commented out? Either uncomment it or remove it.
Attachment #487325 -
Flags: review?(dtownsend) → review+
Assignee | ||
Comment 33•14 years ago
|
||
(In reply to comment #32)
> >diff --git a/toolkit/mozapps/extensions/content/extensions.xul b/toolkit/mozapps/extensions/content/extensions.xul
>
> > <!-- category list -->
> > <richlistbox id="categories" persist="last-selected">
> > <richlistitem id="category-search" value="addons://search/"
> > class="category"
> >- name="&view.search.label;" disabled="true"/>
> >+ name="&view.search.label;"
> >+ tooltiptext="&view.search.label;" disabled="true"/>
>
> Is there much point in having the same text as label and tooltip?
I added the tooltips because I have the category names collapsing when the window is less than 800px wide (makes a *huge* difference in the size of the view-port at those small resolutions). Right now I have the tooltips there regardless of whether the category name is shown or not - with a hack in the category binding I could make it so its only there when the name is hidden, but I didn't see any real benefit.
> Why is this commented out? Either uncomment it or remove it.
Oops - that was meant to have been removed.
Comment 34•14 years ago
|
||
Once Bug 581193 is fixed, the default action for the button in add-on manager may be set as "Check for Updates" with the other options appearing in a dropdown menu.
Assignee | ||
Updated•14 years ago
|
Assignee | ||
Updated•14 years ago
|
Comment 35•14 years ago
|
||
Comment on attachment 487325 [details] [diff] [review]
WiP v4 (Windows and Linux)
>--- a/toolkit/themes/winstripe/mozapps/extensions/about.css
>+++ b/toolkit/themes/winstripe/mozapps/extensions/about.css
>+@media all and (-moz-windows-compositor) {
>+ #genericAbout {
>+ -moz-appearance: -moz-win-borderless-glass;
>+ background: transparent;
>+ }
>+
>+ #clientBox {
>+ border: 1px solid rgb(40%, 40%, 40%);
>+ }
>+}
Just use -moz-win-glass and skip the custom border?
>--- a/toolkit/themes/winstripe/mozapps/extensions/eula.css
>+++ b/toolkit/themes/winstripe/mozapps/extensions/eula.css
>@@ -1,17 +1,54 @@
>+/*
>+ @media all and (-moz-windows-compositor) {
>+ dialog {
>+ -moz-appearance: -moz-win-borderless-glass;
>+ background: transparent;
>+ padding: 0;
>+ }
>+
>+ #content {
>+ border: 1px solid rgb(40%, 40%, 40%);
>+ background: -moz-Dialog;
>+ }
>+}
>+*/
Why is this commented out?
Comment 36•14 years ago
|
||
Comment on attachment 487325 [details] [diff] [review]
WiP v4 (Windows and Linux)
>--- a/toolkit/themes/winstripe/mozapps/extensions/extensions.css
>+++ b/toolkit/themes/winstripe/mozapps/extensions/extensions.css
> #addons-page {
>- background-color: #c5ccd7;
> -moz-appearance: none;
>- margin: 20px;
>+ padding: 18px;
>+ color: #000;
>+ background-repeat: repeat;
>+ background-color: -moz-dialog;
>+ background-image: /* Fade-out texture at the top, and blend with browser tab */
>+ -moz-linear-gradient(rgba(255, 255, 255, 0.5), rgba(255, 255, 255, 0) 30%),
>+ /* Texture */
>+ url("chrome://mozapps/skin/extensions/background-texture.png");
>+}
>+
Looks like you should be using color: -moz-dialogText here.
>+@media all and (-moz-windows-theme: aero) {
>+ #addons-page {
>+ background-color: #CCD9EA;
>+ background-image: /* Fade-out texture at the top */
>+ -moz-linear-gradient(top, rgb(231, 237, 246), rgba(231, 237, 246, 0) 156px),
>+ /* Texture */
>+ url("chrome://mozapps/skin/extensions/background-texture.png");
>+ }
>+}
>+
>+@media all and (-moz-windows-compositor) {
>+ #addons-page {
>+ border: 1px solid rgb(40%, 40%, 40%);
>+ border-top: none;
>+ /* Blame shorlander for this monstrosity. */
>+ background-image: /* Fade-out texture and light beams at the top */
>+ -moz-linear-gradient(top, rgb(231, 237, 246) 3px, rgba(231, 237, 246, 0) 156px),
>+ /* Side gradients */
>+ -moz-linear-gradient(left,
>+ rgba(255, 255, 255, 0.2), rgba(255, 255, 255, 0) 40%,
>+ rgba(255, 255, 255, 0) 60%, rgba(255, 255, 255, 0.2)),
>+ /* Aero-style light beams */
>+ -moz-linear-gradient(left 32deg,
>+ /* First light beam */
>+ rgba(255, 255, 255, 0) 19.5%, rgba(255, 255, 255, 0.1) 20%,
>+ rgba(255, 255, 255, 0.1) 21.5%, rgba(255, 255, 255, 0.2) 22%,
>+ rgba(255, 255, 255, 0.2) 25.5%, rgba(255, 255, 255, 0.1) 26%,
>+ rgba(255, 255, 255, 0.1) 27.5%, rgba(255, 255, 255, 0) 28%,
>+ /* Second light beam */
>+ rgba(255, 255, 255, 0) 49.5%, rgba(255, 255, 255, 0.1) 50%,
>+ rgba(255, 255, 255, 0.1) 52.5%, rgba(255, 255, 255 ,0.2) 53%,
>+ rgba(255, 255, 255, 0.2) 54.5%, rgba(255, 255, 255, 0.1) 55%,
>+ rgba(255, 255, 255, 0.1) 57.5%, rgba(255, 255, 255, 0) 58%,
>+ /* Third light beam */
>+ rgba(255, 255, 255, 0) 87%, rgba(255, 255, 255, 0.2) 90%),
>+ /* Texture */
>+ url("chrome://mozapps/skin/extensions/background-texture.png");
>+ }
> }
Move to extensions-aero.css (and replace -moz-windows-theme: aero with -moz-windows-default-theme).
nit: removing the spaces within rgba() will make this slightly more readable as a whole, I think.
> .name-container {
> font-size: 150%;
>- margin-bottom: 0px;
>+ font-weight: bold;
>+ color: #3F3F3F;
>+ margin-bottom: 0;
>+ text-shadow: 1px 1px 1px #FFF;
trailing spaces
>+.addon-control:active {
>+ background-color: transparent;
>+ border-color: rgba(0, 0, 0, 0.65) rgba(0, 0, 0, 0.55) rgba(0, 0, 0, 0.5);
>+ box-shadow: 0 0 6.5px rgba(0, 0, 0, 0.4) inset,
>+ 0 0 2px rgba(0, 0, 0, 0.4) inset,
>+ 0 1px 0 rgba(255, 255, 255, 0.4);
>+}
Use :active:hover instead of :active? This may apply elsewhere too.
Assignee | ||
Comment 37•14 years ago
|
||
(In reply to comment #36)
> Move to extensions-aero.css (and replace -moz-windows-theme: aero with
> -moz-windows-default-theme).
Any specific reason why? I'd like to avoid it if possible - there's very little that's Aero-specific, so it wouldn't reduce the complexity of extensions.css by much. I asked Dave, and he agrees it'd be nice to avoid - but we both may be missing something.
Comment 38•14 years ago
|
||
It's a waste to have the code loaded, parsed etc. for non-aero Windows.
You may keep all code in one file and use preprocessing -- see http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/popup.css and http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/popup-aero.css
Assignee | ||
Comment 39•14 years ago
|
||
Changes in this patch:
* OSX theme
* Split out aero-specific stuff
* Buttons in the header are now toolbarbuttons (needed for the utilities button to look good on OSX, but I think it generally makes sense for them to be toolbarbuttons)
* Changed the all-results link in the search view to get its label/href set as an attribute (rather than a property), as I noticed some XBL-async-binding weirdness.
* New download progressbar (Boriss is going to attach mockups somewhere)
* Changes to gnomestripe to make certain things much more readable, and generally adapt better to different system themes
* Small tweaks here and there
Attachment #487325 -
Attachment is obsolete: true
Attachment #489749 -
Flags: review?(dao)
Attachment #487325 -
Flags: review?(dao)
Assignee | ||
Updated•14 years ago
|
Attachment #489749 -
Flags: feedback?(dtownsend)
Assignee | ||
Comment 40•14 years ago
|
||
Assignee | ||
Comment 41•14 years ago
|
||
Assignee | ||
Comment 42•14 years ago
|
||
Note: In the latest patch, the icons of disabled addons in view view may not show. This seems to be due to the greyscale SVG filter (which used to work perfectly) - filed bug 611241.
(In reply to comment #35)
> Just use -moz-win-glass and skip the custom border?
I tried that at first, but the top glass border was drawn about 22px below the top of the client box. I don't know enough about -moz-win-glass to tell if its a bug or something weird about that dialog box - and using -moz-win-borderless-glass worked ok, so I didn't spend time investigating.
Comment 43•14 years ago
|
||
(In reply to comment #42)
> > Just use -moz-win-glass and skip the custom border?
>
> I tried that at first, but the top glass border was drawn about 22px below the
> top of the client box. I don't know enough about -moz-win-glass to tell if its
> a bug or something weird about that dialog box - and using
> -moz-win-borderless-glass worked ok, so I didn't spend time investigating.
It's a bug that needs fixing (bug 606061). Please don't work around it.
Comment 44•14 years ago
|
||
Comment on attachment 489749 [details] [diff] [review]
v5 (Windows, Linux, OSX)
>diff --git a/toolkit/mozapps/extensions/content/extensions.xml b/toolkit/mozapps/extensions/content/extensions.xml
> <xul:button anonid="undo-btn" class="button-link"
> label="&addon.undoAction.label;"
> tooltipText="&addon.undoAction.tooltip;"
> oncommand="document.getBindingParent(this).undo();"/>
> <xul:spacer flex="5000"/> <!-- Necessary to allow the message to wrap -->
> </xul:hbox>
>
> <xul:hbox>
>- <xul:vbox align="center" pack="center" class="icon-container">
>+ <xul:vbox align="center" pack="start" class="icon-container">
> <xul:image anonid="icon" class="icon"/>
> </xul:vbox>
> <xul:vbox flex="1">
> <xul:hbox align="start">
> <xul:vbox flex="1">
> <xul:hbox class="name-container" align="end">
I didn't do this because the UI mockups called for the icon to be centered in the regular list view. I don't think it looks right this way, maybe add some padding to the top so it is roughly centered in the normal list but stays put in the expanded item.
Attachment #489749 -
Flags: feedback?(dtownsend) → feedback+
Assignee | ||
Comment 45•14 years ago
|
||
Note: This is on top of the patch in bug 602895, which hasn't landed yet. It adds a glow effect to the global warning icon when the warning message is collapsed (as described in bug 602895 comment 3), and changes the threshold of when the warning message will collapse (because this patch changes the amount of space the warning message has at smaller window sizes).
Also addresses comment 43 and comment 44.
Attachment #489749 -
Attachment is obsolete: true
Attachment #490764 -
Flags: review?(dao)
Attachment #490764 -
Flags: feedback?(dtownsend)
Attachment #489749 -
Flags: review?(dao)
Assignee | ||
Comment 46•14 years ago
|
||
Forgot to mention: I purposefully did not add a glow effect to the icon in gnomestripe, since I don't think that could be made to look good on all themes.
Comment 47•14 years ago
|
||
Can we get screenshots, at least for Win? There were only WIP v1 screenshots for it.
Updated•14 years ago
|
Attachment #490764 -
Flags: feedback?(dtownsend) → feedback+
Assignee | ||
Comment 48•14 years ago
|
||
(In reply to comment #47)
> Can we get screenshots, at least for Win? There were only WIP v1 screenshots
> for it.
Nothing major in the look has changed - mostly just a lot of smaller tweaks, and cleaning up the code. I think the big noticeable difference in this screenshot is the download progress.
Attachment #482399 -
Attachment is obsolete: true
Assignee | ||
Comment 49•14 years ago
|
||
And here's one of the message boxes, in the search view. I re-sized the window, to show how the categories list on the left collapses to give more room to the main content.
Comment 50•14 years ago
|
||
Blair, I miss the tabshadow as given by the mockups by Jennifer. What happened with those?
Assignee | ||
Comment 51•14 years ago
|
||
(In reply to comment #50)
> Blair, I miss the tabshadow as given by the mockups by Jennifer. What happened
> with those?
Do you mean the shadow inside the viewport and the selected category? Sadly, that was one of the things in the mockups that wasn't realistic to do. Same with extending the background color of the list items into the selected category, and extending the background of about:addons into its browser tab (though we figured out a compromise for that).
Comment 52•14 years ago
|
||
(In reply to comment #51)
> Do you mean the shadow inside the viewport and the selected category? Sadly,
> that was one of the things in the mockups that wasn't realistic to do. Same
Yes, exactly that part. Looks like I missed comment 5. Does it mean it is not possible at all? Otherwise we should file a new bug to get it implemented at a later stage.
> with extending the background color of the list items into the selected
> category, and extending the background of about:addons into its browser tab
> (though we figured out a compromise for that).
It's because of the hbox and that both parts are positioned beneath each other, right? Is there a way to overlay items in XUL? Means putting the category list on top of the list, extend the list to the left, and add a padding at the left?
Comment 53•14 years ago
|
||
Comment on attachment 490764 [details] [diff] [review]
Patch v6
>--- a/toolkit/themes/pinstripe/mozapps/extensions/extensions.css
>+++ b/toolkit/themes/pinstripe/mozapps/extensions/extensions.css
>
> .download-progress .progress {
>- background-color: transparent;
> /* Force the progress meter to use the default binding rather than the OSX
> animated binding */
> -moz-binding: url("chrome://global/content/bindings/progressmeter.xml#progressmeter") !important;
This isn't needed anymore. http://hg.mozilla.org/mozilla-central/rev/a0351ac4eb45
>--- a/toolkit/themes/winstripe/mozapps/extensions/extensions.css
>+++ b/toolkit/themes/winstripe/mozapps/extensions/extensions.css
> #addons-page {
[...]
>+%ifdef WINSTRIPE_AERO
>+ color: #000;
>+ background-color: #CCD9EA;
>+ background-image: /* Fade-out texture at the top */
>+ -moz-linear-gradient(top, rgb(231,237,246), rgba(231,237,246,0) 156px),
>+ /* Texture */
>+ url("chrome://mozapps/skin/extensions/background-texture.png");
>+%else
>+ color: -moz-dialogText;
>+ background-color: -moz-dialog;
>+ background-image: /* Fade-out texture at the top, and blend with browser tab */
>+ -moz-linear-gradient(rgba(255,255,255,0.5), rgba(255,255,255,0) 30%),
>+ /* Texture */
>+ url("chrome://mozapps/skin/extensions/background-texture.png");
>+%endif
> }
>
>+%ifdef WINSTRIPE_AERO
>+@media all and (-moz-windows-compositor) {
>+ #addons-page {
[...]
>+ }
>+}
>+%endif
Rather than Classic or third-party themes on Win 7, I suspect you want to target aero basic with the first %ifdef WINSTRIPE_AERO section. Here's this executed correcty:
> #addons-page {
[...]
> color: -moz-dialogText;
> background-color: -moz-dialog;
> background-image: /* Fade-out texture at the top, and blend with browser tab */
> -moz-linear-gradient(rgba(255,255,255,0.5), rgba(255,255,255,0) 30%),
> /* Texture */
> url("chrome://mozapps/skin/extensions/background-texture.png");
> }
>
> %ifdef WINSTRIPE_AERO
> @media all and (-moz-windows-default-theme) {
> #addons-page {
> color: #000;
> background-color: #CCD9EA;
> background-image: /* Fade-out texture at the top */
> -moz-linear-gradient(top, rgb(231,237,246), rgba(231,237,246,0) 156px),
> /* Texture */
> url("chrome://mozapps/skin/extensions/background-texture.png");
> }
> }
> @media all and (-moz-windows-compositor) {
> #addons-page {
[...]
> }
> }
> %endif
Otherwise, lots of hardcoded colors in winstripe. To ensure legibility with various OS themes, I hope you hardcoded all the background and foreground colors consistently rather than mixing them with native ones.
Ideally, you'd use native colors throughout like in gnomestripe and wrap all the hardcoded stuff in a @media all and (-moz-windows-default-theme) { } section.
Attachment #490764 -
Flags: review?(dao) → review+
Comment 54•14 years ago
|
||
(In reply to comment #53)
> I hope you hardcoded all the background and foreground
> colors consistently rather than mixing them with native ones.
And by that I don't mean you explicitly using a platform color but merely omitting e.g. a text color such that winstripe/global/ styling would kick in with a color which might be black in your case but could be anything else in the wild.
Comment 55•14 years ago
|
||
Landed with changes to address Dão's comments. I tested the patch with Windows classic and high contrast themes to make sure the text was still legible.
http://hg.mozilla.org/mozilla-central/rev/367398a8cbfe
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Target Milestone: --- → mozilla2.0b8
Comment 56•14 years ago
|
||
The background ( http://tinyurl.com/2upbong ) is quiet different than the screenshot ( https://bugzilla.mozilla.org/attachment.cgi?id=491016 ). Is this a bug, or just you made the screenshot with other patches applied?
Comment 57•14 years ago
|
||
(In reply to comment #56)
> The background ( http://tinyurl.com/2upbong ) is quiet different than the
> screenshot ( https://bugzilla.mozilla.org/attachment.cgi?id=491016 ). Is this a
> bug, or just you made the screenshot with other patches applied?
It should be following your windows color preferences, it looks ok for me. If not please file a new bug.
Comment 58•14 years ago
|
||
Marking as verified fixed with builds on all platforms like Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101125 Firefox/4.0b8pre ID:20101125030318
Status: RESOLVED → VERIFIED
Flags: in-testsuite-
Flags: in-litmus-
Comment 59•14 years ago
|
||
I see the same issue. :WonderCsabo have you filled a bug?(In reply to comment #56)
> The background ( http://tinyurl.com/2upbong ) is quiet different than the
> screenshot ( https://bugzilla.mozilla.org/attachment.cgi?id=491016 ). Is this a
> bug, or just you made the screenshot with other patches applied?
I see the same issue. :WonderCsabo have you filled a bug?
Comment 60•14 years ago
|
||
I didn't file one, because I still dunno this is a bug or not.
It's obvious that the screenshot made with the "hide main browser chrome" patch, maybe that effects that.
Waiting for Blair to give feedback on this.
Updated•14 years ago
|
Comment 61•14 years ago
|
||
Csaba, lets file a bug! Even when it's unclear what the situation is. I just want to make sure that it doesn't get lost. Thanks.
Comment 62•14 years ago
|
||
Ok, filed bug Bug 614824. I hope the decorations will make, it looks much better, and reminds me of shorlander's beautiful mockups of in-content UI.
Comment 63•14 years ago
|
||
Marking as verified fixed with build: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101125 Firefox/4.0b8pre ID: 20101125030318
Comment 64•14 years ago
|
||
Please ignore the last comment, it was intended for bug 614152.
Updated•14 years ago
|
Comment 65•14 years ago
|
||
From Bug 588764 comment #60:
> The new Addon manager magically creates a good looking border around the
> content, why?
Assignee | ||
Comment 66•14 years ago
|
||
(In reply to comment #65)
> From Bug 588764 comment #60:
> > The new Addon manager magically creates a good looking border around the
> > content, why?
See bug 614838.
You need to log in
before you can comment on or make changes to this bug.
Description
•