Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#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 Richard Laager)

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:

  1. There is another level: <div class="flex-container form-multiline">
  2. There is another level: <div>
  3. The <div class="help"> is no longer inside of the <div class="fieldBox field-X">.

The Bugs

  1. 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.
  1. 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 no width: 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)

phone-after1.png (35.3 KB ) - added by Richard Laager 3 months ago.
phone-after2.png (30.5 KB ) - added by Richard Laager 3 months ago.
phone-before1.png (33.5 KB ) - added by Richard Laager 3 months ago.
phone-before2.png (28.9 KB ) - added by Richard Laager 3 months ago.
tablet-after2.png (31.6 KB ) - added by Richard Laager 3 months ago.
tablet-mid2.png (28.4 KB ) - added by Richard Laager 3 months ago.
test_first_field_focus--focus-multi-widget--desktop_size.png (54.1 KB ) - added by Sarah Boyce 3 months ago.
test_first_field_focus--focus-multi-widget--mobile_size.png (32.0 KB ) - added by Sarah Boyce 3 months ago.

Download all attachments as: .zip

Change History (14)

by Richard Laager, 3 months ago

Attachment: phone-after1.png added

by Richard Laager, 3 months ago

Attachment: phone-after2.png added

by Richard Laager, 3 months ago

Attachment: phone-before1.png added

by Richard Laager, 3 months ago

Attachment: phone-before2.png added

by Richard Laager, 3 months ago

Attachment: tablet-after2.png added

by Richard Laager, 3 months ago

Attachment: tablet-mid2.png added

comment:1 by Richard Laager, 3 months ago

Description: modified (diff)
Has patch: set

comment:2 by Sarah Boyce, 3 months ago

Resolution: worksforme
Status: newclosed

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

    a b site.register(Bookmark)  
    12331233site.register(CyclicOne)
    12341234site.register(CyclicTwo)
    12351235site.register(WorkHour, WorkHourAdmin)
    1236 site.register(Reservation)
     1236site.register(Reservation, fields=(("start_date", "price"),))
    12371237site.register(FoodDelivery, FoodDeliveryAdmin)

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?

comment:3 by Natalia Bidart, 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.)

comment:4 by Sarah Boyce, 3 months ago

If it's only about check boxes, it might be a duplicate of #35386

comment:5 by Richard Laager, 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.

in reply to:  5 comment:6 by Natalia Bidart, 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!

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