Bug 7500 - Use CSS Sprites in the OPAC for faster page loading
Summary: Use CSS Sprites in the OPAC for faster page loading
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low minor (vote)
Assignee: Owen Leonard
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 10125
  Show dependency treegraph
 
Reported: 2012-02-03 20:11 UTC by Ian Walls
Modified: 2013-12-05 20:05 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7500 - Use CSS Sprites for faster page loading (29.42 KB, patch)
2012-04-17 13:33 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP] Use CSS Sprites for faster page loading (25.40 KB, patch)
2012-04-18 13:30 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7500 - Use CSS Sprites for faster page loading (28.12 KB, patch)
2012-04-18 14:23 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP][Social networks] Use CSS Sprites for faster page loading (7.49 KB, patch)
2012-05-15 18:38 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP][Star ratings] Use CSS Sprites for faster page loading (7.44 KB, patch)
2012-05-15 19:41 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP][Missing images] Use CSS Sprites for faster page loading (23.89 KB, patch)
2012-05-16 17:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP][Social networks] Use CSS Sprites for faster page loading (4.32 KB, patch)
2012-05-21 13:40 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP][Social networks] Use CSS Sprites for faster page loading (7.51 KB, patch)
2012-05-21 14:20 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP][Social networks] Use CSS Sprites for faster page loading (7.51 KB, patch)
2012-05-21 14:26 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7500 [SIGNED-OFF][FOLLOW-UP][Social networks] Use CSS Sprites for faster page loading (7.53 KB, patch)
2012-05-21 14:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP][Star ratings] Use CSS Sprites for faster page loading (7.45 KB, patch)
2012-08-07 04:11 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP][Missing images] Use CSS Sprites for faster page loading (23.90 KB, patch)
2012-08-07 04:14 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 7500 [FOLLOW-UP, revised][Social networks] Use CSS Sprites for faster page loading (13.82 KB, patch)
2012-09-12 12:45 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7500 [Lists follow-up] Use CSS Sprites for faster page loading (1.77 KB, patch)
2012-11-08 17:27 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed patch (lists follow-up) (8.83 KB, patch)
2012-11-12 08:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Signed patch (lists follow-up) (1.83 KB, patch)
2012-11-13 12:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Walls 2012-02-03 20:11:45 UTC
In order to reduce the number of image requests on the OPAC, perhaps it would be advantageous to create a single PNG containing all the images tiled together, then use CSS values to 'mask' the unnecessary parts.  This could reduce the overall page load time by several hundred milliseconds.  Not a significant percentage in the grand scheme of things, but every little bit counts.

Thoughts?
Comment 1 Owen Leonard 2012-02-03 20:22:18 UTC
I agree, a worthy goal.
Comment 2 Paul Poulain 2012-02-10 13:57:00 UTC
I don't see which discussion to start here. it's the kind of question we have on a lot of other bugs: it's a cool idea, now who do it ?
removing the "in discussion" status. If one of you think it's usefull, switch back, and start the discussion on the wiki & mailing list
Comment 3 Owen Leonard 2012-04-17 13:33:16 UTC Comment hidden (obsolete)
Comment 4 Dobrica Pavlinusic 2012-04-17 22:53:34 UTC
Owen, I love this patch, it goes a long way towards reducing round trip times to server :-)

However, I don't think we should include koha-logo.gif in sprites, since it's element which most library customize, and applying your patch on top of customized installation shows correct logo but shifted because of css modifications to h1#libraryname

