Closed Bug 1380043 Opened 7 years ago Closed 7 years ago

Update about:addons category icons

Categories

(Toolkit :: Add-ons Manager, defect, P1)

Product:
Toolkit
Component:
Add-ons Manager
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla57
Iteration:
57.2 - Aug 29
Tracking Flags:
Tracking Status
firefox57 --- verified

People

(Reporter: dao, Assigned: daleharvey)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-visual][p2])

Attachments

(3 files)

aboutaddons-photon-icon.zip
6.30 KB, application/zip
Details
Bug 1380043 - Update about:addons icons.
59 bytes, text/x-review-board-request
dao
: review+
Details
addons px.png
76.13 KB, image/png
Details 
Icons are in attachment 8870496 [details]
Flags: qe-verify+
QA Contact: brindusa.tot
Assignee: nobody → dharvey
Status: NEW → ASSIGNED
Iteration: --- → 57.1 - Aug 15
Priority: P2 → P1
So most of these go in fine, however not entirely certain what 'extensions-update.svg' is for, there doesnt seem to be a current equivalent and it looks like we currently handle updates by overlaying http://searchfox.org/mozilla-central/source/toolkit/themes/osx/mozapps/update/buttons.png which gives more information about the state of the update.

Is there another place for this icon or do we need more assets here?

