Closed Bug 972871 Opened 7 years ago Closed 7 years ago
[Calendar] custom font with vector icons used by visual refresh
46 bytes, text/x-github-pull-request
|Details | Review|
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.
Summary: [Calendar] Create custom font with all vector icons used by 1.4 visual refresh → [Calendar] Create custom font with vector icons used by 1.4 visual refresh
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.
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).
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.
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.
Target Milestone: 1.4 S1 (14feb) → 1.4 S2 (28feb)
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.
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.
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.
Target Milestone: 1.4 S2 (28feb) → ---
Attachment #8387878 - Flags: review?(gaye) → review+
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.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
See Also: → 951593
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: [Calendar] Create custom font with vector icons used by 1.4 visual refresh → [Calendar] custom font with vector icons used by visual refresh
Target Milestone: --- → 1.4 S6 (25apr)
targeting master branch; contains more icons than previous patch; includes icomoon JSON "project" to make it easier to edit font in the future.
Comment on attachment 8411410 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18611 Nice nice :)
Attachment #8411410 - Flags: review?(gaye) → review+
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.