[Django] #35115: Empty footer element in main section of admin layout
Django
------------------------------------------------+------------------------
Reporter: Marijke Luttekes | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 5.0
Severity: Normal | Keywords: HTML
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------------+------------------------
The Django admin layout has an empty `div#footer` in the main element.
== Location
Find the code in `django/contrib/admin/templates/admin/base.html`:
{{{
{% block footer %}<div id="footer"></div>{% endblock %}
}}}
The code was last touched about four years ago when someone fixed the
indentation on this template.
== Proposed solutions
There are two ways to go about this:
1. Remove it, as there appears to be no immediate use for this element.
2. Keep it, but in the spirit of semantic HTML, replace it with a
`<footer>` element outside the `<main>` element.
--
Ticket URL: <https://code.djangoproject.com/ticket/35115>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Marijke Luttekes | Owner: nobody
Cleanup/optimization |
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: HTML | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Natalia Bidart):
* cc: Tom Carrick (added)
Comment:
Hello! Thank you for your report. I do see the empty section as you
describe, but I also see some specific CSS that was defined for this:
https://github.com/django/django/blob/main/django/contrib/admin/static/admin/css/base.css#L896
I'm going to add Tom to this ticket to see if they have any further
feedback/insight.
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Marijke Luttekes | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: HTML | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Baptiste Mispelon):
Using `git log -S footer -- django/contrib/admin` I found what I think is
the commit that added the line: 2673aa366a3def5ba7afc26badf6c194b515b3bf
The commit message seems to indicate it was added an an extension point.
In my opinion that means we could instead leave the `{% block footer %}`
empty, and delete any css associated with `#footer`.
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Marijke Luttekes | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: HTML | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tom Carrick):
Yeah I think this is intended to be overridden. I agree with removing the
CSS, and also with moving it out of the main element and to use a semantic
footer element by default.
This is backwards incompatible however, as it may affect a user's
overridden template. Since this isn't explicitly documented I would find
it acceptable. It would be nice to see if this is used much in the wild
but searching for `{% block footer %}` will yield more false positives
than anything else. I think it would at least need something in the
release notes explaining it.
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:3>
Django
Reporter: Marijke Luttekes | Owner: nobody
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Unreviewed => Accepted
Comment:
Replying to [comment:3 Tom Carrick]:
> Yeah I think this is intended to be overridden. I agree with removing
the CSS, and also with moving it out of the main element and to use a
semantic footer element by default.
Sounds like a plan.
{{{#!diff
diff --git a/django/contrib/admin/templates/admin/base.html
b/django/contrib/admin/templates/admin/base.html
index 1ca50e508d..46d75c6090 100644
--- a/django/contrib/admin/templates/admin/base.html
+++ b/django/contrib/admin/templates/admin/base.html
@@ -108,8 +108,8 @@
<br class="clear">
</div>
<!-- END Content -->
- {% block footer %}<div id="footer"></div>{% endblock %}
</main>
+ {% block footer %}<footer></footer>{% endblock %}
</div>
</div>
<!-- END Container -->
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: Marijke Luttekes | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: HTML | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Thibaud Colas):
I have two concerns with this that I’d recommend investigating further:
- Do screen readers announce landmark regions like footer that are empty?
If so that would be problematic. You should be able to test this with
VoiceOver on mac, and in particular the rotor: https://support.apple.com
/en-gb/guide/voiceover/mchlp2719/mac. If that feels like too much, please
say so and we’ll have the Django accessibility team picking this up
- Is it clear enough to people who would override this, that with it
outside `main`, they’d definitely need to use a `<footer>` tag? Otherwise
their content would be outside a landmark
(https://dequeuniversity.com/rules/axe/4.3/region).
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:5>
Django
--------------------------------------+------------------------------------
Reporter: Marijke Luttekes | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: HTML | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:5 Thibaud Colas]:
> I have two concerns with this that I’d recommend investigating further:
>
> - Do screen readers announce landmark regions like footer that are
empty? If so that would be problematic. You should be able to test this
with VoiceOver on mac, and in particular the rotor:
https://support.apple.com/en-gb/guide/voiceover/mchlp2719/mac. If that
feels like too much, please say so and we’ll have the Django accessibility
team picking this up
> - Is it clear enough to people who would override this, that with it
outside `main`, they’d definitely need to use a `<footer>` tag? Otherwise
their content would be outside a landmark
(https://dequeuniversity.com/rules/axe/4.3/region).
I wanted do keep `<footer>` as a guide for others. We can also force them
to use it by:
{{{
<footer>{% block footer %}{% endblock %}</footer>
}}}
This, of course, does not answer your first question.
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:6>
Django
Reporter: Marijke Luttekes | Owner: Marijke
Type: | Luttekes
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: HTML | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Thibaud Colas):
* owner: nobody => Marijke Luttekes
* status: new => assigned
Comment:
Thanks Mariusz, yep, I think that’s a good way to achieve this. I believe
Marijke intends to work on this so will assign her. Am happy to do the
screen reader testing if it helps.
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Marijke Luttekes | Owner: Marijke
Type: | Luttekes
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: HTML | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Marijke Luttekes):
Thank you, I will pick it up!
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Marijke Luttekes | Owner: Marijke
Type: | Luttekes
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: HTML | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Marijke Luttekes):
I've added a patch.
I have used macOS VoiceOver (VO) to test spoken text of this landmark, and
Firefox's built-in accessiblity inspector.
The accessiblity inspector describes this element with having role
`contentinfo`.
VO ignores the element when it is empty.
When putting a link in the footer, VO reads it. When selecting the parent
using Rotor (VO modifier + shift + up arrow), it only spoke "content".
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Marijke Luttekes | Owner: Marijke
Type: | Luttekes
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: HTML | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Marijke Luttekes | Owner: Marijke
Type: | Luttekes
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: fixed
Keywords: HTML | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
Fixed by e412d85b4626bc56eb25206a40c3529162ce5dfc.
--
Ticket URL: <https://code.djangoproject.com/ticket/35115#comment:11>