Support rerendering the toolbar on HTMX boosted pages. #1686
Conversation
|
@knyghty can you verify that this resolves the issue on your project? |
docs/installation.rst
Outdated
| .. code-block:: javascript | ||
|
|
||
| if (typeof htmx !== "undefined") { | ||
| htmx.on("htmx:afterSettle", function(detail) { |
There was a problem hiding this comment.
This should check for the existence of djdt. People are likely to paste this in and use it on production.
There was a problem hiding this comment.
I don't know htmx that well but does it ever happen that window.htmx is undefined on such pages?
Maybe const h = window.htmx, d = window.djdt; if (h && d) { ... }? This is all a matter of taste and a very personal opinion (not as a DJDT maintainer!) of course but I prefer window.<var> to the somewhat magic global <var>.
There was a problem hiding this comment.
I can make that change.
There was a problem hiding this comment.
I decided against creating constants and went with window.htmx. I did realize that we can't check window.djdt there since it may not be loaded in yet. So I pushed that check into the event handler, then wrapped it all with {% if debug %} to better signal that this should only be used when the toolbar is installed.
| }; | ||
| window.djdt = { | ||
| show_toolbar: djdt.show_toolbar, | ||
| hide_toolbar: djdt.hide_toolbar, | ||
| init: djdt.init, | ||
| close: djdt.hide_one_level, | ||
| cookie: djdt.cookie, | ||
| get_debug_element: djdt.get_debug_element, |
There was a problem hiding this comment.
I'm a bit sad that we're using a mix of snake_case and camelCase in this file; camelCase seems to be more idiomatic when writing javascript since almost all of the JS runtime itself uses camelCase (especially the DOM) but consistency is more important in the end. Sorry for the borderline bikeshedding.
The more important question is maybe if get_debug_element really has to be exposed here? Can't the method "just" be an internal helper?
There was a problem hiding this comment.
You're right. It can be internal. I'll switch it to camelCase. And then have another commit changing everything else to camelCase so we can stay consistent moving forward.
|
@tim-schilling once I use the snippet from the example it works fine, yes 👍🏻 |
This reworks the JavaScript integration to put most event handlers on document.body. This means we'll have slightly slower performance, but it's easier to handle re-rendering the toolbar when the DOM has been replaced.
This leaves the public API with two snake case functions, show_toolbar and hide_toolbar.
This reworks the JavaScript integration to put most event handlers on document.body. This means we'll have slightly slower performance, but it's easier to handle re-rendering the toolbar when the DOM has been replaced.