Dropping koha-logo.gif from sprites.png would also make sprites smaller, since all other icons are smaller than Koha logo.
Comment 5 Mason James 2012-04-18 06:51:24 UTC
(In reply to comment #3)
> Created attachment 9226 [details] [review]
> Bug 7500 - Use CSS Sprites for faster page loading
> 
> This patch moves *most* small interface icons and
> the Koha log into a single 10K sprite image. CSS
> is used to position the sprite correctly for each
> icon instance.

woo - nice idea!

Owen, whats the method to create the 10k sprites file?, just curious...
Comment 6 Dobrica Pavlinusic 2012-04-18 12:39:51 UTC
A long time ago, I wrote automatic CSS sprite generator:

http://svn.rot13.org/index.cgi/Frey/view/trunk/lib/Frey/Web/CombineImages.pm

Would it make sense to turn it into Template Toolkit plugin, possibly as part of Bug 1918 so we can generate sprites automatically?
Comment 7 Owen Leonard 2012-04-18 12:54:24 UTC
(In reply to comment #5)
> Owen, whats the method to create the 10k sprites file?, just curious...

The images are just grouped together in one tall narrow file. I did it in Photoshop, but it could have been done just as easily in another image editor.

(In reply to comment #6)
> Would it make sense to turn it into Template Toolkit plugin, possibly as
> part of Bug 1918 so we can generate sprites automatically?

That seems like more work for the server than is necessary considering how seldom we add new image files for the interface.
Comment 8 Owen Leonard 2012-04-18 13:30:23 UTC Comment hidden (obsolete)
Comment 9 Dobrica Pavlinusic 2012-04-18 14:23:59 UTC Comment hidden (obsolete)
Comment 10 Paul Poulain 2012-04-20 14:07:29 UTC
QA comment: nothing specific to say, passed QA

Just some number YSLOW gives for a random opac-detail page (with "socialnetwork syspref activated, that add some image not concerned by this patch) :
* BEFORE the patch = 50 components, 16 css images, images+css image = 30.5k
* AFTER the patch = 43 components, 7 css images, images+css image = 36.3k (that's a larger size, probably because we load 1 large image, not X tiny ones. But they'll be cached, so it's OK)

passed QA
Comment 11 Paul Poulain 2012-04-20 14:11:02 UTC
I'd really like to push this patch before the release, but its an ENH that will wait for after 3.8
If there's something wrong hidden, I don't want to release without a good testing time.

A question for Owen:
* you say "most". Does it mean you'll provide a follow-up for other images, or images that aren't in won't be added -whatever the reason-
* could you provide also the follow-up that removes now useless images ?
Comment 12 Owen Leonard 2012-04-20 14:57:44 UTC
I missing from this first patch is:

- Social network images
- Star ratings images
- XSLT icons

Each of these is a feature which can be turned on or off. Options:

- Make a single sprite for each
- Add them all to the main sprite

Opinions?
Comment 13 Paul Poulain 2012-04-24 03:38:03 UTC
(In reply to comment #12)
> I missing from this first patch is:
> 
> - Social network images
> - Star ratings images
> - XSLT icons
> 
> Each of these is a feature which can be turned on or off. Options:
> 
> - Make a single sprite for each
> - Add them all to the main sprite
I would say a sprite for each. About XSLT icons, are you talking of itemtype images ? if you're I'm not sure we should "sprite" them: most libraries use a few of them.
Comment 14 Paul Poulain 2012-05-10 16:35:17 UTC
(In reply to comment #12)
> I missing from this first patch is:
> 
> - Social network images
> - Star ratings images
> - XSLT icons
> 
> Each of these is a feature which can be turned on or off. Options:
> 
> - Make a single sprite for each
> - Add them all to the main sprite
I think social network images is worth being put in a separate sprite and star rating in another one, and XSLT icons are probably not a good idea because only one is used on a given record and we have a lot of icons, so we would upload an image much larger than needed

PS: for social network & star, the sprite being loaded only if those features are ON, of course
Comment 15 Owen Leonard 2012-05-15 18:38:06 UTC Comment hidden (obsolete)
Comment 16 Owen Leonard 2012-05-15 19:41:26 UTC Comment hidden (obsolete)
Comment 17 Owen Leonard 2012-05-16 17:08:23 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2012-05-21 07:28:55 UTC
(In reply to comment #15)
> Created attachment 9594 [details] [review]
> Bug 7500 [FOLLOW-UP][Social networks] Use CSS Sprites for faster page loading

This patch does not apply (I just test this one), can you rebase it please ?
Comment 19 Owen Leonard 2012-05-21 13:40:18 UTC Comment hidden (obsolete)
Comment 20 Owen Leonard 2012-05-21 14:20:47 UTC Comment hidden (obsolete)
Comment 21 Owen Leonard 2012-05-21 14:26:00 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2012-05-21 14:35:57 UTC Comment hidden (obsolete)
Comment 23 Jared Camins-Esakov 2012-07-07 14:41:42 UTC
I am a little confused by the status of this bug. Does anything here still need sign off?
Comment 24 Owen Leonard 2012-07-10 12:33:59 UTC
Yes, two patches still need to be signed off: Attachment 9599 [details] (Star ratings) and Attachment 9623 [details] (Missing images).
Comment 25 Chris Cormack 2012-08-07 04:11:47 UTC Comment hidden (obsolete)
Comment 26 Chris Cormack 2012-08-07 04:14:06 UTC Comment hidden (obsolete)
Comment 27 Marcel de Rooy 2012-08-30 10:01:11 UTC
QA Comment:
Looks good to me.
Passed QA
Comment 28 Paul Poulain 2012-09-04 16:38:47 UTC
QA comment: if i'm not wrong owen, you replaced /opac-tmpl/prog/images/socnet/facebook16.png by a social sprite, but facebook16.png is not removed.

Shouldn't it be removed ? (same thing for other social network links)
Comment 29 Paul Poulain 2012-09-12 12:21:24 UTC
(In reply to comment #28)
> QA comment: if i'm not wrong owen, you replaced
> /opac-tmpl/prog/images/socnet/facebook16.png by a social sprite, but
> facebook16.png is not removed.
> 
> Shouldn't it be removed ? (same thing for other social network links)

Owen = ping
Comment 30 Owen Leonard 2012-09-12 12:45:53 UTC Comment hidden (obsolete)
Comment 31 Paul Poulain 2012-09-13 15:54:19 UTC
The 3 follow-up have been pushed to master:
 Bug 7500 [FOLLOW-UP][Star ratings] Use CSS Sprites for faster page loading
 Bug 7500 [FOLLOW-UP][Missing images] Use CSS Sprites for faster page loading
 Bug 7500 [FOLLOW-UP, revised][Social networks] Use CSS Sprites for faster page loading 
(does not apply to new/Bug_7500, that is old, so pushed directly to master)
Comment 32 Owen Leonard 2012-11-08 17:27:41 UTC Comment hidden (obsolete)
Comment 33 Katrin Fischer 2012-11-10 20:51:50 UTC
Owen's description for the last patch sounds like a bug - please reset to enhancement if I am wrong.
Comment 34 Marcel de Rooy 2012-11-12 08:14:19 UTC Comment hidden (obsolete)
Comment 35 Marcel de Rooy 2012-11-12 08:19:01 UTC
QA Comment for attachment 13386 [details] [review]: trivial one-line fix. Passed QA
Comment 36 Jared Camins-Esakov 2012-11-12 14:54:16 UTC
The patch that is attached to this bug seems to be unrelated. Could you please attach the signed-off version of the patch, Marcel?
Comment 37 Marcel de Rooy 2012-11-13 12:18:48 UTC
Created attachment 13421 [details] [review]
Signed patch (lists follow-up)

This should be the right one now :)
Comment 38 Jared Camins-Esakov 2012-11-13 14:51:32 UTC
This patch has been pushed to master.
Comment 39 Paul Poulain 2012-11-16 17:34:54 UTC
Patch pushed to branch 3.10.x
Comment 40 Owen Leonard 2013-04-05 13:19:21 UTC
When YUI toolbars are fully purged from the staff client we'll need to do some cleanup on stray image files and evaluate what images need to be "sprited" for that interface.