[Django] #20597: Replace admin icons images by an icon font.
Django
--------------------------------------+--------------------
Reporter: loic84 | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
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 [http://fortawesome.github.io/Font-Awesome/ 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: loic84 | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:1>
Django
Reporter: loic84 | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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/`.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:2>
Django
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by Einar Johannsson <hey@…>):
* status: new => assigned
* owner: nobody => anonymous
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:3>
Django
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by timo):
Sample project looks good to me. I guess this approach supersedes #16216?
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:4>
Django
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by collinanderson):
* ui_ux: 0 => 1
Comment:
correct me if UX/UI is not proper here.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:5>
Django
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 1
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by iamstephenkerr):
Should you not use the more up to date Font Awesome, I'll be happy to look
into this if its ok?
http://fortawesome.github.io/Font-Awesome/icons/
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:6>
Django
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 1
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by collinanderson):
I also wonder if at some point we'll want to switch to svg. Though, we
would need to drop IE8 first.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:7>
Django
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:8>
Django
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by timgraham):
If I read it correctly, IE8 support ends Jan. 12, 2016 so Django 2.0 or
later should be a good candidate.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:9>
Django
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by rm_):
* cc: riccardo.magliocchetti@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by timgraham):
Me on [https://groups.google.com/d/msg/django-
developers/HJAikaEBqJ4/UsvIjs_QCgAJ 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."
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (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
[[http://stackoverflow.com/questions/11978995/how-to-change-color-of-svg-
image-using-css-jquery-svg-image-replacement|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 [[http://caniuse.com/#search=mask-image|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).
[[https://groups.google.com/d/msg/django-
developers/HJAikaEBqJ4/UsvIjs_QCgAJ|See discussion]] in django-developers.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by timgraham):
If we lack other expert opinions (I am not one), I don't object to your
reasoning.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (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".
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (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.
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (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'''
- Customizing. Changing icon color with CSS
- Additional 100KB (font file + css)
- Lots of changes in CSS, HTML and JS
- To support IE8: just add *.ttf file (+ 70KB)
- Code Diff: https://github.com/django/django/compare/master...elky:font-
icons?diff=unified&name=font-icons
'''SVG'''
- Much less changes
- Code is elegant - I actually ''just replaced'' gifs with svg in CSS. No
pseudo-elements. No alignment tweaks
- 25 additional files (requests) but just 19KB in total
- To support IE8: add fallback in CSS (since we already have gif icons in
the repo, my suggestion is to show them in IE8)
- Code Diff: https://github.com/django/django/compare/master...elky:svg-
icons?diff=unified&name=svg-icons
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by timgraham):
* keywords: => 1.9
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (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?
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: 1.9 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
We are going with the inline SVG solution so we can use `alt`.
[https://github.com/django/django/pull/5186 PR] (with some todos
remaining).
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: 1.9 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: 1.9 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c32b61c6fd6ddf39fd49585954d422ef8d7304f6" c32b61c6]:
{{{
#!CommitTicketReference repository=""
revision="c32b61c6fd6ddf39fd49585954d422ef8d7304f6"
Fixed #20597 -- Replaced admin GIF/PNG icons by SVG
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: loic84 | Owner: anonymous
Type: | Status: closed
Cleanup/optimization |
Severity: Normal | Resolution: fixed
Keywords: 1.9 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"22a791e608922d55552da6c7c9ddc81ae29b7ba4" 22a791e]:
{{{
#!CommitTicketReference repository=""
revision="22a791e608922d55552da6c7c9ddc81ae29b7ba4"
Refs #20597 -- Fixed spelling of HiDPI.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20597#comment:22>