Opened 5 years ago

Closed 5 years ago

#16186 closed Cleanup/optimization (fixed)

Remove inline CSS from admin templates

Reported by: bsimons Owned by: Chi Shang Cheng
Component: contrib.admin Version: master
Severity: Normal Keywords: admin, templates, html, css, javascript
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

The templates in contrib.admin have a lot of inline javascript. Also inline css styling appears at several places. This should be cleaned up so improving or redesigning the admin in the future is more easy. Javascripts could and should also be more based on jQuery, since it has been available in the admin anyway for some time now.

Attachments (2)

16186-inline-css-removed.diff (872 bytes) - added by Chi Shang Cheng 5 years ago.
Patch for removing inline style in change_list_results.html (16186)
16186-inline-css-removed-updated.diff (2.0 KB) - added by Niels van Dijk 5 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 years ago by Chi Shang Cheng

Has patch: set
Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Chi Shang Cheng
Patch needs improvement: unset

Changed 5 years ago by Chi Shang Cheng

Patch for removing inline style in change_list_results.html (16186)

comment:2 Changed 5 years ago by Julien Phalip

Triage Stage: UnreviewedAccepted
UI/UX: set

comment:3 Changed 5 years ago by DHessing

Aren't these two different issues? Cleaning up the admin pages to remove the inline javascript/css. Then as a seperate ticket, you could start converting the javascript to jQuery in a central place, if we even want to do that. Also, like Idan said, we should first create a shitlist of all inline css/js in the admin, maybe on a wiki.

comment:4 in reply to:  1 Changed 5 years ago by DHessing

Patch needs improvement: set

Replying to cscheng:
Patch looks ok, but we might need more to close this ticket ;)

comment:5 Changed 5 years ago by Julien Phalip

Triage Stage: AcceptedDesign decision needed

Yeah, on further reflection, this ticket aims to tackle way too much. There are already a number of tickets suggesting to convert some existing pieces of javascript to using jQuery, for example: #13, #25, #15220, #13883, #15231, #13068. So the jQuery conversion should definitely stay out of this ticket and either be tackled piecemeal by these various tickets or be done all in one go as part of a major revamp of the admin.

Removing the inline CSS seems more reasonable for this ticket. However, a thorough audit of the existing codebase should be done first before deciding on the right approach for tackling this task.

For these reasons, I'm marking this ticket as DDN. I'm going to ask Idan for his view on this, as I assume he's got plans :)

Last edited 5 years ago by Julien Phalip (previous) (diff)

comment:6 Changed 5 years ago by Idan Gazit

Summary: The admin templates need some loveRemove inline CSS from admin templates
Triage Stage: Design decision neededAccepted

OK, let's limit the scope of this ticket to removing inline CSS in the admin.

Eventually, we can open a separate ticket for inline JS/refactoring which doesn't fall under one of the existing tickets you mentioned already.

Changed 5 years ago by Niels van Dijk

comment:7 Changed 5 years ago by Niels van Dijk

Updated the patch. Removed some more inline styles..

comment:8 Changed 5 years ago by Jannis Leidel

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:9 Changed 5 years ago by Idan Gazit

Resolution: fixed
Status: newclosed

In [16346]:

Fixes #16186 -- remove inline CSS in contrib.admin

Thanks to bsimons for the report and the patches from cscheng, nvandijk!

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