search

django-polymorphic / django-polymorphic-tree

Polymorphic MPTT tree support for models
Other
170 stars 45 forks source link

Change grp-container from ID to class #48

Closed jpotterm closed 8 years ago

jpotterm commented 8 years ago

Grappelli has recently changed grp-container from an ID to a class: https://github.com/sehmaschine/django-grappelli/commit/9eaca21706d5ad900a215218d9708a85c550df62

So this brings django-polymorphic-tree in line with that change.

vdboor commented 8 years ago

Good to know, thanks for adding this!

vdboor commented 8 years ago

@jpotterm can you tell me which release this happens in? I notice the current master doesn't show this:

https://github.com/sehmaschine/django-grappelli/search?utf8=%E2%9C%93&q=grp-container

jpotterm commented 8 years ago

True, it's in the stable/2.8.x branch: https://github.com/sehmaschine/django-grappelli/blob/stable/2.8.x/grappelli/templates/admin/base.html

vdboor commented 8 years ago

Ah, ok! From what I see, the 2.8.1 release solves it differently: https://github.com/sehmaschine/django-grappelli/blob/2.8.1/grappelli/templates/admin/base.html - it gives the element 2 ID's. I've reverted the commit for now, as the current release would be incompatible otherwise. however, it it pops up with a different release, please let me know!

jpotterm commented 8 years ago

@vdboor Yes 2.8.1 did it that way, but that actually breaks the functionality entirely because elements are only allowed to have one ID (see https://github.com/sehmaschine/django-grappelli/issues/741). So it's impossible to keep compatibility with 2.8.1 since the #grp-container selector won't match at all.

So I figure we might as well adopt the new functionality now, since both ways (#grp-container and .grp-container) are broken in 2.8.1 but the new way will work in the next release.

vdboor commented 8 years ago

I see. We might be able to use #container now, as the file is only loaded with grappelli installed - that should work regardless of the grappelli version,

jpotterm commented 8 years ago

That's true, that would probably work. Either way, I don't think it's a huge deal to miss a single grappelli release especially since it's grappelli's fault that they broke compatibility in a bugfix release. But yeah, whatever you want to do.