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

custom js for action form also handles grappelli #719

Merged

Conversation

frnhr
Copy link
Contributor

@frnhr frnhr commented Dec 29, 2017

ExportActionModelAdmin works fine with Django-Grappelli except for one little detail: on valila admin, there is a little jQuery snippet that toggles visibility of the added file_format field. Because Grappelli uses different CSS classes and IDs, this toggling does not occur.

This in vanila admin:
select_experiment_to_change___django_site_admin

... looks like this in grapppelli:
screenshot_29_12_2017__04_24

... but in the default state looks like this:
screenshot_29_12_2017__04_24

Changes in this PR enable grappelli to benefit from the existing feature in the same way that vanila django-admin does, by hiding the second dropdown in grappelli until the "export" action is selected. Original functionality in vanila admin is not affected.


Contributing guidelines... yes I have read them. Cannot comply to most of the requirements there because they assume changes to Python code. Please let me know if I should comply to some. Happy to. Also happy if this gets merged in as is :)

In order to be merged into django-import-export, contributions must have the following:

  • A solid patch that:
    • is clear.
    • works across all supported versions of Python/Django.
    • follows the existing style of the code base (mostly PEP-8).
    • comments included as needed to explain why the code functions as it does
  • A test case that demonstrates the previous flaw that now passes with the included patch.
  • If it adds/changes a public API, it must also include documentation for those changes.
  • Must be appropriately licensed (see Philosophy).
  • Adds yourself to the AUTHORS file.
    If your contribution lacks any of these things, they will have to be added by a core contributor before being merged into django-import-export proper, which may take substantial time for the all-volunteer team to get to.

@coveralls
Copy link

coveralls commented Dec 29, 2017

Coverage Status

Coverage remained the same at 91.276% when pulling 70f236f on frnhr:action_form_on_grappelli into d0157e2 on django-import-export:master.

@bmihelac
Copy link
Member

bmihelac commented Jan 5, 2018

Hi @frnhr and thanks for your contribution.

Even we do not support various admin themes, I see this as usable and unobtrusive change. I'll merge unless there are objections from other maintainers.

@bmihelac bmihelac self-assigned this Jan 5, 2018
@manelclos
Copy link
Contributor

Haven't tested but LGTM

@bmihelac bmihelac merged commit 5940bf5 into django-import-export:master Mar 1, 2018
@bmihelac
Copy link
Member

bmihelac commented Mar 1, 2018

merged, thanks!

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.

None yet

4 participants