Closed
Bug 948856
Opened 11 years ago
Closed 10 years ago
[Settings] Update setting icons to use Icon Fonts
Categories
(Firefox OS Graveyard :: Gaia::Settings, defect)
Tracking
(b2g-v2.1 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
b2g-v2.1 | --- | fixed |
People
(Reporter: epang, Assigned: arnau)
References
Details
(Whiteboard: ux-tracking, visual design, visual-tracking, bokken)
Attachments
(3 files, 2 obsolete files)
This bug is to replace the current setting icons (using a sprite) with icon fonts. This will help with the various icons sizes that are needed.
Assignee | ||
Comment 1•11 years ago
|
||
Vivien, we've had some difficulties with the icon font in status bar (bug 901989), so we have decided trying icon fonts in Settings app. I wanted to have your advice on how to proceed with the font. You once told me we should first land the font in gaia, and after that we should move it gecko. In this PR I have embedded the font inside icons.css. Please let me know if that works as a first step.
Please note this PR is intended to replace only the icons in Settings main screen. After we land it we will open bug/s to replace the rest of the icons in the app.
Attachment #8346889 -
Flags: review?(21)
Comment 2•11 years ago
|
||
Comment on attachment 8346889 [details]
patch in github
I know I'm a bit obsessed with this but can we add names to the icons in the font ? Right now this is very hard to say if you use the correct icon or not :)
Attachment #8346889 -
Flags: review?(21)
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 8346889 [details]
patch in github
Vivien, I've fixed your requests:
-Changed name for the font
-Removed unneeded gifs
-And for the naming of the icons, I have added an icons.css as a BB (/shared/style/icon.css) and created a cheatsheet (/shared/style/icons/index.html) to document all class names you have to use to get each icon.
This way it will be easier to link icons in each app without having to duplicate code.
Attachment #8346889 -
Flags: review?(21)
Comment 4•11 years ago
|
||
Comment on attachment 8346889 [details]
patch in github
The PR sounds good to me. Let's try it and see what happens.
Attachment #8346889 -
Flags: review?(21) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Thanks Vivien!
Merged: 16f95c8500a9f8eb793aa3501871edd292cd7968
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
No longer blocks: 2.0-visual-refresh
Updated•11 years ago
|
Blocks: Settings-1.4-VSD
Updated•11 years ago
|
No longer blocks: Settings-1.4-VSD
Comment 6•11 years ago
|
||
We saw a fairly large launch regression in settings for the time window where this landed. Can you check to see if this commit caused the problem?
To test run:
sudo pip install b2gperf
adb forward tcp:2828 tcp:2828
b2gperf --delay=10 Settings
And the look at the reported median value. Thank you!
Assignee | ||
Comment 7•11 years ago
|
||
After this patch (commit: 16f95c8500a9f8eb793aa3501871edd292cd7968):
Results for Settings, cold_load_time: median:900, mean:903, std: 20, max:986, min:881, all:986,910,891,887,904,905,909,907,899,909,890,891,904,881,892,891,891,898,956,882,918,896,893,903,920,902,907,901,895,896
Before this patch:
Results for Settings, cold_load_time: median:761, mean:799, std: 196, max:1857, min:739, all:802,762,764,768,756,758,752,757,739,748,759,761,758,757,748,1857,769,755,765,764,774,810,760,780,754,762,779,766,750,761
Comment 8•11 years ago
|
||
Thanks for running the test Arnau! Do you have any ideas why this is loading slower? If you think a fix might take some time we should probably back this out.
Assignee | ||
Comment 9•11 years ago
|
||
Ben, in this patch we are using icon fonts instead of gifs, Kevin run some tests in the past showing this should be faster than bitmaps. Something we wanted to do after this patch landed was moving the font into gecko, maybe if the font is preloaded we don't have this delay. I'll test that tomorrow.
Assignee | ||
Comment 10•11 years ago
|
||
BTW: should I only look at median value? Will any of the other values indicate anything useful when I do this new test?
Comment 11•11 years ago
|
||
The median value basically is just a better measure than mean for our tests since we get a lot of outliers. The median will not be skewed due to intermittent outliers.
Arnau, can we show where the icon gifs are faster? Is it during launch or scrolling or some other activity?
Finally, if we can show that the time to get to the first usable screen is actually faster with icon fonts, then its ok to regress cold launch time. We just need to show that if we can.
Finally, can we back this out in the meantime? We really need to avoid leaving large regressions in the tree while we investigate. If we can't figure it out tomorrow I will go ahead with the back out. Sorry! It can of course re-land later once the investigation is done. Thanks!
Assignee | ||
Comment 12•11 years ago
|
||
\o/
Moving the font to moztt repo improves the median value!
Results for Settings, cold_load_time: median:790, mean:797, std: 41, max:1011, min:767, all:1011,817,796,796,787,792,790,798,785,781,767,807,788,784,794,788,784,818,773,796,780,788,782,790,791,784,781,794,791,799
Ben, let me know if that works for you and I'll create a new patch.
Comment 13•11 years ago
|
||
Awesome! If I understand correctly that means we need to put the font in gonk? How will settings work if you're running on an older gonk without the font?
Flags: needinfo?(arnau)
Assignee | ||
Comment 14•11 years ago
|
||
Unfortunately if you don't update your gonk won't see the icons.
Here is the first step for this new patch: https://bugzilla.mozilla.org/show_bug.cgi?id=951593
After this lands I will remove the font embedded in icons.css, which is what is making Settings load slower.
Flags: needinfo?(arnau)
Comment 15•11 years ago
|
||
The problem here, though, is that we get the gonk firmware from the OEM. They just provided us with firmware for 1.2 in November. There is probably going to be a long lead time to get new firmware with the fonts on them. We could have months until we have OEM firmware with the new fonts rolled out.
Michael, Naoki, do you know of any better way to roll out new system fonts?
If it really will take that long, I'm sorry, but I must insist we back this out until the fonts are available. :-(
Flags: needinfo?(nhirata.bugzilla)
Flags: needinfo?(mwu)
Comment 16•11 years ago
|
||
Talking to Naoki in IRC, it appears getting new fonts through the OEM firmware will take at least a few weeks.
Flags: needinfo?(nhirata.bugzilla)
Comment 17•11 years ago
|
||
Kevin, Vivien, can you comment here? It sounds like using icon fonts in settings was a bit experimental.
Placing the icon font in gaia introduced a very large (150ms) launch regression.
Placing the icon font in mozTT fixed this launch regression and resulted in about a 10ms improvement. Also, it will take a while to get font through the OEM firmware process in order to have it in gonk on our automation devices.
Is it worth pursuing the icon font further if it did not help launch time very much? Or does it offer other improvements here?
Either way it seems we should back this out in the meantime due to the current launch regression.
Again, I want to thank Arnau for helping to investigate the problem and possible solutions here!
Flags: needinfo?(kgrandon)
Flags: needinfo?(21)
Comment 18•11 years ago
|
||
The main reasoning for icon fonts is actually resolution independence. It's something we definitely want.
I feel like the problem here is that we do not have the proper workflows in place to be able to adequately leverage icon fonts right now. Ideally we would have some scripts to install the fonts on top of any gonk, so we don't have to wait for partners. (I am not a font master, so I could be wrong about this)
I would like mwu to chime in to see if we can modify scripts to install fonts where needed on top of gonk. If this is not something we can do before the break then we should back this out.
Flags: needinfo?(kgrandon)
Assignee | ||
Comment 19•11 years ago
|
||
Please have in mind icon font is not mainly intended to improve performance, is a tool that will give us more flexibility in UI implementation:
1- We can use icons in different colors (e.g depending on the background color used in headers) without the need of create different assets.
2- Scaling: we don't need to duplicate assets for @x1.5, @x2 and in future @x3, @x4 plus tablet UI.
3- Having a centralized place for icons, if we are able to preload icons, we won't have the actual issue of having icons all over the apps in different folders, which makes assets difficult to maintain. Sometimes graphics are updated in an app and not in all places where it appears, so we avoid inconsistencies.
I guess all this reasons could be worth a small regression :)
So I'd like to have Patryk's feedback as well.
Flags: needinfo?(padamczyk)
Comment 20•11 years ago
|
||
(In reply to Arnau March from comment #19)
> I guess all this reasons could be worth a small regression :)
> So I'd like to have Patryk's feedback as well.
I might agree if this was on the order of 20ms or so. But 150ms is close to a 20% regression in launch time.
I think we'll eventually get this enabled, but we should get the fonts in place first.
Assignee | ||
Comment 21•11 years ago
|
||
I meant with the font in gecko.
If you think we need to back this out, and continue improving the workflow after the break as Kevin said, please do so.
Comment 22•11 years ago
|
||
We are at the very beginning of version 1.4, so we are not in rush to fix this performance regression. I think it is much better to keep this patch landed than back it out, because we can take advantage now of the improvements added, like the ones Arnau explained.
Comment 23•11 years ago
|
||
My apologies Marcelino, but performance regressions are *not allowed*. They must be backed out immediately.
Because we are at the beginning of 1.4 development, it gives you very much time to find the *correct* fix.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 24•11 years ago
|
||
Backed out per comment 21 and comment 23. Sorry for the hassle, but we really do need to stay on top of the regressions or it will be more painful for everyone in the end. Thanks for understanding!
Revert rev: 744f691f670dae93f160dc1199592a033b30b78a
Comment 25•11 years ago
|
||
Agreed, we shouldn't have a performance hit. Does anyone know what could be causing this regression in performance? Is it just the nature of SVG fonts? Or are the some optimizations that need to happen first?
We need to know this soon as v.1.4 has multiple new devices, and we'll need to start up scaling icons asap. I really don't want our apps to increase in byte size 6-8x by the time we hit v.1.5. Instead with this SVG font change we can reduce most of our apps by 2-3x in byte size.
Flags: needinfo?(padamczyk)
Assignee | ||
Comment 26•11 years ago
|
||
Michael, Vivien,
Could you please provide further info on how would impact in the workflow having the font in moztt repo?
This way I could continue with bug 951593.
Thanks
Comment 27•11 years ago
|
||
(In reply to Dietrich Ayala (:dietrich) from comment #23)
> My apologies Marcelino, but performance regressions are *not allowed*. They
> must be backed out immediately.
>
> Because we are at the beginning of 1.4 development, it gives you very much
> time to find the *correct* fix.
Good to hear you *always* follow the rules
Comment 28•11 years ago
|
||
(In reply to Ben Kelly [:bkelly] from comment #15)
> The problem here, though, is that we get the gonk firmware from the OEM.
> They just provided us with firmware for 1.2 in November. There is probably
> going to be a long lead time to get new firmware with the fonts on them. We
> could have months until we have OEM firmware with the new fonts rolled out.
>
> Michael, Naoki, do you know of any better way to roll out new system fonts?
>
This doesn't sound like a system font issue. (otherwise I or jfkthame would have had to review the changes)
Flags: needinfo?(mwu)
Comment 29•11 years ago
|
||
Michael, I think the issue is that we get acceptable performance if the icon fonts are included as system fonts, but we pay a heavy start up penalty if we include the fonts as part of gaia. So whats the best way to include them as system fonts?
Flags: needinfo?(mwu)
Comment 30•11 years ago
|
||
I agree with back-outing a 150ms regression, that sounds perfectly reasonable (even a 20ms sounds reasonable to me).
So yeah, let's add this font into the system image directly.
Flags: needinfo?(21)
Comment 31•11 years ago
|
||
(In reply to Ben Kelly [:bkelly] from comment #29)
> Michael, I think the issue is that we get acceptable performance if the icon
> fonts are included as system fonts, but we pay a heavy start up penalty if
> we include the fonts as part of gaia. So whats the best way to include them
> as system fonts?
Just do it. There's nothing special there - we just have to synchronize with QA and maybe add some manual font updating as part of the flash process if we need it.
Flags: needinfo?(mwu)
Assignee | ||
Comment 32•11 years ago
|
||
Vivien, I have removed the font embedded in /shared/style/icons.css and move it to the fonts repo.
Could you please review that after we land 951593?
Thanks!
Attachment #8350550 -
Flags: review?(21)
Attachment #8350550 -
Flags: review?(21) → review+
Comment 33•11 years ago
|
||
If we include this font in /system/fonts, then it'll be loaded by every process on the device (which will have a slight impact on Gecko startup time, and will slightly increase memory usage for all Gecko processes). While these effects won't be very large, as it's a single small font, they still seem undesirable to me. Also, this sets an unwelcome precedent of "polluting" the global font list with what is in fact a single-application resource. If we keep doing this as more applications decide they want to use icon fonts, there'll be an ever-increasing startup time and RAM usage burden on -every- process.
I think we should reconsider whether this is the right approach. ISTM that if a font is intended for use by one specific application (Settings, in this case), it should be loaded ONLY for that application and not for every process on the system.
Perhaps we need to look again at why including the font as part of Gaia was so expensive; maybe there's something we can do to mitigate that problem without turning the app-specific font into a system-wide one.
Comment 34•11 years ago
|
||
In https://github.com/mozilla-b2g/gaia/pull/14589, it looks like the icon font was embedded into fonts.css using a base64-encoded data: URL. Have you compared the performance of this approach with the alternative of having the font as a separate binary file? I don't know which will be faster to load, but it might be worth trying both and comparing.
I suspect the main reason the icon font within Gaia/Settings seemed so expensive is that as the CSS was written, it looks like the font load will not be initiated until we're actually trying to layout the Settings screen. At this point, we'll reflow the text with its style properties, discover that the icon font is needed, and ask the style system to load it. That takes time, and it'll block until the load completes. (Data-URL fonts load synchronously.) Loading from a separate file may be better, as it'll load async; though it'll then trigger a reflow when loading completes, so that might negate some of the gain.
The "cold load time" figures in comment 7 refer to what exactly - the time to open Settings from the Gaia homescreen on a device that has just been rebooted? Or am I misunderstanding where the regression was seen?
Does adding the font to /system/fonts have any impact on Gecko startup time? I'd expect a slight regression there, but it may be much less than was seen in the Settings launch.
Am I right in understanding that all Gaia apps run within a single Gecko process? If that's the case, one option might be to "preload" the font from CSS within Gaia, so that it doesn't have to be loaded subsequently as part of the Settings app startup. This should avoid any significant impact on the actual Settings launch.
Assignee | ||
Comment 35•11 years ago
|
||
Jonathan,
This is not an app specific patch. We have started testing icon fonts in settings app, but the idea is to have all icons in gaia in that font.
Please let me know if any of you have a better suggestion to improve loading time, or we should try landing 951593.
Thanks!
Comment 36•11 years ago
|
||
(In reply to Arnau March from comment #35)
> Jonathan,
> This is not an app specific patch. We have started testing icon fonts in
> settings app, but the idea is to have all icons in gaia in that font.
OK... but still, if it's for Gaia icons, it should ideally be loaded only by the process that's running Gaia, not by all Gecko processes on the device.
> Please let me know if any of you have a better suggestion to improve loading
> time, or we should try landing 951593.
Two suggestions:
(1) Try loading the font from a separate .woff file instead of embedding it in a data: URL. This will allow it to load asynchronously, which might reduce the impact on Settings launch.
(2) In addition to this, try causing Gaia to "pre-load" the font before the Settings app is launched, by causing it to be used to style and measure a piece of text (somewhere invisible/off-screen) as soon as Gaia starts up. Provided it's being loaded from a file (NOT a data: URL) the load should be asynchronous and not block the rest of startup.
Assignee | ||
Comment 37•11 years ago
|
||
Guys,
after following Jonathan's suggestions, I have created a couple more tests (adding median values for cold launch):
1- Master using current gif sprite (median: 1383)
2- Replacing gif with svg sprite (median: 1364)
3- Replacing gif with icon font with the font in shared/style (median: 1595)
4- Replacing gif with icon font with the font in system/fonts (median: 1010)
As you can see, if we add the font in moztt repo (4) we get the best launch time.
Is important to mention we have a better performance using svg sprites rather than icon font, the only way I could think of having common assets is having them as a Building Block, that's what I've tested in (2)
and (3), so we don't have duplicated icons in all apps, which make them difficult to maintain.
Jonathan was mentioning loading the font before Settings app is launched, but as assets should be encapsulated in each app, I don't know how to do that.
Using SVGs versus an icon font has some pros:
- As we have seen, performance.
- We could have multiple colored icons
and cons:
- We cannot change the color using CSS (AFAIK), so in case we need a black and a white version of the same icon we need to include both in the sprite (same thing we do in png/gif sprites right now).
Please, let me know what you think!
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(21)
Comment 38•11 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #34)
>
> The "cold load time" figures in comment 7 refer to what exactly - the time
> to open Settings from the Gaia homescreen on a device that has just been
> rebooted? Or am I misunderstanding where the regression was seen?
>
The cold load time is the time it takes to start one application from the click on the homescreen to the 'load' event of the app. It is opposed to the warm launch time where the application is already opened in background and we just measure how long it tooks to bring it back to the screen.
> Does adding the font to /system/fonts have any impact on Gecko startup time?
> I'd expect a slight regression there, but it may be much less than was seen
> in the Settings launch.
I'm not worried to much by that since processes that host apps are pre-loaded. Also depending on how fonts are loaded into memory I wonder if Nuwa will not less us share the font information.
>
> Am I right in understanding that all Gaia apps run within a single Gecko
> process? If that's the case, one option might be to "preload" the font from
> CSS within Gaia, so that it doesn't have to be loaded subsequently as part
> of the Settings app startup. This should avoid any significant impact on the
> actual Settings launch.
Each Gaia app lives into its own process. So we can't really preload it using a css trick in the System app (who contains all the other apps).
Flags: needinfo?(21)
Comment 39•11 years ago
|
||
(In reply to Arnau March from comment #37)
> Guys,
> after following Jonathan's suggestions, I have created a couple more tests
> (adding median values for cold launch):
>
> 1- Master using current gif sprite (median: 1383)
> 2- Replacing gif with svg sprite (median: 1364)
> 3- Replacing gif with icon font with the font in shared/style (median: 1595)
> 4- Replacing gif with icon font with the font in system/fonts (median: 1010)
>
The median value for (4) sounds awesome compared to the others. We definitively want that. I', very curious to see how it may affects other apps as well.
Assignee | ||
Comment 40•11 years ago
|
||
Cool! then we should first push bug 951593 first, could you please give it a look?
Flags: needinfo?(21)
Comment 41•11 years ago
|
||
(In reply to Arnau March from comment #40)
> Cool! then we should first push bug 951593 first, could you please give it a
> look?
What do you want me to do? To push it ?
Flags: needinfo?(21) → needinfo?(arnau)
Assignee | ||
Comment 42•11 years ago
|
||
:21 your comments are more than welcome ;)
Flags: needinfo?(arnau)
Assignee | ||
Comment 43•11 years ago
|
||
after comment https://bugzilla.mozilla.org/show_bug.cgi?id=951593#c9, should we go for option (2) and use SVGs?
Then we have two options then:
2a: having sets of icons (e.g settings, comms, ...) and have them in gaia/shared
that would be great for maintenance, but in case you only need to icons from 2 different sets could increase your app size.
2b: creating sprites or individual assets per app (just replacing want we have right now in png, but vectorial), bad for maintenance but will get rid of @x1.5, @x2...
WDYT?
Flags: needinfo?(21)
Comment 44•11 years ago
|
||
(In reply to Arnau March [:arnau] from comment #43)
> after comment https://bugzilla.mozilla.org/show_bug.cgi?id=951593#c9, should
> we go for option (2) and use SVGs?
> Then we have two options then:
> 2a: having sets of icons (e.g settings, comms, ...) and have them in
> gaia/shared
> that would be great for maintenance, but in case you only need to icons from
> 2 different sets could increase your app size.
>
> 2b: creating sprites or individual assets per app (just replacing want we
> have right now in png, but vectorial), bad for maintenance but will get rid
> of @x1.5, @x2...
>
> WDYT?
I still think we should push for the system font stuff and keep experimenting. I know this is particularly frustating but the numbers you have in https://bugzilla.mozilla.org/show_bug.cgi?id=948856#c37 are a big difference.
Flags: needinfo?(21)
Comment 45•11 years ago
|
||
my 2 cents. I did some experimentation for the calendar app (Bug 972871) and found out that SVG uses ~2x the memory of icon fonts, and is a little bit slower to render (when scrolling a view with overflow-y:scroll or just a long HTML page). I did not compare the app launch performance.
for me the biggest advantage of icon fonts over SVG is that you can control everything with CSS (no need to add <svg> elements to the DOM). Since we are moving towards "flat design" icon fonts are a very good fit (no need for multiple colors).
Ideally each icon font should be handled by the app that needs it. It will be really hard to maintain/use a single font with all the possible icons. - How would you add an extra icon to the Base-64 encoded font? How many merge conflicts? Each app might need icons designed to fit a different pixel grid (the icon is only sharp if it fits pixel grid properly, aka. hinting) Each app might need a different "alarm icon", so we will need to namespace things... The less coupling we have between the apps the better. Maybe the "shared" font should be only for the building blocks.
PS: I decided to use WOFF instead of SVG fonts since that's what the keyboard app uses. I also decided to keep the SVG font on the repository just in case we need to edit the font later (http://icomoon.io/ can read SVG fonts).
PPS: SVG painting used to be way slower than images/fonts until Firefox 26. (Bug 764299)
Updated•11 years ago
|
I think a good solution here is to create a new system font where we can stick whatever icons we want but then only make that system font available to certified apps.
This means that we're not modifying the web platform in a way that encourages 3rd party apps to do bad things, while also allowing 3rd party apps to reuse our icons by simply downloading our font and using it as a custom font.
Comment 47•11 years ago
|
||
Flagging Patryk as we need to determine an approach here, with 2.0 beginning landing on Monday. Patryk, please meet with Vivien and Jonas to see if they have a recommendation that visual design can work with, and then hopefully the outcome can be a recommendation posted to this bug, which will then need to be assigned. I would do this myself but I'm on PTO next week. Thanks!
Flags: needinfo?(padamczyk)
Assignee | ||
Comment 48•11 years ago
|
||
Attachment #8346889 -
Attachment is obsolete: true
Attachment #8350550 -
Attachment is obsolete: true
Comment 49•11 years ago
|
||
Blocked on bug 998844 because using icon fonts in Firefox OS will break the simulator, unless we find a way to load custom fonts on desktop platforms.
Comment 50•11 years ago
|
||
Re:Comment 47
Arnau is now working on the bug as there seems to be a resolution, and will post a visual update.
Flags: needinfo?(padamczyk)
Comment 51•11 years ago
|
||
What is motivating the use of icon fonts? If SVG-as-an-image is made to perform well enough, would <img src="blah.svg"> something that would be better?
Assignee | ||
Comment 52•11 years ago
|
||
Jonathan, please see comment 37:
Icon fonts are much faster than PNGs when set on gecko.
And SVGs are slower than PNGs
Comment 53•11 years ago
|
||
(In reply to Arnau March [:arnau] from comment #52)
> Jonathan, please see comment 37:
> Icon fonts are much faster than PNGs when set on gecko.
> And SVGs are slower than PNGs
So this is only about performance? I'm interested in knowing what would be required to make SVG work as a better alternative, so I'm interested in all issues.
As far as performance goes, the work in bug 999931's dependencies has improved the SVG performance and memory use a fair bit, and more improvements are in the works.
(And, by the way, comment 37 actually seems to measure GIF, not PNG, and seems to suggest for your testcase SVG was marginally faster than GIF (1364 for SVG vs 1383 for GIF).
Comment 54•11 years ago
|
||
(In reply to Arnau March [:arnau] from comment #37)
> Using SVGs versus an icon font has some pros:
> - As we have seen, performance.
> - We could have multiple colored icons
>
> and cons:
> - We cannot change the color using CSS (AFAIK), so in case we need a black
> and a white version of the same icon we need to include both in the sprite
> (same thing we do in png/gif sprites right now).
SVG 2 has introduced the concept of "context" fill, stroke, etc., where the colors used in an SVG glyph/use come from the element that is referencing the SVG. (We've already implemented support for this.) Currently context fill etc. isn't specified to work to take the context from an embedding <img> element, but we could probably get that into the spec and implement it if that was desirable.
Assignee | ||
Comment 55•10 years ago
|
||
Comment on attachment 8419751 [details]
patch in github
Etienne, could you please review this patch?
I've created an icon font in /shared, which is a duplicated of a hidden font in /system/fonts.
If both fonts matches, we get a better performance as the font in system will be preloaded.
Please read https://bugzilla.mozilla.org/show_bug.cgi?id=951593#c77
Although the font contains most icons in gaia (see shared/icons/index.html for the full list), this patch for settings is only replacing the root icons.
We will need a follow up to replace the rest of the icons if performance test are good enough :)
Attachment #8419751 -
Flags: review?(etienne)
Assignee | ||
Comment 56•10 years ago
|
||
Comment on attachment 8419751 [details]
patch in github
Sorry Etienne, confused settings with system :)
Arthur, could you please review that one? (please read my previous comment)
Attachment #8419751 -
Flags: review?(etienne) → review?(arthur.chen)
Comment 57•10 years ago
|
||
Comment on attachment 8419751 [details]
patch in github
Arnau, thank you for the effort working on this!
IMHO it seems better to separate the bug to two. One for introducing the icon fonts and one for the settings app updates (maybe another for the calendar app).
Although I've read the thread on bug 951593 but I'm afraid that I am not able to review the shared part of the patch. I would prefer to have Vivien and Jonathan take a look on the patch.
Attachment #8419751 -
Flags: review?(arthur.chen)
Assignee | ||
Comment 58•10 years ago
|
||
Arthur, make sense.
I'll submit a new patch (bug 1035670) for the shared part a will update that one to remove the unneeded files.
Thanks!
Assignee | ||
Comment 59•10 years ago
|
||
Comment on attachment 8419751 [details]
patch in github
Arthur, could you please review it again?
I'm only including the settings part, now.
Attachment #8419751 -
Flags: review?(arthur.chen)
Comment 60•10 years ago
|
||
Comment on attachment 8419751 [details]
patch in github
Looks good to me, thank you!
Attachment #8419751 -
Flags: review?(arthur.chen) → review+
Assignee | ||
Comment 61•10 years ago
|
||
merged: 5db347ac8bf38ea5399d4f99e83b8bce1227d24f
Status: REOPENED → RESOLVED
Closed: 11 years ago → 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
status-b2g-v2.1:
--- → fixed
Comment 62•10 years ago
|
||
Excuse me, do you mean the Settings icon should display as a gear? Could you provide the detailed steps and correct screenshot for me to verify this bug. Thanks!
Flags: needinfo?(epang)
Reporter | ||
Comment 63•10 years ago
|
||
Hi Sue, this bug replaces all the icons on the left margin of the settings app to use SVGs instead of a sprite. I'll attached a screenshot of the icons I'm referring too shortly :)
Flags: needinfo?(epang)
Reporter | ||
Comment 64•10 years ago
|
||
an outdated image, but these are the icons I'm referring to.
Comment 65•10 years ago
|
||
Hi,
I have uploaded the video of all Settings icons, could you help with it? Thanks.
See attachment: verify_video.MP4
Flags: needinfo?(epang)
Reporter | ||
Comment 66•10 years ago
|
||
(In reply to Sue from comment #65)
> Created attachment 8531400 [details]
> verify_video.MP4
>
> Hi,
> I have uploaded the video of all Settings icons, could you help with it?
> Thanks.
> See attachment: verify_video.MP4
Hi Sue,
Looks like the video is corrupt. We should be able to as Helen (who owns settings) to verify if the current settings app uses svgs instead of a sprite. Helen can you help verify? Thanks!
Flags: needinfo?(epang) → needinfo?(hhuang)
Comment 67•10 years ago
|
||
Hi,
Sorry I'm not quite sure what you want me to check in the video.
I've confirmed with Arthur, who is developer owns Settings, the current icons on root panel are already using icon font. Hope that helps your question.
Flags: needinfo?(hhuang)
You need to log in
before you can comment on or make changes to this bug.
Description
•