Opened 7 years ago

Last modified 7 years ago

#28267 new New feature

Change submit_line implementation to allow easier modification

Reported by: Karolis Ryselis Owned by: nobody
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently submit_line renders its buttons inside a div element and 5 if statements in the template are used to show or hide these buttons. This makes it difficult to add more buttons. Whole html template has to be overridden because there is no way to place the buttons inside div.submit-row element.
We needed quite a few of those buttons, e.g., save as draft, save and add invoice (shortcut button).
There is no way to change the default button either - class="default" is hardcoded in Save button.

Proposed solution:
define buttons in admin class. ModelAdmin could have a method get_submit_line_buttons which returns a list of buttons. A single item of this list could be either a dict or an object of a new Button class. Suggested dict keys or object properties:
id - translates to HTML id attribute
display_name - visible text on the button
name - HTML name attribute
highlighted - bool, equivalent to current class="default" if True
onclick - hook to attach JavaScript

Current buttons could be defined in default ModelAdmin.get_submit_line_buttons implementation with the same conditions as now. Template would consist of a loop that iterates over a list of buttons and renders them.

Change History (6)

comment:1 by Tim Graham, 7 years ago

Does the PR for #27728 ease this? The tests/admin_views/templates/admin/admin_views/article/submit_line.html template looks like adding another button using a template isn't too onerous.

comment:2 by Karolis Ryselis, 7 years ago

Correct me if I am wrong, but the only way to add a new button is to create a submit_line.html under, say, admin/app/model, copy and paste the content from original submit_line.html and make modifications.
If I extend base submit_line.html, I can only add buttons before div.submit-row or after div.submit-row, there is no way to add one new button while reusing existing code. Moreover, if I want to add new button, I will have to copy and paste HTML of another button, changing a few attributes. Finally, if I want to change style of my buttons by adding an HTML class attribute, I will have to modify each button that exists in any of the templates.
If we used the proposed approach, adding a button would become overriding get_submit_line_buttons and adding a button to list. Finally, I could iterate over all buttons in Python code and add a class, change the title of a default button without having to copy and paste existing templates.

What do you think about it?

comment:3 by Tim Graham, 7 years ago

Type: UncategorizedNew feature

That analysis seems correct. My view is that the admin isn't intended for building pixel perfect UIs. The amount of hooks is already somewhat unwieldy so it's a tradeoff between simplicity and what to add to accomplish the common use cases.

comment:4 by Karolis Ryselis, 7 years ago

I agree that current solution does indeed allow to add a new button to submit line. However, I still see some more issues.
I have done several Django version upgrades on a large Django project. Every time problems arise when something is overridden without calling super - by copy - pasting all code from Django default implementation. When Django code is updated, I have to manually make sure that every overridden method is intact with latest changes. Current implementation adds to the number of possible modifications that need to be checked and migration to a new version becomes painful.
I don't know if proposed solution is the best that can be done. What I like about it is that I can avoid the problems I have described; I like defining things in Python more than defining them in HTML.

If you don't like changing everything in the template, there are different possible solutions:

  • move wrapping div outside of submit_line.html. Pros: easy button adding before and after default buttons by extending existing template. Cons: if inner HTML of default buttons changes and I want my custom buttons to match them, I have to change every button; difficult to add a button in the middle - still requires overriding whole template; div must be added in all places where submit_line.html is used.
  • wrap existing buttons in a block that lives inside the div. Pros and cons are the same, except for adding div everywhere around submit_line.html

Despite this I feel like buttons belong to the admin, not the template, because admin needs to handle these buttons anyway. As far as current implementation, buttons (their names specifically) are defined in the template and used in change form view in admin.

I do not know if proposed solution is the best we can do, but I feel like current implementation is not excellent either

comment:5 by Tim Graham, 7 years ago

Triage Stage: UnreviewedAccepted

I'm open to seeing a patch to accommodate your use case.

comment:6 by Karolis Ryselis, 7 years ago

https://github.com/ryselis/django/commit/94c8fd8fc5cf38000a1b3dea7a1717768b7fac40
Here is a commit that implements the suggested change. It currently has bad documentation and unit tests are not fixed. Please tell me what you think about this approach.

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