Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ticket 21353 #1882

Closed
wants to merge 2 commits into from
Closed

Ticket 21353 #1882

wants to merge 2 commits into from

Conversation

nofeet
Copy link

@nofeet nofeet commented Nov 5, 2013

Fix and test for ticket 21353: 'description' key in InlineModelAdmin fieldsets is ignored

https://code.djangoproject.com/ticket/21353

@@ -42,6 +42,10 @@
{% endspaceless %}
</td>
{% for fieldset in inline_admin_form %}
{% if fieldset.description and forloop.parentloop.first %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On looking at this a bit more, I'm not sure about it... fieldset.name isn't rendered either, maybe this was intentionally omitted in the tabular inline. Just stacking up the descriptions where they don't really correspond to the fields doesn't seem very elegant.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timgraham Yeah, I agree it's not very elegant. I had tried several different approaches and using forloop.parentloop.first was the only one that worked.

You make a great point about fieldset.name not being used. Now I'm thinking that fieldsets and fields have a different relationship in TabularInlines than they do in ModelAdmin and even StackedInlines. It would be bad to force more fieldset attributes into it.

I'll give it more thought. Thanks for your feedback.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. It may be best just to document that description doesn't apply when using TabularInlines. I'll close this PR for now. Feel free to continue the discussion on the ticket.

@timgraham timgraham closed this Nov 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants