[Django] #35439: Hardcoded HTML in python code.

[Django]

#35439: Hardcoded HTML in python code.
------------------------------------------------+------------------------
Reporter: sesostris | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 5.0
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 |
------------------------------------------------+------------------------
There is a hardcoded snippet of HTML in the
django.contrib.admin.templatetags.admin_list module on lines 99 to 110
that will be used in the header of the changelist results table.

https://github.com/django/django/blob/0e445badd54fafc75dd1a5dff9fee6e6a171eafe/django/contrib/admin/templatetags/admin_list.py#L99C1-L110C25

{{{
# if the field is the action checkbox: no sorting and special
class
if field_name == "action_checkbox":
aria_label = _("Select all objects on this page for an
action")
yield {
"text": mark_safe(
f'<input type="checkbox" id="action-toggle" '
f'aria-label="{aria_label}">'
),
"class_attrib": mark_safe(' class="action-checkbox-
column"'),
"sortable": False,
}
continue
}}}

It would be better to use a CheckboxInput widget to render this HTML
element. The code would look like this:

{{{
from django.forms import CheckboxInput

# if the field is the action checkbox: no sorting and special
class
if field_name == "action_checkbox":
widget = CheckboxInput(
attrs={
"aria-label": _(
"Select all objects on this page for an action"
),
"id": "action-toggle",
}
)
yield {
"text": mark_safe(
widget.render(name="action-toggle", value=False)
),
"class_attrib": mark_safe(' class="action-checkbox-
column"'),
"sortable": False,
}
continue
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35439>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35439: Hardcoded HTML in python code.

-------------------------------------+-------------------------------------
Reporter: sesostris | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: wontfix

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 David Smith):

* resolution: => wontfix
* status: new => closed

Comment:

Thank you for the ticket.

I'm not sure it's worth changing as there are test failures with this
change so it is not equivalent. The ticket mentions that using a widget
would be better but I'm unsure why using a widget and rending the HTML
using the template engine would be better in this particular case.
--
Ticket URL: <https://code.djangoproject.com/ticket/35439#comment:1>

[Django]

#35439: Hardcoded HTML in python code.
-------------------------------------+-------------------------------------
Reporter: sesostris | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: wontfix
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 Natalia Bidart):

I would also like to read the answer to the last question (''why using a
widget and rending the HTML using the template engine would be better'')
but is worth nothing that the following diff has the test passing OK so at
a code level it seems equivalent:

{{{#!diff
diff --git a/django/contrib/admin/templatetags/admin_list.py
b/django/contrib/admin/templatetags/admin_list.py
index fdf6e63f5f..e7b4fcb81f 100644
--- a/django/contrib/admin/templatetags/admin_list.py
+++ b/django/contrib/admin/templatetags/admin_list.py
@@ -1,5 +1,6 @@
import datetime

+from django.forms import CheckboxInput
from django.contrib.admin.templatetags.admin_urls import
add_preserved_filters
from django.contrib.admin.utils import (
display_for_field,
@@ -99,10 +100,15 @@ def result_headers(cl):

# if the field is the action checkbox: no sorting and special
class
if field_name == "action_checkbox":
aria_label = _("Select all objects on this page for an
action")

+ widget = CheckboxInput(
+ attrs={
+ "aria-label": aria_label,
+ "id": "action-toggle",
+ }
+ )
yield {
"text": mark_safe(
- f'<input type="checkbox" id="action-toggle" '
- f'aria-label="{aria_label}">'
+ widget.render(name="action-toggle", value=False)

),
"class_attrib": mark_safe(' class="action-checkbox-
column"'),
"sortable": False,

diff --git a/tests/admin_changelist/tests.py
b/tests/admin_changelist/tests.py
index bf85cf038f..bf6245ca1f 100644
--- a/tests/admin_changelist/tests.py
+++ b/tests/admin_changelist/tests.py
@@ -359,7 +359,7 @@ class ChangeListTests(TestCase):
"Failed to find expected row element: %s" % table_output,
)
self.assertInHTML(
- '<input type="checkbox" id="action-toggle" '
+ '<input type="checkbox" id="action-toggle" name="action-
toggle"'
'aria-label="Select all objects on this page for an
action">',
table_output,
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35439#comment:2>