Problem/Motivation

Adding an icon sprite for UAs that don't support data URIs as arguments to the CSS url() function. This is part of toolbar followup further to code freeze.
Meta issue:#1846970: [meta] Responsive Toolbar follow-ups

Proposed resolution

To be determined.

Remaining tasks

To be determined.

User interface changes

None.

API changes

None.

Dependencies

#1849082: Implement the data URI converter for CSS images

Files: 
CommentFileSizeAuthor
#18 horiztool.jpg36.68 KBdcrocks
#18 verttool.jpg33.59 KBdcrocks
#15 toolbar-sprite-1849078-15.patch26.39 KBoresh
PASSED: [[SimpleTest]]: [MySQL] 54,039 pass(es).
[ View ]
#14 3-4-2013 9-21-52.png8.26 KBdroplet
#12 toolbarD8-Chrome.png68.75 KBahimsauzi
#12 toolbarD8-Firefox.png37.93 KBahimsauzi
#10 toolbar-sprite-1849078-10.patch24.08 KBoresh
PASSED: [[SimpleTest]]: [MySQL] 53,096 pass(es).
[ View ]
#9 after patch.png191.07 KBMandakini_Kumari
#5 toolbar-sprite-1849078-5.patch13.16 KBbryanbraun
PASSED: [[SimpleTest]]: [MySQL] 52,195 pass(es).
[ View ]

Comments

Status:Needs work» Active

change status to active

Title:Adding an icon sprite for Toolbar UAsReplace many toolbar icon files with a single CSS sprite image

I don't know what UAs are, (User Agents?) but I'm guessing the point of this issue is to replace all the icon files in the toolbar module with a single sprite image? I think the active state versions could probably also be done with opacity rather than using a whole other icon.

Updating title. (If this is not what this issue is about, please comment so I can create a new issue) :)

Yes you are right 'UA' is User Agent, got copied from the description. Thanks for the title edit!

Please go ahead and create another issue for

I think the active state versions could probably also be done with opacity rather than using a whole other icon.

It might be worth starting with a simpler approach, but Assetic should be able to handle sprite generation/formatting, once that is in.

Status:Active» Needs review
StatusFileSize
new13.16 KB
PASSED: [[SimpleTest]]: [MySQL] 52,195 pass(es).
[ View ]

Here is a patch to get the ball rolling. This combines all the Toolbar icons into one sprite and changes the CSS to reference the sprite. I'm unsure about whether to include other icons in the sprite, like those provided by the Edit or Shortcut modules. Those icons were impacted by the change and haven't been repositioned yet.

Issue tags:+Novice

Adding tag novice.

This issue needs review.

Status:Needs review» Needs work

- Menu -> Help icon, some pixels cuts off
- the icons on sprite image placed too close. It should add more spacing between each icons. eg. when you increase the font size to 20px or add more paddings. the top & bottom icons will show on the menus.
- why all icons on the stylesheel missing ".toolbar" qualifier
- position of shortcuts & admins icons doesn't looks right.

Issue tags:+Spark

Tagging so we can keep track of this. Thanks for the patch, bryan!

StatusFileSize
new191.07 KB

Tested patch on firefox 19.x and found below issues:

  1. Shortcuts and Admin images are not align with text.
  2. shortcut.png and icon-user.png need to be part of toolbar-sprite.png

Status:Needs work» Needs review
StatusFileSize
new24.08 KB
PASSED: [[SimpleTest]]: [MySQL] 53,096 pass(es).
[ View ]

Fixes for #5 patch:
- Background image repeat in css removed,
- Icon being cut for help and admin links changed
- Upper toolbar icons alignment fixed
- Some other icons position breaking fixed - orientation and dropdown icons
- Deleted unused icons
- Image sprite compressed with http://tinypng.org/

Please review. Tested on Chrome v24 on Ubuntu.

Issue tags:+Needs manual testing

+adding tags for testing

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new37.93 KB
new68.75 KB