Also while I am asking a needinfo, is http://i.imgur.com/IkQssqy.png ok or do we want to make these icon smaller? This is with their size unchanged but obviously the svg's scale differently
Flags: needinfo?(shorlander)
(In reply to Dale Harvey (:daleharvey) from comment #1)
> So most of these go in fine, however not entirely certain what
> 'extensions-update.svg' is for, there doesnt seem to be a current equivalent
> and it looks like we currently handle updates by overlaying
> http://searchfox.org/mozilla-central/source/toolkit/themes/osx/mozapps/
> update/buttons.png which gives more information about the state of the
> update.
> 
> Is there another place for this icon or do we need more assets here?
> 
> Also while I am asking a needinfo, is http://i.imgur.com/IkQssqy.png ok or
> do we want to make these icon smaller? This is with their size unchanged but
> obviously the svg's scale differently

Hi, Dale! I designed the icons. Can you give me the different cases of when we use the overlay? I probably miss something there, sorry.

Anyway, since I draw them, we changed our mind regarding the size  - we thought to move to 32x32, but for now, we stay with 24x24. It means I need to redraw them. I can send you the new assets by tomorrow evening (CEST time). I hope it works for you all.
Flags: needinfo?(dharvey)
> Can you give me the different cases of when we use the overlay? 

Sorry I dont know, I dont use addons extensively I just guessed from the name, can you say where "extensions-update.svg" is used or what current icon it is replacing?

> I can send you the new assets by tomorrow evening (CEST time). I hope it works for you all.

No problem, thanks
Flags: needinfo?(shorlander)
Flags: needinfo?(emanuela)
Flags: needinfo?(dharvey)
Ok, so here the latest assets for the sidebar in the about:addons category icons. The icons are dark since the sidebar shall be light as in about:preferences. 

I got a new update for the size: icons are going to be 16x16 (as they are right now).
Flags: needinfo?(emanuela)
(In reply to emanuela [ux team] from comment #4)
> Created attachment 8896326 [details]
> aboutaddons-photon-icon.zip
> 
> Ok, so here the latest assets for the sidebar in the about:addons category
> icons. The icons are dark since the sidebar shall be light as in
> about:preferences. 
> 
> I got a new update for the size: icons are going to be 16x16 (as they are
> right now).

Ahem, they're actually 24x24 right now.
Flags: needinfo?(emanuela)
(In reply to Dão Gottwald [::dao] from comment #5)
> (In reply to emanuela [ux team] from comment #4)
> > Created attachment 8896326 [details]
> > aboutaddons-photon-icon.zip
> > 
> > Ok, so here the latest assets for the sidebar in the about:addons category
> > icons. The icons are dark since the sidebar shall be light as in
> > about:preferences. 
> > 
> > I got a new update for the size: icons are going to be 16x16 (as they are
> > right now).
> 
> Ahem, they're actually 24x24 right now.

As soon as the size is consistent with about:preferences, I'm happy.
The icons in the zip are all svgs 16x16. You can scale them up for now.
Flags: needinfo?(emanuela)
I don't see Languages icon here. Can we get an updated icon for that as well?
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #8)
> I don't see Languages icon here. Can we get an updated icon for that as well?

I took the list of the icons from chrome://mozapps/skin/extensions/.

I'm aware we're currently missing the Languages icon. My suggestion so far it's to use the same icon of dictionaries. We still have the label to help the users.
Iteration: 57.1 - Aug 15 → 57.2 - Aug 29
I havent replaced https://searchfox.org/mozilla-central/source/toolkit/themes/osx/mozapps/plugins/pluginGeneric.png in this patch, its possible it could be replaced with category-plugins.svg however there are additional assets required if we are looking to change those (notifyPluginGeneric etc). I think if we do want to replace those (I wasnt certain) we should probably do so in a seperate bug
Comment on attachment 8897368 [details]
Bug 1380043 - Update about:addons icons.

https://reviewboard.mozilla.org/r/168686/#review173948

make[6]: *** /home/dao/mozilla/central/objdir-frontend/toolkit/themes/linux/mozapps/extensions/: No such file or directory.  Stop.
 0:17.65 /home/dao/mozilla/central/config/faster/rules.mk:75: recipe for target '/home/dao/mozilla/central/objdir-frontend/toolkit/themes/linux/mozapps/extensions/category-search.png' failed

::: toolkit/themes/shared/extensions/category-available.svg:4
(Diff revision 1)
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16">

All of these icons should use fill="context-fill"
Attachment #8897368 - Flags: review?(dao+bmo) → review-
Comment on attachment 8897368 [details]
Bug 1380043 - Update about:addons icons.

https://reviewboard.mozilla.org/r/168686/#review174406

The extension and appearance categories don't have icons with this patch (tested on Linux). chrome://mozapps/skin/extensions/category-extensions.svg and chrome://mozapps/skin/extensions/category-themes.svg don't seem to exist.
Attachment #8897368 - Flags: review?(dao+bmo) → review-
Comment on attachment 8897368 [details]
Bug 1380043 - Update about:addons icons.

https://reviewboard.mozilla.org/r/168686/#review174874
Attachment #8897368 - Flags: review?(dao+bmo) → review+
Comment on attachment 8897368 [details]
Bug 1380043 - Update about:addons icons.

https://reviewboard.mozilla.org/r/168686/#review174960

::: toolkit/themes/shared/extensions/category-extensions.svg:4
(Diff revision 3)
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="context-fill">

These are all missing fill-opacity="context-fill-opacity"
Attachment #8897368 - Flags: review-
Comment on attachment 8897368 [details]
Bug 1380043 - Update about:addons icons.

As this is pending autoland already (and there is no way to remove something from the autoland queue), then if this lands without the context-fill-opacity we can land that in a follow-up patch.
Attachment #8897368 - Flags: review-
Setting leave open and will follow up in here, I hadnt heard of context-fill-opacity before
Keywords: leave-open
Blocks: 1390171
Ah sorry, updated to fix tests and address above review, pushed to try @  https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7086194ec4bcad52244c9961fc039da53c54783
Flags: needinfo?(dharvey)
ok those are looking green to me, will rereview since I touched a lot more stuff
What's the status here?
Flags: needinfo?(dharvey)
Comment on attachment 8897368 [details]
Bug 1380043 - Update about:addons icons.

Sorry I meant to ask for review again
Flags: needinfo?(dharvey)
Attachment #8897368 - Flags: review+ → review?(dao+bmo)
Comment on attachment 8897368 [details]
Bug 1380043 - Update about:addons icons.

https://reviewboard.mozilla.org/r/168686/#review175930
Attachment #8897368 - Flags: review?(dao+bmo) → review+
Keywords: leave-open → checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f2b8d974db3c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Hi Dale,

Please tell us what we need to verify here, thanks.
Flags: needinfo?(dharvey)
QA Contact: brindusa.tot → ovidiu.boca
The icons in about:addons should be changed to the icons attached in this bug
Flags: needinfo?(dharvey)
Please see the attachment, this is with Nightly 57.0a1(2017-08-22) on Mac OS X 10.10.

I have 2 questions:

1. This are the expected icons, that we need to verify?
2. All the icons should be placed on 9px from the text? The last icon is placed on 10 px and the first 2 are placed on 9 px. 
Thanks
Flags: needinfo?(dharvey)
Ovidiu looks like you missed the attachment
Flags: needinfo?(dharvey)
Attached image addons px.png — 
Sorry about that.
No bother, those are the icons I expect, the exact spacing rendered may be different between icons with different padding but the spacing wasnt changed in this patch, will be coming from the icons design
Thanks Dale, based on your comment 37, I will mark this as verified fixed.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
Flags: qe-verify+
Depends on: 1393099
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: