#30459 closed Bug (fixed)
If a StackedInline has fieldsets with the "collapsed" class, the "Show" link doesn't work on inline forms added with the "Add another [inline object]" link
Reported by: | Antonis Christofides | Owned by: | Carlton Gibson |
---|---|---|---|
Component: | contrib.admin | Version: | 2.2 |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | yes |
Description
I attach a screenshot with explanations, which is the easiest way to get a grip of this problem.
Steps to reproduce the problem illustrated in the screenshot:
- Create a
myblog
django project and ablog
app.
- Specify this
models.py
in theblog
app:from django.db import models class BlogPost(models.Model): content = models.TextField() class Author(models.Model): blog_post = models.ForeignKey(BlogPost, on_delete=models.CASCADE) name = models.CharField(max_length=100) birthday = models.DateField(blank=True)
- Specify this
admin.py
:from django.contrib import admin from . import models class AuthorInline(admin.StackedInline): model = models.Author extra = 1 fieldsets = [ ("Essential", {"fields": ("name",), "classes": ("collapse",)}), ("Advanced", {"fields": ("birthday",), "classes": ("collapse",)}), ] @admin.register(models.BlogPost) class BlogPostAdmin(admin.ModelAdmin): inlines = [AuthorInline]
- Add the
blog
app toINSTALLED_APPS
, makemigrations, migrate, createsuperuser, runserver.
- Visit /admin/, login, go to "Blog posts", click on "Add new blog post", then click on "Add new Author".
Result: The "Show" links on the new Author form don't work. See the screenshot for more information.
Attachments (2)
Change History (15)
by , 5 years ago
Attachment: | Selection_011.png added |
---|
comment:1 by , 5 years ago
I could not reproduce this. See this screencast, both the links in my case works properly. Pardon me if did something wrong, or misunderstood something
by , 5 years ago
Attachment: | Screenshot 2019-05-09 at 12.47.34 AM.png added |
---|
comment:2 by , 5 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
OK, yes. Thanks for the report
This a regression in ba83378a7762c51be235b521aa5b48233d6c6c82, which was part of Django v2.2.
(Reproduced at af5ec222ccd24e81f9fec6c34836a4e503e7ccf7)
The change from using jQuery's `on()` did not correctly account for the _delegation_ of event handlers.
Instead of attaching directly to the a.collapse-toggle
elements (which in this case do not exist in the DOM at when the listener is added) we should attach to a parent element and filter events by the event target class (as jQuery does/did for us).
comment:3 by , 5 years ago
Are you sure that's the regression? Looking at the code, jQuery was also directly attaching the handler to the a
link ($("fieldset.collapse a.collapse-toggle").on('click', function(ev) {
).
comment:4 by , 5 years ago
Hey Claude.
I've tested on both 2.1 and 2.2. It works on 2.1, but not 2.2.
I (manually) bisected to the referenced commit, using this history view:
https://github.com/django/django/commits/master/django/contrib/admin/static/admin/js
It works at a5f139236f930df06ae0642507530ca98081e2a9 but fails at the next commit ba83378a7762c51be235b521aa5b48233d6c6c82 — so I pinned it to that. (It's the only change that looks relevant.)
You are right about the on()
call though... 🤔
I was going to write a js_tests
case to reproduce this. I'll then be able to actually git bisect
.
(Either way, I think the need to _delegate_ the event listener is correct.)
comment:6 by , 5 years ago
I think this has something to do with the jQuery clone(true)
method (called from inlines.js
) which is supposed to also copy events from the original DOM element. It may be that it doesn't work with vanilla JS events? To be investigated.
comment:7 by , 5 years ago
Hey Claude.
No, I haven’t got there yet. (Worked out where to put it but not actually written it.)
My plan was to look at it next week (2019 WK20). If you beat me to it, that is fine. 🙂
comment:10 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:11 by , 5 years ago
Needs tests: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Screenshot that illustrates the problem