#35769 closed Bug (worksforme)
Multiline fields display wrong in tablet/phone sizes
Reported by: | Richard Laager | Owned by: | |
---|---|---|---|
Component: | contrib.admin | Version: | 4.2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | yes |
Description (last modified by )
This is another regression from commit 96a598356a9ea8c2c05b22cadc12e256a3b295fd:
https://github.com/django/django/commit/96a598356a9ea8c2c05b22cadc12e256a3b295fd
from PR 16161:
https://github.com/django/django/pull/16161
Background:
In that commit, the structure of the HTML changed.
If there are multiple fields in a row, it changed from:
<div class="form-row field-a field-b"> <div class="fieldBox field-a"> <label><!-- NOTE: Checkboxes have label and input reversed. --> <input> <div class="help">...</div> </div> <div class="fieldBox field-b"> <label> <input> <div class="help">...</div> </div> </div>
to:
<div class="form-row field-a field-b"> <div class="flex-container form-multiline"> <div> <div class="flex-container fieldBox field-a"><!-- NOTE: Checkboxes have a checkbox-row class. --> <label><!-- NOTE: Checkboxes have label and input reversed. --> <input> </div> <div class="help">...</div> </div> <div> <div class="flex-container fieldBox field-b"> <label> <input> </div> <div class="help">...</div> </div> </div> </div>
If there is only one field in the row, it changed from:
<div class="form-row field-a field-b"> <div><!-- NOTE: Checkboxes have a checkbox-row class. --> <label><!-- NOTE: Checkboxes have label and input reversed. --> <input> <div class="help">...</div> </div> </div>
to:
<div class="form-row field-a"> <div> <div class="flex-container"><!-- NOTE: Checkboxes have a checkbox-row class. --> <label><!-- NOTE: Checkboxes have label and input reversed. --> <input> </div> <div class="help">...</div> </div> </div>
There are several changes to the HTML structure relevant to this discussion:
- There is another level:
<div class="flex-container form-multiline">
- There is another level:
<div>
- The
<div class="help">
is no longer inside of the<div class="fieldBox field-X">
.
The Bugs
- When there are multiple fields in a row, there are issues in both the tablet and mobile screen sizes. First off, there are two instances of a selector that's supposed to match, but doesn't because of the changed structure. If we fix that to match the new structure, then that fixes the phone size class. See phone-before1.png, phone-before2.png, phone-after1.png, and phone-after2.png. The "1" images are manually grabbed/cropped and show a single-line field above it, which is good for comparing to the working single-line example. The "2" images are of that particular DOM node via the Google Chrome Inspector's feature to screenshot just a given node, so those are easier to directly compare.
- With that first issue fixed, the tablet view still isn't correct. In Django 3.2, moving to the tablet view forced one-field-per-line whether that was necessary (width-wise) or not. If I take the same approach here, by setting
width: 100%
on that<div>
, then I get the same behavior and it looks good. Compare tablet-mid2.png (which has the fixed selectors, but nowidth: 100%
) to tablet-after2.png (which has both).
The Patch
--- a/django/contrib/admin/static/admin/css/responsive.css +++ b/django/contrib/admin/static/admin/css/responsive.css @@ -199,7 +199,8 @@ input[type="submit"], button { min-height: 0; } - fieldset .fieldBox + .fieldBox { + fieldset div:has(> .fieldBox) + div:has(> .fieldBox) { + width: 100%; margin-top: 10px; padding-top: 10px; border-top: 1px solid var(--hairline-color); @@ -577,7 +578,7 @@ input[type="submit"], button { width: auto; } - fieldset .fieldBox + .fieldBox { + fieldset div:has(> .fieldBox) + div:has(> .fieldBox) { margin-top: 15px; padding-top: 15p
Other concerns:
This doesn't affect Django itself, but affects third-party customizations of the admin... It's a pain to select the div for a given field. (For one example use case, we do this all over the place to set a width on the first field in the row, such that the second fields in the row will line up visually.) Previously, I could use div.fieldBox.field-X
(for the multiple-fields-per-line case). But since the help div is not inside that div, but a peer to it, I need to get the parent div, but that has no classes. To get a selector that matches it (but not its children), I end up with this ugliness: div.field-X > div > div:first-child
I'm skeptical that even with all the fixes and everything I've specifically discussed above that all the CSS has been updated properly. For example, search the tree for the regex \+ div.help
. There are things like form .aligned input + div.help
that would have matched before, but don't match any more. I also suggest checking all the .vCheckboxLabel
selectors.
Attachments (8)
Change History (14)
by , 3 months ago
Attachment: | phone-after1.png added |
---|
by , 3 months ago
Attachment: | phone-after2.png added |
---|
by , 3 months ago
Attachment: | phone-before1.png added |
---|
by , 3 months ago
Attachment: | phone-before2.png added |
---|
by , 3 months ago
Attachment: | tablet-after2.png added |
---|
by , 3 months ago
Attachment: | tablet-mid2.png added |
---|
comment:1 by , 3 months ago
Description: | modified (diff) |
---|---|
Has patch: | set |
by , 3 months ago
Attachment: | test_first_field_focus--focus-multi-widget--desktop_size.png added |
---|
by , 3 months ago
Attachment: | test_first_field_focus--focus-multi-widget--mobile_size.png added |
---|
comment:2 by , 3 months ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:3 by , 3 months ago
Hello Richard! Besides what Sarah said, please note that Django version 4.2 is on security-only maintenance mode, so if you still see this issue we need confirmation that this is occurring in Django 5.1 and/or the main
branch.
(I have tested in main
using models with checkboxes, thinking it may be a checkbox-only issue, but I can't reproduce.)
follow-up: 6 comment:5 by , 3 months ago
Whatever the issue is, it's not a duplicate of #35386, as I re-applied the fix for that already (reverted the revert) in my tree.
comment:6 by , 3 months ago
Replying to Richard Laager:
Whatever the issue is, it's not a duplicate of #35386, as I re-applied the fix for that already (reverted the revert) in my tree.
Then in that case we definitely need a reliable way to reproduce this. This could be a minimal Django test project, or the model and admin definitions that would showcase this issue.
Please re-open when you can provide that.
Thank you!
When I make the following update and run the screenshot tests (
python ./runtests.py --selenium=chrome --screenshots admin_views.tests.SeleniumTests
),tests/admin_views/admin.py
)I get the following which look fine to me
Desktop:
Mobile:
Have you been able to replicate on main? Can you share the code of a Django admin/model to replicate?