Miller Medeiros [:millermedeiros] Assignee
	Description • 10 years ago

IcoMoon is a good option: http://icomoon.io/app/

Did a performance test and confirmed that custom fonts renders faster on Hamachi than SVG (and uses ~50% of the memory), and it will also be easier to tweak it with CSS.

I already started doing it, creating this bug mostly to track progress and since it blocks the v1.4 Visual Refresh.

	Andrew Sutherland [:asuth] (he/him)
	Comment 1 • 10 years ago

Did you get a chance to see what the memory differential was?  I'm expecting the memory to have been much better on the font too, but that would be a useful data-point for further evangelism of this technique in gaia.

	Miller Medeiros [:millermedeiros] Assignee
	Comment 2 • 10 years ago

To use the alarm icon you just need to create an element with class `.icon-alarm` and you should be good to go. You can set the icon color by simply changing the element text color.

The SVG font is kept in the repository just in case we need to recreate the font using IcoMoon or any other software. - we only use the WOFF file.

With those icons we can go ahead and implement day, week and month views. Later we can create a new issue to track the other icons.

We should merge this PR before the other visual refresh changes (since they all depend on this).

	Miller Medeiros [:millermedeiros] Assignee
	Comment 3 • 10 years ago

Hi Andrew,

SVG performance improved a lot after Bug 764299, but icon fonts still uses less memory. - at least on my extreme test cases.

I did a few tests and confirmed that woff font used 50% of the memory of SVG elements. I did the test with 400 icons in the same page (with an <hr> in between each group of 10 icons), and tried to scroll the page to measure FPS and memory usage. - I did that with many different ways to embed the SVG (<img>, background image, inline SVG, <object>, SVG font, <use>), each technique have its pros and cons.

	Anthony Ricaud (:rik)
	Comment 4 • 10 years ago

There is some similar work being done for the Settings app. See bug 951593 and bug 948856. I suggest getting in touch with people working on those to have a best practice that we can share to the whole Gaia group.

	Miller Medeiros [:millermedeiros] Assignee
	Comment 5 • 10 years ago

I rebased the changes and renamed old target branch (from "calendar-1.4-visual-refresh" to "calendar-visual-refresh"), so had to do a new pull request.

I think we should merge this into the "calendar-visual-refresh" branch as soon as possible to avoid multiple long lived branches - rebasing multiple branches takes time and might introduce errors (specially when branches depends on each other).

I think we should NOT care about the performance until it actually becomes an issue - let's finish the visual refresh before doing any performance analysis/optimization, we can always improve it later.

	Gareth Aye [:gaye] (back from PTO)
	Comment 6 • 10 years ago

Hey Miller! This looks great. I would like for you to add some documentation to the calendar README though for the future when/if someone else wants to modify/understand/extend these icons. Even a small explanation of what icomoon is, what we used it for, and some guidance for noobs like me would be awesome.

	Miller Medeiros [:millermedeiros] Assignee
	Comment 7 • 10 years ago

Hi Gareth, I just added some comments to the "icons.css" file itself since I think it makes more sense tahn keeping it on the README: https://github.com/millermedeiros/gaia/commit/e9b3cf59fa9a2f7498f608c04b5daf5bb70645ac - added links to 2 blog posts that describes it in more detail.

	Miller Medeiros [:millermedeiros] Assignee
	Comment 8 • 10 years ago

There are some performance implication related to @font-face and app start-up but we decided to move forward with this patch during the visual refresh updates and we will profile the whole app after the visual refresh is completed (compare with v1.4). If it really degrades performance and we can't find a solution for it, it will be very easy to replace all the icons with images since they will all be grouped into the same CSS file and we won't need to change the markup.

This will allow us to move faster during the visual refresh update and it's also easier to convert the vectors into images than to do the opposite later.

	Miller Medeiros [:millermedeiros] Assignee
	Comment 9 • 10 years ago

going to open a PR targeting master since this will be used by all the visual refresh patches and we will start landing visual refresh patches next week. The feature branch approach is not working really well since we can't rebase the changes. I'll also add the icons introduced on Bug 972876 in the same commit.

	Miller Medeiros [:millermedeiros] Assignee
	Comment 10 • 10 years ago

targeting master branch; contains more icons than previous patch; includes icomoon JSON "project" to make it easier to edit font in the future.

	Gareth Aye [:gaye] (back from PTO)
	Comment 12 • 10 years ago

Comment on attachment 8411410 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18611

Nice nice :)

	Miller Medeiros [:millermedeiros] Assignee
	Comment 13 • 10 years ago

landed https://github.com/mozilla-b2g/gaia/commit/2614fa7e0b339bbe15c8eb95f7a8d417b7c55e4b