Opened 4 years ago

Closed 2 years ago

#16216 closed Cleanup/optimization (wontfix)

Replace all occurences of img src attributes with css backgrounds

Reported by: bheesink Owned by: zerok
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: zerok@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

Cleanup admin pages and remove unnecessary occurrences of img src attributes. This should be cleaned up so improving or redesigning the admin in the future is more easy.

The img src attributes occur in the following files:

django/contrib/admin/templates/admin/change_list_result.html (line 19)
django/contrib/admin/templates/admin/search_form.html (line 6)
django/contrib/admin/templatetags/admin_list.py (line 161)
django/contrib/admin/widgets.py (line 139, 245)
django/template/defaulttags.py (line 1296)
tests/regressiontests/admin_util/tests.py (line 136)

tests/regressiontests/admin_widgets/tests.py (line 258, 273, 290, 309, 414)
tests/regressiontests/defaultfilters/tests.py (line 272, 275)

Change History (12)

comment:1 Changed 4 years ago by bheesink

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from 1.3 to SVN

comment:2 follow-up: Changed 4 years ago by lukeplant

  • UI/UX set

It's not at all obvious to me that this is correct. When images are purely decorative then it is fine to replace them with CSS. However, in some cases they are definitely not e.g. django/contrib/admin/templatetags/admin_list.py (line 161) - this icon carries information. In other cases it might be appropriate. But even things like the search icon carry clues as to purpose. If you convert to CSS, this information is lost if the user is, for instance, blind.

BTW, src is a required attribute of img, so we can't just remove that attribute.

comment:3 in reply to: ↑ 2 Changed 4 years ago by bheesink

Replying to lukeplant:

It's not at all obvious to me that this is correct. When images are purely decorative then it is fine to replace them with CSS. However, in some cases they are definitely not e.g. django/contrib/admin/templatetags/admin_list.py (line 161) - this icon carries information. In other cases it might be appropriate. But even things like the search icon carry clues as to purpose. If you convert to CSS, this information is lost if the user is, for instance, blind.

BTW, src is a required attribute of img, so we can't just remove that attribute.

You are right... i meant that i want to remove the img tags and replace them for divs.
The icons that carry information can be replaced for example by the following elements:

<img src="some_icon.gif" alt="configuration" />

should be:

<div class="icon-configuration">configuration</div>

or:

<a href="#" class="icon-configuration">configuration</a>

styling:

/* example icons styling */

.icon-configuration {
display: block;
width: 10px;
height: 10px;
text-indent: -9000px;
background: transparent url(/path/to/icon.png);
}

Usability wise I think this is the proper solution. It's also better if the icons getting merged into a sprite in the future.

comment:4 Changed 4 years ago by Horst Gutmann <zerok@…>

  • Cc zerok@… added

Personally I'm very much in favor of moving all the graphical icons from the markup and into CSS to improve customizability. IMO the question might be, though, if this should be done now or if this might be combined with some other improvements regarding the JavaScript integration in the admin and/or a refactoring of the admin altogether.

comment:5 Changed 4 years ago by melinath

  • Triage Stage changed from Unreviewed to Design decision needed

I'd second making this part of a general admin refactor.

comment:6 Changed 2 years ago by aaugustin

  • Triage Stage changed from Design decision needed to Accepted

comment:7 Changed 2 years ago by zerok

So should this ticket perhaps be postponed in favor of the admin-rewrite or would it be worth doing this within the current admin app?

comment:8 Changed 2 years ago by apollo13

Now is fine, the wanted admin rewrite shouldn't block changes in the current app.

Last edited 2 years ago by apollo13 (previous) (diff)

comment:9 Changed 2 years ago by zerok

  • Owner changed from bheesink to zerok
  • Status changed from new to assigned

comment:11 Changed 2 years ago by timo

I guess this may be superseded by #20597 (using an icon font for the admin images).

comment:12 Changed 2 years ago by aaugustin

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

Yes, I think so.

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