Patch applies cleanly and tested with Firefox, chrome and Safari.

Repo of applied patch: https://github.com/ahimsauzi/Drupal-Ladder

Assigned:Unassigned» jessebeach

Awesome, thanks! Performance++

Assigning to Jesse to just give a once-over.

StatusFileSize
new8.26 KB

Don't mean to block it move forward.

+++ b/core/modules/toolbar/css/toolbar.icons.cssundefined
@@ -39,79 +42,89 @@
+  background-position: -1px -379px;
...
+  background-position: -1px -347px;
+++ b/core/modules/toolbar/css/toolbar.menu.cssundefined
@@ -87,15 +87,15 @@
+  background: url("../images/toolbar-sprite.png") 1px -503px;
...
+  background: url("../images/toolbar-sprite.png") 1px -474px;
+++ b/core/modules/toolbar/css/toolbar.theme.cssundefined
@@ -143,8 +143,8 @@
+  background: url('../images/toolbar-sprite.png') 1px -721px;

Just out of curious why these icons added "-1px". I'm only check the icon-home button. It's 21px but width of icon DIV is 20px.

Also about #7:

- the icons on sprite image placed too close. It should add more spacing between each icons. eg. when you increase the font size to 20px or add more paddings. the top & bottom icons will show on the menus.

3-4-2013 9-21-52.png

Also,
If icons meant to be used on everywhere, therefore this is remove ".toolbar" qualifier. Why not move it to CORE stylesheet ?

StatusFileSize
new26.39 KB
PASSED: [[SimpleTest]]: [MySQL] 54,039 pass(es).
[ View ]

@droplet,
I'm not sure about moving the icons to core stylesheet. And where exactly do you propose to move them?

Anyway here's a patch covering #14 issues.
I'd like it it be reviews asap, i'm working (actually done) on refactoring the css and js for toolbar module for the smacss principle and reducing the selector weight.

For example:
.toolbar .vertical to .toolbar-vertical
Hope this makes sens.

Status:Reviewed & tested by the community» Needs review

Can we implement the retina-sized version (200%) of the icons? I could make these if I can find the original full-size icons.

StatusFileSize
new33.59 KB
new36.68 KB

Patched 8.x-dev. Wanted to see if it is faster. Still can be slow, but at least images aren't being painted 1 by 1.

Created #1963886: Use HiDPI icons in the toolbar for my comment in #17.

@oresh,

Thanks, it looks better now.

@droplet,
I'm not sure about moving the icons to core stylesheet. And where exactly do you propose to move them?

Leave it to maintainers. :)

Not pertinent but relevant. What is the source of the icons? Is there any notion of a 'drupal standard icon' collection? As more iconic menus show up in drupal core I would think there is an interest in a consistent ui for drupal core.

What about creating an icon font?

We have played with the idea of creating an icon font for this sort of thing for Drupal and I am more than happy allocating time for it.

Assigned:jessebeach» Unassigned

@robin.prieschl, So, we talked about icon fonts.
Guys are trying to write an understandable document on using icon font. The thing is - everybody knows how to use sprites, but not a lot of people know how to use icon fonts. I've never done that before for example.
Also this issues is about sprites. Font icon will weight more then this sprite ( i guess ) and it needs specific documentation, when sprite do not. So if you want font icons search for this issue, if there's nothing about that, create a follow up :)

@robin.prieschl: Yes, please open a new issue for icon fonts.

I tried re-doing this with icon fonts instead of images or a sprite file. It is pretty straight forward but before I open another issue with a patch there are some things to think about. I don't see any performance or page size issues but using a font or a sprite file does mitigate the 'late' drawing of the images when the page is repainted that you see with individual images. The icon font solution does lessen the css required. The problems I wasn't able to or couldn't figure out how to address are:

1) The toolbar is actually built from 4 different menus from 4 different modules; toolbar, user, shortcut, and contextual. With the possibility of other modules, including contrib, chiming in. Currently each module does its own styling of added icons. So, any changes to toolbar icons have to be aware that other modules have to be considered, and any other module adding to the toolbar has to be aware it needs to do its own styling for icons.

2) There are accessibility issues that arise from using icons with links and they all apply to the current toolbar implementation. I don't see a straightforward way to address these in the current implementation. Are there any future plans for enhancing the toolbar module?

I think we need to create an issue to address the icon link pattern.

Naming all icons in CORE and make the icons button/links html structure more consistently.

eg.

<i class="icon-flag"></i><a href="#">Link</a>
<i class="icon-menu"></i><button>Button</button>

It will be more easy to reuse/replace SAME icon everywhere.

Status:Needs review» Reviewed & tested by the community

Review again, it's good enough now, other issue could work in a new issue thread. RTBC

This does not convert the code in modules user, shortcut, and contextual to using the sprite file. You still get the late painting affect. Is it really better for those to be separate issues? You will still see the late painting affect.

@dcrocks,

What is "late painting affect" ?

In the current D8, there are times when the page you are on is completely redrawn on the screen. For example if you were in the admin overlay and made some change, saved, and after the overlay returned you click on the 'home' link. When the page is redrawn you 1st see the home page itself and the toolbar, but it only has the text. Then the icon images are painted on the toolbar overlay one by one with a perceptible pause between each. With the sprite patch you only see this on user and shortcut link icons because they are still using background images.
This is very evident on firefox(20.0), much less so on chrome(26..) and less on safari(5.1.9). I'm sure it is also a factor of my machine's age, old but not that old. I see it even when I use a fast server on a fast connection that is idle otherwise. Each image can mean a trip to the server, that's why sprites are better. I am beginning to think that it is firefox specific, but a lot of people use firefox.

Status:Reviewed & tested by the community» Needs work

Ahh. missing 2 icons in sprite image.

Not all elements that appear in the toolbar are provided by toolbar.module, as described in #25. I don't think it's reasonable to grab images from edit and shortcut module into a sprite of toolbar.module's images right now, as 1. there is no consistent battleplan yet for how to handle icons in the menu additional to those provided by core, 2. we can't just shove every icon image that may ever appear into a sprite that lives in toolbar. (This is probably an argument for using a symbol font, which is probably the long-term play here).

All this to say, if we are to create a sprite, we should for the scope of this ticket limit it to the images currently provided by toolbar module alone, and worry about how to more efficiently package edit and shortcut and whatever other modules's admin icons in a separate ticket. It's not a full solution, but it would provide savings in terms of number of requests anyway.

Patch from #15 does not apply cleanly to 8.x HEAD as of May 24.

Gonna attempt a re-roll.

Status:Needs work» Postponed

Let's get the monster patch #1938044: Prefix all toolbar classes to prevent theme clashes in first and then re-role here.

Issue tags:+sprite

tagging

Status:Postponed» Needs work

Back to CNW.

So, are we still going to create a sprite, or a icon font or something?

Status:Needs work» Closed (won't fix)

We're addressing toolbar icons in #1963886: Use HiDPI icons in the toolbar. That issue supercedes this issue.

Status:Closed (won't fix)» Needs work

It looks like these issue does not implement a sprite to reduce http requests to an absolute minimum. The other case still requests a lot if single files.

That's true. We're using SVGs as the primary icon source, which means we can't spritesheet them. Only older browsers that don't support SVGs will get the PNG fallbacks. If we spritesheet the PNGs, it will be to benefit these older browsers.

Issue summary:View changes

added details

Issue summary:View changes

"uris" is incomprehensible.

Given that IE8 and lower is no longer supported in core, do we need to worry about generating the spritesheet for the fallback icons?

The toolbar (and anything that needs jQuery for that matter) doesn't even remotely work in IE8, so getting the fallback icons (which don't even match the SVG files) into a spritesheet seems superfluous.

Can we close this issue jessebeach?