Closed Bug 1141425 Opened 9 years ago Closed 6 years ago

[Stingray][Component] Merge icons of smart-icons in gaia-icons

Categories

(Firefox OS Graveyard :: Gaia::TV, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: dwi2, Unassigned)

References

Details

Attachments

(1 file)

53 bytes, text/x-github-pull-request
wilsonpage
: feedback-
Details | Review
We are going to move smart-icons component:
https://github.com/smart-components/smart-icons
to gaia-components.

I think what we need to do are:
1. open a smart-icons repo in gaia-components;
2. Review and check-in smart-icons there
   (Pull request 1: to gaia-components/smart-icons);
3. Make apps that is using smart-icons point to gaia-components
   (Pull request 2: to gaia).
Depends on: 1140982
I haven't yet looked at the style of these icons yet. Is it possible to merge them with the existing gaia-icons? If not, should we consider adding theming, or replacement icon support to it? https://github.com/gaia-components/gaia-icons
Hi Kevin,

Did you mean 
1) to merge smart-icons font file and gaia-icons font file into one common font file (and let people use different font-family for smart-icons and gaia-icons)
or 
2) to put smart-icons.css and smart-icons.ttf into existing gaia-components/gaia-icons folder ? (and let people use different set of icons by including different css file)
(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #2)
> Hi Kevin,
> 
> Did you mean 

Yes, I meant one of these :) I have not done an analysis of the various icons and use cases yet, so I'm not sure what would work best. My preference would be to merge the font files into a common file if possible.
Post my findings here.

It seems that some of the icon names in smart-icons are collided with that in gaia-icons. For example, both smart-icons and gaia-icons have icons named 'add', 'menu', 'more', ... etc. But I guess some of them probably are the same icon in both smart-icons and gaia-icons. If we are going to merge two font files together, we probably need to change some icon names (or maybe not) in smart-icons (and also the html files who use them) and regenerate into one font file. 

I think we need some help to generate font file from UX team if we decide to do that.

Hi Scott,
 I am wondering if you have any concern about merging font icons of phone (gaia-icons) and font icons of smart-screen (smart-icons).
Flags: needinfo?(scwu)
Hi Tzu-Lin,

I think we had this discussion with John and Mike a little while. And our conclusion is to separate the font files due to a couple reasons. 1. we want the icon style to be flexible and different. 2. Gaia-icon is owned by other group, not us. And we cannot control and ensure all the icons are always up to date. 

thanks,
Scott
Flags: needinfo?(scwu)
Assignee: nobody → tzhuang
Thanks Scott. 
I'll merge smart-icons repo into gaia-icons but keep two font files separated then. (which is option 2 in comment 2)
Status: NEW → ASSIGNED
I agree with Kevin. Iconography should be all in one place to keep everyone sane. If TV needs a slightly different variation we can simply add an extra icon to the set eg: 'add.svg' and 'add-2.svg'. Leaving the designer to choose the most appropriate 'add' icon for their use-case.
I just examined svg(s) of gaia-components/gaia-icons and smart-components/smart-icons and found they have some essential difference:
gaia-icons: document is 30px*30px, with only one <path> in it
smart-icons: document is 54px*54px, often with multiple <path> in it

And it's rare to use both gaia-icons and smart-icons in one single app at the same time. So I have another proposal. Take Scott's, Kevin's, and Wilson's idea into consideration, I say what if we split icon svg(s) into two folders (images/gaia-icons/ and images/gaia-tv-icons/) and modify grunt script to generate two sets of ttf and css files:
  fonts/gaia-icons.ttf
  fonts/gaia-tv-icons.ttf
  gaia-icons-embedded.css
  gaia-icons.css
  gaia-tv-icons-embedded.css
  gaia-tv-icons.css

Folks could choose which set of icons they'd like to use by include different css file.
Does that sound reasonable to you?
Flags: needinfo?(wilsonpage)
Flags: needinfo?(kgrandon)
It sounds ok to me, I'll leave it up to Wilson though.
Flags: needinfo?(kgrandon)
That sounds a little fragmented to me. Can't we just scale down the 54x54 smart-icons to match the existing gaia-icons? This will have no impact on the size of the icons 'in app'.

The Firefox OS project should have a single icon set.
Flags: needinfo?(wilsonpage)
> That sounds a little fragmented to me. Can't we just scale down the 54x54
> smart-icons to match the existing gaia-icons? 

I cannot answer this question. Does scaling TV icons down to 30x30 make sense to you, Scott?

> The Firefox OS project should have a single icon set.

I agree that in the long run there should be only one single icon set in Firefox OS project. But I think for now our UX team has other concern about merging icon set of TV into original icon set (See comment 5). 

And we should figure a way out to merge icon sets before we have a new set of icons (say, for simple phones or watches). But if we do want icons of TV and phones reside in the same repository for now, given that TV icons and phone icons are two different style of icons, split folders (thus split font files) is probably a solution for now.
Flags: needinfo?(scwu)
Attached file pull request
In fact, I just made a patch. 
Set f? for now until we have Scott's opinion about scaling TV icons to 30x30.
Attachment #8579802 - Flags: feedback?(wilsonpage)
Attachment #8579802 - Flags: feedback?(kgrandon)
Comment on attachment 8579802 [details] [review]
pull request

Thanks. I am going to defer this to Wilson.
Attachment #8579802 - Flags: feedback?(kgrandon)
Hey guys, thanks for trying to figure out which and what solution works best.
I'm more prefer to go option #2 to separate the icon fonts since there are not much icons will be used in TV, not to mention the TV icon for phone.

Scaling down all the svg icons, yes, will definitely need sometime to resize them. Also, we will need to go over again with all of our visual spec for which icon name is request to update/rename. And for developers, they will need to revise their code, of course.

Let me know what you guys think.

Thanks!
Flags: needinfo?(scwu)
Comment on attachment 8579802 [details] [review]
pull request

I don't agree with this split. Firefox OS is a single project and should have consistent iconography across all its forms. 

I can see many duplicate icons between the two sets already:

- bluetooth
- more
- menu
- picture
- refresh
- reload

To be honest I can't that many brand-new icons.

This small decision now will ensure the future visual consistency across our products. Creating a new font now just gives permission for another font to be made, where do we stop? If we don't merge the two sets now, it will never happen.

If there are conflicts I can help resolve them. The size of the source image isn't important as apps can scale them to their desired size.

Sorry to be annoying, but I feel this is very important to the growth of Firefox OS.
Attachment #8579802 - Flags: feedback?(wilsonpage) → feedback-
Summary: [Stingray][Component] Move smart-icons to gaia-components repository → [Stingray][Component] Merge icons of smart-icons in gaia-icons
based on comment 15, things need to do are:
1. To have UX team re-examine visual spec and svg(s) in smart-icons, and resize them down to 30x30
2. Rename icon names if there is any conflict
3. Add svg(s) from step 1 and step2
4. Regenerate gaia-icon font file.
Assignee: tzhuang → nobody
Status: ASSIGNED → NEW
Blocks: 1218722
No longer blocks: 1218722
Blocks: 1219540
No longer blocks: 1219540
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: