Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hello,
It's generally better to use sprite. Here a patch for that. Old files sizes are 196 and 187 bytes. New file size (sprite) is 150 bytes. And sometimes 1 http request is saved.
Comment | File | Size | Author |
---|---|---|---|
#27 | extlink-963374-27.patch | 1.22 KB | elachlan |
#25 | extlink-963374-25.patch | 1.35 KB | elachlan |
#18 | extlink-sprite-d7-963374-18.patch | 4.91 KB | mgifford |
#17 | extlink-sprite-963374-17.patch | 1.76 KB | mgifford |
#8 | extlink-sprite.patch | 2.46 KB | jcisio |
Comments
Comment #1
quicksketchIn this situation, I think we'll need to use a horizontal sprite rather than a vertical one. Either that or give it more vertical space. This patch would have a problem with large font sizes.
Comment #2
jcisio CreditAttribution: jcisio commentedNew patch.
Comment #3
elachlan CreditAttribution: elachlan commentedWhat is the progress on this? It would be great to see it implemented.
Comment #4
quicksketchThe patch looks great, I'm just a bit busy and this is a minor issue. I'll commit it next time I'm working on Extlink.
Comment #5
quicksketchArg, bummer. In my testing this breaks the description text on admin/settings/extlink. We've tried to fix this in a dozen ways to no real avail. See #403786: Show themed icons on settings page.
Comment #6
elachlan CreditAttribution: elachlan commentedNow that issue is closed, has this been committed?
Comment #7
quicksketchIt's not closed, it's "needs work". No it hasn't been committed since it breaks the display on admin/settings/extlink.
Comment #8
jcisio CreditAttribution: jcisio commentedShameless try on settings page. It even could be better to remove the class and add a background-url, so that there is no problem if someone overrides the default css. However I'm not quite comfortable doing that with vi now.
I'm not trying to resolve #403786: Show themed icons on settings page, that has been just won't fixed.
Comment #9
elachlan CreditAttribution: elachlan commentedTested and works.
Good work jcisio!!
I will leave the status changes to quicksketch
Comment #10
elachlan CreditAttribution: elachlan commented@quicksketch - Does this fix the issue?
Comment #11
quicksketchI'm not sure these changes are going to work across browsers as-is. I know the "inline-block" CSS is not supported by IE6 (not sure about IE7). Do you think anyone would really care if we just kept all 3 images? That way we could use the sprite image on the front-end and the individual images on the back-end and not worry about this problem at all.
Comment #12
jcisio CreditAttribution: jcisio commentedYes, if it won't put more maintain work on your shoulders ;)
Comment #13
elachlan CreditAttribution: elachlan commentedThe main reason for the change is performance, if the individual images are used in the backend, it doesn't matter as people don't care about that for normal users.
Comment #14
Dane Powell CreditAttribution: Dane Powell commentedGood idea, let's just make sure that the images are losslessly compressed as much as possible (I'm marking #1098186: extlink.png can be losslessly compressed as duplicate)
Comment #15
elachlan CreditAttribution: elachlan commentedHas this been implemented yet?
Comment #16
Dane Powell CreditAttribution: Dane Powell commentedWould you mind re-rolling that patch git-style? Thanks.
Comment #17
mgiffordHere's a D6 patch. Just rerolled as requested in August.
Comment #18
mgiffordHere's a D7 version too. I've added invisible text in so that it is as accessible as it can be. Sprites vs alt text is still an ongoing debate.
It works fine in Firefox, but not in Chrome for some reason.
Comment #19
elachlan CreditAttribution: elachlan commentedBumping too see its inclusion.
Comment #20
elachlan CreditAttribution: elachlan commentedPatch to be re-rolled.
See reference: http://www.w3schools.com/tags/att_global_title.asp
Comment #21
mgiffordThanks @elachlan. Hopefully someone gets to re-rolling that patch.
Adding alt text via a title attribute is a bad idea. Many screenreader simply ignore titles, particularly on images.
Now if an organization like WebAIM.org was suggesting titles, that would be another issue, but w3schools does not have expertise in accessibility.
Comment #22
elachlan CreditAttribution: elachlan commentedGood call.
Will use what we already have then.
Comment #23
mgiffordYup. That will work better.
Comment #24
elachlan CreditAttribution: elachlan commentedComment #25
elachlan CreditAttribution: elachlan commentedRe-rolled the patch with changes.
Comment #27
elachlan CreditAttribution: elachlan commentedRemoved the sprite image file for now from patch.
Comment #28
elachlan CreditAttribution: elachlan commentedCommited to Git. Needs Port to 6.x and 8.x
Comment #29
DamienMcKennaMoving to the correct branch.
Comment #30
elachlan CreditAttribution: elachlan commentedLooks like it's already implemented in 8.x-1.x-dev.
Comment #31
elachlan CreditAttribution: elachlan commented