Opened 2 years ago

Closed 30 hours ago

Last modified 28 hours ago

#20597 closed Cleanup/optimization (fixed)

Replace admin icons images by inline SVG

Reported by: loic84 Owned by: anonymous
Component: contrib.admin Version: master
Severity: Normal Keywords: 1.9
Cc: idangazit, riccardo.magliocchetti@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: yes

Description

Some of the benefits of using an icon font:

  • Resolution independence: works well on Retina displays.
  • Can be customized by CSS: color, opacity, shadow, etc.
  • Most icon fonts offer a large collection of commonly used icons - while remaining small in file size - covering possible future needs.
  • Widespread browser support, including IE7 support.

I'd like to suggest Font-Awesome which is actively maintained, currently offers 361 icons and whose license is compatible with BSD.

A quick glance at the img assets of django.contrib.admin showed that most if not all icons have an equivalent in Font-Awesome.

Note: This issue was discussed on IRC with Idan and follows previous discussion on #19900.

Change History (22)

comment:1 Changed 2 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 2 years ago by loic84

https://github.com/loic/django/compare/ticket20597

Proof of concept that replaces the following:

  • add/change icons for each model on index.
  • add/change/delete icon on the recent items widget on index.
  • delete link icon on change_view.
  • yes/no/unknown images on boolean fields.

I committed a sample project to make it easy to review: just syncdb, runserver then head to /admin/.

comment:3 Changed 2 years ago by Einar Johannsson <hey@…>

  • Owner changed from nobody to anonymous
  • Status changed from new to assigned

comment:4 Changed 2 years ago by timo

Sample project looks good to me. I guess this approach supersedes #16216?

comment:5 Changed 11 months ago by collinanderson

  • UI/UX set

correct me if UX/UI is not proper here.

comment:6 Changed 11 months ago by iamstephenkerr

Should you not use the more up to date Font Awesome?

http://fortawesome.github.io/Font-Awesome/icons/

Last edited 11 months ago by iamstephenkerr (previous) (diff)

comment:7 Changed 11 months ago by collinanderson

I also wonder if at some point we'll want to switch to svg. Though, we would need to drop IE8 first.

comment:8 Changed 3 months ago by collinanderson

More on fonts vs SVG https://css-tricks.com/icon-fonts-vs-svg/. I think we should hold out until we can simply switch to SVG.

comment:9 Changed 3 months ago by timgraham

If I read it correctly, IE8 support ends Jan. 12, 2016 so Django 1.10 or later should be a good candidate.

Last edited 8 weeks ago by timgraham (previous) (diff)

comment:10 Changed 2 months ago by rm_

  • Cc riccardo.magliocchetti@… added

comment:11 Changed 4 weeks ago by timgraham

  • Summary changed from Replace admin icons images by an icon font. to Replace admin icons images by inline SVG

Me on django-developers: "Now that Django 1.9 is slated for December and IE8 support ends the next month, I think it might be acceptable to drop IE8 support in 1.9 and go with the SVG solution now."

comment:12 Changed 3 weeks ago by elky

Despite all advantages of inline SVG it is pretty complex solution to implement and support in Django admin.

  1. SVG is pretty big to insert in markup (include tag for each icon as a solution?)
  2. We can use simple <img> with SVG as a source but this approach is weak - we can't change SVG color in this case. So we'll have to serve already colorized SVG files (not flexible solution). There is JS solution to replace img with inline SVG but I'm not really sure that it's good practice.
  3. There are many places where icons render in JS (widgets) - I don't see any not ugly solution to add inline SVG into it. Using div element with SVG as a background is a weak solution too - no way to colorize it (CSS3 mask has poor browser support)
  4. 20+ additional server requests
  5. Support. Contributors will have to add (and draw?) new icons manually which brakes development process.

I vote for font icons (like Font Awesome). See discussion in django-developers.

comment:13 Changed 3 weeks ago by timgraham

If we lack other expert opinions (I am not one), I don't object to your reasoning.

comment:14 Changed 3 weeks ago by mjtamlyn

This isn't a clear cut argument either way at the moment. Icon fonts are simpler to use in many cases, they can easily be coloured etc. Inline SVG I agree is not worth the extra work most of the time - unless you are wanting to change the SVG content a background is much easier and can be achieved with similar CSS.

SVG definitely wins on accessibility, which is something we need to consider, but I'm also happy with "working code".

comment:15 Changed 3 weeks ago by collinanderson

We could instead just swap out our .gifs with .svg and not touch the markup much if at all (not inline the svg, leave them as img tags). That could be an even simpler situation than using fonts. It should be the same number of requests that we currently have (rather than a reduced number of requests).

Otherwise, I'm fine with fonts. I just wanted to make sure we looked into the SVG option before switching to fonts. The bonus is that we can keep IE8 support at little longer.

Last edited 3 weeks ago by collinanderson (previous) (diff)

comment:16 Changed 12 days ago by elky

Well, only week ago I was 100% happy with font icons. But after Collin's comment I vote for SVG (<img> with svg as a source).
Below I listed advantages/disadvantages for both options.

Font

SVG

And the final and main argument to choose SVG is that some apps use a bit overridden Django CSS classes so font approach may cause visual issues (I checked it with few CMS apps). So font approach is an extra headache for app developers.

comment:17 Changed 7 days ago by timgraham

  • Keywords 1.9 added

comment:18 Changed 4 days ago by hobarrera

I mentioned this on the list, but don't see this mentioned here: Where do font-icons stand in terms of accessibility? Do we have something similar to the alt attribute?

comment:19 Changed 4 days ago by timgraham

  • Has patch set
  • Patch needs improvement set

We are going with the inline SVG solution so we can use alt.

PR (with some todos remaining).

comment:20 Changed 33 hours ago by elky

We cannot use alt for elements where image used as background. Actually Django had accessibility problems before SVG implementation -- there are only few places where alt attribute available.

I think we need separate ticket to implement accessibility support.

comment:21 Changed 30 hours ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In c32b61c6:

Fixed #20597 -- Replaced admin GIF/PNG icons by SVG

comment:22 Changed 28 hours ago by Tim Graham <timograham@…>

In 22a791e:

Refs #20597 -- Fixed spelling of HiDPI.

Note: See TracTickets for help on using tickets.
Back to Top