[Django] #28267: Change submit_line implementation to allow easier modification
Django
-------------------------------------------+------------------------
Reporter: Karolis Ryselis | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28267>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Karolis Ryselis | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Does the [https://github.com/django/django/pull/7840 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28267#comment:1>
Django
Reporter: Karolis Ryselis | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Karolis Ryselis):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/28267#comment:2>
Django
Reporter: Karolis Ryselis | Owner: nobody
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Uncategorized => New feature
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28267#comment:3>
Django
Reporter: Karolis Ryselis | Owner: nobody
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Karolis Ryselis):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/28267#comment:4>
Django
Reporter: Karolis Ryselis | Owner: nobody
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
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 Tim Graham):
* stage: Unreviewed => Accepted
Comment:
I'm open to seeing a patch to accommodate your use case.
--
Ticket URL: <https://code.djangoproject.com/ticket/28267#comment:5>
Django
Reporter: Karolis Ryselis | Owner: nobody
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Karolis Ryselis):
[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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28267#comment:6>