Like bug 1150627 for TB is this bug for Lightning to use SVG graphics instead PNG graphics for Linux, OS X and Windows Vista+. The plus side is a smaller XPI as the different png graphics aren't packe d and only one file with the SVG graphics is needed. Also are the icons inverted on Linux and OS X when using dark LW-themes or dark system themes. The minus side could be that Linux is now using the same monochrome icons and no more colored ones. But Firefox has done the same and I heard no howl in the Linux community about this.
Created attachment 8591334 [details] [diff] [review] SVGforLightning.patch This is a full patch which changes the calendar- and task toolbars, the task header toolbar and also the imip buttons and the event dialog toolbar. Additionally I began to remove the need of the special files for XP and Win Vista+ (win-classic and win-aero) through differing them with media queries in the normal files. I'll completely remove them after this bug in a follow-up bug.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
I'm waiting for review until I have a feedback from TB bug. But feedback to this bug is also welcome.
Created attachment 8591341 [details] comparison-svg-png-winaero.png I've given this a test on Win7 based try build from . Compared to the current Aero icons, most of the icons look less well defined and a little bit noisy - see the attached screenshots. Referencing Richard's try builds for 4.2:  https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a113818e2f82 (Win + Linux)  https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d378ce077386 (Mac)
Created attachment 8591362 [details] SVG-icons.png Screenshot with almost every icon. (In reply to MakeMyDay from comment #3) > Created attachment 8591341 [details] > comparison-svg-png-winaero.png > > I've given this a test on Win7 based try build from . Compared to the > current Aero icons, most of the icons look less well defined and a little > bit noisy - see the attached screenshots. This is because you haven't used the TB patch. It uses 18px icons. With 16px icons they are scaled and blurry.
Sorry, I didn't realize that you have to apply this combined. On your screenshot, it looks good in general. I noticed some other icons have slightly changed: delete (strike through is in different direction), add attendee (the head seems to be a little bigger compared to the shoulders), new event/task (the + is much bolder now). The last one is looks odd, but this is just my personal preference. I assume you will consider the icons for the imip bar as well?
Yes, they are a bit different. First it's now a mixture from OS X and Windows. And for the different dimension of details, I had to go to even dimensions to let it draw not blurry. On PNG we had the possibility to make some part sharp manually, but with SVG the image is calculated from the description and this needs to be observed at the beginning. But for example the bigger + makes it also better remarkable.
Do you really have to use the <use> tag? Wouldn't just giving the <g> tags some class names do just as well. There's a lot of overhead in <use> tags tracking changes.
Do class names work with anchor tags? I thought the <use> tags was needed so you can do chrome://calendar/skin/filename.svg#print-icon and have it show the print icon.
If you do it that way, yes. But you might be able to position things in different places and use an svg view fragment with a viewBox parameter to select each icon (http://www.w3.org/TR/SVG/linking.html#SVGFragmentIdentifiers) At the moment use clones each item so your memory cost is going to be high, that's why you want to avoid it.
Interesting. I've been using <use> because I saw it this way in Firefox, e.g. http://mxr.mozilla.org/comm-central/source/mozilla/browser/components/loop/content/shared/img/icons-14x14.svg?raw=1 I kind of liked not having to specify dimensions like we did before with -moz-image-region. Is there a Core bug about the <use> memory usage and/or a Firefox bug to replace use of <use> ?
And is somewhere a example svg to look how it is done? I failed with my experiments to display the desired image.
(In reply to Philipp Kewisch [:Fallen] from comment #10) > Interesting. I've been using <use> because I saw it this way in Firefox, > e.g. > http://mxr.mozilla.org/comm-central/source/mozilla/browser/components/loop/ > content/shared/img/icons-14x14.svg?raw=1 oh dear. > > I kind of liked not having to specify dimensions like we did before with > -moz-image-region. Is there a Core bug about the <use> memory usage and/or a > Firefox bug to replace use of <use> ? Nothing ideal really. There's bug 1103397 and bug 964815 which are perf bugs where the content is mainly use elements. Ideally we'd fix <use> not to clone.
image.svg#svgView(viewBox(0,0,100,100)) image.svg#svgView(viewBox(0,100,100,100)) etc. draw the separate icons within each viewBox. You should use an <svg> element rather than a <g> element for each icon so you get overflow:hidden and each view doesn't spill over into the next.
Created attachment 8591644 [details] test.svg I'm sorry, but I don't get it working. Please could you add the needed elements in this test file? The viewbox should be 18x18.
Attachment #8591690 - Attachment mime type: text/plain → image/svg+xml
(In reply to Richard Marti (:Paenglab) from comment #14) > Created attachment 8591644 [details] > test.svg > > I'm sorry, but I don't get it working. Please could you add the needed > elements in this test file? The viewbox should be 18x18. I couldn't get viewBox working either. I think that's a dead end, sorry. This may be what you need though, one file, two different displays.
Thanks a lot Robert! Now I need to look how I can apply the different colors for the platforms. Probably with classes.
Created attachment 8594389 [details] [diff] [review] SVGforLightning.patch v2 Now using the proposal from Robert for the SVG. Needs the patch from bug 1150627 to apply and work cleanly. Robert, is this now okay with the SVG file? Or do you know what could be improved?
Comment on attachment 8594389 [details] [diff] [review] SVGforLightning.patch v2 Review of attachment 8594389 [details] [diff] [review]: ----------------------------------------------------------------- Seems fine to me now.
Attachment #8594389 - Flags: feedback?(longsonr) → feedback+
Created attachment 8595525 [details] [diff] [review] SVGforLightning.patch v3 Patch using smaller icons.
Attachment #8594389 - Attachment is obsolete: true
Attachment #8591334 - Attachment is obsolete: true
Created attachment 8616773 [details] [diff] [review] SVGforLightning.patch v4 Bug 1150627 has now c-i, and this patch is ready for reviewing. Made the images 16px (in a 18px area) and added Win10 support.
Created attachment 8627010 [details] [diff] [review] SVGforLightning.patch v5 Updated the toolbar icon color for Win10 to the color of bug 1166930.
Comment on attachment 8627010 [details] [diff] [review] SVGforLightning.patch v5 Review of attachment 8627010 [details] [diff] [review]: ----------------------------------------------------------------- I like it, r=philipp. I haven't checked all the icons, but I assume they will look nice :-)
Attachment #8627010 - Flags: review?(philipp) → review+
Pushed to comm-central changeset 89e2a4ab8cd9
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.4
You need to log in before you can comment on or make changes to this bug.