Closed
Bug 972871
Opened 11 years ago
Closed 11 years ago
[Calendar] custom font with vector icons used by visual refresh
Categories
(Firefox OS Graveyard :: Gaia::Calendar, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.4 S6 (25apr)
People
(Reporter: mmedeiros, Assigned: mmedeiros)
References
Details
(Whiteboard: [p=2])
Attachments
(1 file, 2 obsolete files)
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.
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Updated•11 years ago
|
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
Comment 1•11 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.
Assignee | ||
Comment 2•11 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).
Attachment #8376364 -
Flags: review?(gaye)
Assignee | ||
Comment 3•11 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.
Comment 4•11 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.
Assignee | ||
Updated•11 years ago
|
Target Milestone: 1.4 S1 (14feb) → 1.4 S2 (28feb)
Assignee | ||
Comment 5•11 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.
Attachment #8376364 -
Attachment is obsolete: true
Attachment #8376364 -
Flags: review?(gaye)
Attachment #8387878 -
Flags: review?(gaye)
Comment 6•11 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.
Flags: needinfo?(mmedeiros)
Assignee | ||
Comment 7•11 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.
Flags: needinfo?(mmedeiros)
Assignee | ||
Updated•11 years ago
|
Target Milestone: 1.4 S2 (28feb) → ---
Updated•11 years ago
|
Attachment #8387878 -
Flags: review?(gaye) → review+
Assignee | ||
Comment 8•11 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.
Assignee | ||
Comment 9•11 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.
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)
Assignee | ||
Comment 10•11 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.
Attachment #8387878 -
Attachment is obsolete: true
Attachment #8411410 -
Flags: review?(gaye)
Comment 12•11 years ago
|
||
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+
Assignee | ||
Comment 13•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•