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.

Sign up for GitHub

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

Jump to bottom

Don't close related modal if error exists #257

Open
ArkieCoder opened this issue Feb 14, 2023 · 6 comments
Open

Don't close related modal if error exists #257

ArkieCoder opened this issue Feb 14, 2023 · 6 comments
Assignees
@fabiocaccamo
Labels
enhancement New feature or request

Comments

@ArkieCoder
Copy link

ArkieCoder commented Feb 14, 2023 •
edited by polar-sh bot
Loading

Hello,

I am overriding save_model in a ModelAdmin class that is related to another class. In my save_model I am detecting if there were errors in the save process, which can happen if a user provides a malformed file to one of the fields. If there is an error, I use django.contrib.messages to display the error to the user.

The problem with using the related modal for this in my app is that the modal closes and the error message does not display. Is there a way to either 1) prevent the modal from closing if there is a message to display or 2) to force the parent page to reload if there are messages in the queue to display?

Thanks - this is a great module that radically improved the UX of my app with just a few small changes. Very much appreciated.

Fund with Polar
@ArkieCoder ArkieCoder added the enhancement New feature or request label Feb 14, 2023
@fabiocaccamo
Copy link
Owner

@ArkieCoder nice catch... I never faced it personally.

I think the best thing to do here would be display the errors in the modal without closing it automatically.

@ArkieCoder
Copy link
Author

Agreed. Does django-admin-interface support not closing the modal in the event of error messages? Or is this something that will require changes to be made to the module? I would be happy to work on a solution and submit a PR if needed.

@fabiocaccamo
Copy link
Owner

@ArkieCoder the modal closing is intercepted here:
https://github.com/fabiocaccamo/django-admin-interface/blob/main/admin_interface/static/admin/js/popup_response.js#L22

@ArkieCoder
Copy link
Author

I took an extended look at this issue. Unfortunately simply blocking the close in Javascript brings no joy, because the code still redirects to popup_response.html which just displays a spinner. I don't mind the spinner, per se, but would like the error message to display over the top of it. I think by typing this out here I may have formulated a plan. :) We will see.

@ArkieCoder
Copy link
Author

Ok I have a solution - I added a popup_response_template to my admin class:

popup_response_template = "music_library/popup_response.html"

popup_response.html:

<html>
  <head><title>{% trans 'Popup closing...' %}</title></head>
  <body>
    {% for message in messages %}
      {% if message.level_tag != 'success' %}
        {% include "admin/base_site.html" %}
          <input
            onClick="parent.dismissRelatedObjectModal(); return false;"
            type="submit"
            style="position: absolute; top: 162px; right: 5px; padding: 0; height: 30px; width: 95px; background-color: 
#f00;"
            value="Cancel">
      {% else %}
        {% include "music_library/popup_success_js.html" %}
      {% endif %}
    {% endfor %}
  </body>
</html>

popup_success_js.html:

{% load static %}<script type="text/javascript"
            id="django-admin-popup-response-constants"
            src="{% static "admin/js/popup_response.js" %}"
            {% if popup_response_data %}
            data-popup-response="{{ popup_response_data }}"
            {% else %}
            data-popup-response='{"action":"{{ action|escape }}","value":"{{ value|escape }}","obj":"{{ obj|escape }}","
new_value":"{{ new_value|escape }}"}'
            {% endif %}>
    </script>
    <script type="text/javascript">
      window.dismissRelatedObjectModal();
    </script>

This provides the desired behavior - normal behavior on success, and display of the error message when there is one. Perhaps this can be of help to others.

@fabiocaccamo
Copy link
Owner

fabiocaccamo commented Feb 18, 2023 •
edited
Loading

@ArkieCoder thank you for sharing your solution!

Maybe this should be included in the package, I keep the issue open.

@fabiocaccamo fabiocaccamo reopened this Feb 18, 2023
@fabiocaccamo fabiocaccamo moved this to In Progress in Open Source Feb 21, 2023
@fabiocaccamo fabiocaccamo moved this from In Progress to Todo in Open Source Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fabiocaccamo fabiocaccamo

Labels
enhancement New feature or request
Projects
Open Source
Status: Todo
Milestone
No milestone
Development

No branches or pull requests
2 participants
@fabiocaccamo @ArkieCoder