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

Improve integration with Django Debug Toolbar #9213

Merged
merged 1 commit into from Mar 7, 2024

Conversation

realsuayip
Copy link
Contributor

Description

Currently, DRF does not work well with debug_toolbar, due to the way it handles non-GET and non-POST requests.

To perform PUT, PATCH, DELETE and OPTIONS requests in browsable API, we send an ajax request to fetch the resulting HTML. The contents are then dynamically rendered using JavaScript.

During this time, the entire toolbar gets re-rendered (with proper data) but since the related JavaScript code is not executed (normally done on document load), the toolbar is instead hidden.

I added a simple check during JavaScript re-render to load the toolbar. It's unlikely this issue could be solved on debug_toolbar side given the way DRF handles content rendering.

IMHO this should be a great QOL improvement given how annoying it becomes to track requests with mentioned methods.

Related:

jazzband/django-debug-toolbar#1743

@math-a3k math-a3k mentioned this pull request Jan 24, 2024
@auvipy auvipy self-requested a review March 6, 2024 10:48
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

would you mind sharing some screen shots please? also pulling from main branch?

@realsuayip
Copy link
Contributor Author

@auvipy, Applied changes from main.

While preparing for screen capture, I realized that this was not working on Chrome. I had to change it like this:

@@ -7,7 +7,7 @@ function replaceDocument(docString) {
   if (window.djdt) {
     // If Django Debug Toolbar is available, reinitialize it so that
     // it can show updated panels from new `docString`.
-    document.addEventListener("DOMContentLoaded", djdt.init);
+    window.addEventListener("load", djdt.init);
   }
 }

For some reason, Chrome does not fire DOMContentLoaded event after document.write() but Safari and Firefox does. After some tinkering, I decided this was the most reliable method. I tested on Safari, Firefox, Chrome and Opera.

The following screen capture uses Chrome:

output.mp4

@auvipy auvipy merged commit a2eabfc into encode:master Mar 7, 2024
9 checks passed
@auvipy auvipy added this to the 3.15 milestone Mar 7, 2024
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

2 participants