Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#13599 closed (fixed)

Incorrect HTML in change_list when list_editable is used

Reported by: skevy Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords: list_editable, html, rendering, admin
Cc: adam.skevy@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When using list_editable, an "id" form field gets created at the end of each row in the change list. It get's placed in it's own TD (correctly, I might add), but even though the field is hidden, the TD remains (thus causing incorrect rendering, as seen here: http://cl.ly/4d97f294a05facbb895d).

Posted along with this ticket is a patch that simply adds "display:none;" to any TD's containing hidden fields. Simple fix.

Attachments (4)

hide_td_containing_hidden_input_in_changelist.diff (829 bytes) - added by skevy 4 years ago.
Hide's the td containing a hidden input in the changelist.
separate_hidden_fields.patch (3.1 KB) - added by magnus 4 years ago.
A patch with the hidden inputs placed above the results table
improved_tests_base.diff (3.6 KB) - added by jbronn 4 years ago.
seperate_hidden_fields.patch (6.8 KB) - added by skevy 4 years ago.
Separate hidden fields from table w/ updated tests.

Download all attachments as: .zip

Change History (13)

Changed 4 years ago by skevy

Hide's the td containing a hidden input in the changelist.

comment:1 Changed 4 years ago by skevy

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 4 years ago by russellm

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

I'm not a huge fan of just disappearing the problem -- the hidden field should be rendered such that it doesn't require an extra TD element.

Changed 4 years ago by magnus

A patch with the hidden inputs placed above the results table

comment:3 Changed 4 years ago by magnus

I added a patch where the hidden fields are rendered in a hidden div above the table instead.

comment:4 Changed 4 years ago by anonymous

  • milestone set to 1.3

comment:5 Changed 4 years ago by skevy

I think most of the possible patches for this problem are kinda ugly - but definitely agree with russel that "hiding" the td is not a good solution. I think magnus's solution is good - and it works just fine.

Changed 4 years ago by jbronn

comment:6 Changed 4 years ago by skevy

  • Patch needs improvement unset

Updated the patch to include jbroon and I's updated tests. Also replace the 'style="display:none"' on the div containing the hidden fields with 'class="hiddenfields"' and some css, to make it less ugly (I hate inline css).

Changed 4 years ago by skevy

Separate hidden fields from table w/ updated tests.

comment:7 Changed 4 years ago by jbronn

  • Resolution set to fixed
  • Status changed from new to closed

(In [13744]) Fixed #13599 -- No longer embed hidden <td> elements in ChangeList that cause improper rendering when list_editable is enabled; refactored admin_changelist tests. Thanks, skevy for bug report and patch.

comment:8 Changed 4 years ago by jbronn

(In [13745]) [1.2.X] Fixed #13599 -- No longer embed hidden <td> elements in ChangeList that cause improper rendering when list_editable is enabled; refactored admin_changelist tests. Thanks, skevy for bug report and patch.

Backport of r13744 from trunk.

comment:9 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.