-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add config option to ignore nested oob-swaps instead of processing them #1235
Conversation
This looks good—could you add tests that validate the new functionality? |
I added a test that should show the new behavior, but I'm not sure it's correct. |
80285ea
to
2bd14b4
Compare
I updated the test so it should pass if it were to run again. That said I'm not sure if the behavior is exactly what we want. Specifically, should the final innerHTML contain the btn.innerHTML.should.equal('<button>Clicked <span id="when" hx-swap-oob="true">now!</span></button>'); |
What happens for a successful OOB swap? Is the attribute there? It should match that behavior.
Unfortunately it does not pass, but you can run it locally with Also, if you could update the |
bfc9f5e
to
afd7b12
Compare
Ok I looked into this further and this PR seems to be in tension with the The question is: What to do when an HX response body contains a nested element with the
IMO, I wanted to surface this difference and get feedback before I proceed due to the behavior change. Thoughts? |
Very hard to know if anyone is relying on this behavior, though I agree with you that, conceptually, it's a little absurd. |
I will note that the documentation currently warns: “Out of band elements must be in the top level of the response, and not children of the top level elements.” Perhaps adding another property like hx-swap-oob=“true with-depth” or a server response header HX-Swap-OOB-Depth: true? Just some way to either reverse new functionality (or to set new functionality to keep backwards compatibility) |
This has been paused for a while, but I'm currently making this change as a configuration option to switch behaviors. A 2.0 release might consider changing the default for this configuration option. |
Yeah I apologize about this being stalled, it's a tough one for the reasons identified. We're closing in on having many fewer PRs, which will make it easier to focus on the hard choices. I think a config is a good call. |
bfc9f5e
to
a23fe23
Compare
5638a73
to
aa321e3
Compare
With this change, the current behavior is preserved by default, and can be changed by adjusting the config option. Perhaps a later version could change the default and eventually remove the config option. |
The description has been updated to add my motivation for opening this PR. Just off the top of my head here are a couple things that I think would improve it:
Should I continue working on this? What are htmx collaborators' disposition to this change? |
I like the config option and I'm going to push for it. I'll get you back a final up or down soon (< a week). Would probably be against the htmx 2 branch, if that's alright? |
A good reason to support this, which I think you're calling out, is that it composes really well with template fragments. You can send the fragment as an OOB swap, or just send the template normally. |
Go ahead and just summarize my giant meandering paragraphs in two pithy sentences. It's fine. I'm not mad. Why would I be mad? I'm a great writer I'll have you know. 😡😆😭 |
If you weren't a good writer I wouldn't have know what you were talking about Anyway, got the go-ahead! Can you rebase again the 2.0 branch and add the docs? |
Great! I'll get it done. |
|
@infogulch ready for review? |
@alexpetros yessir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all your hard for on this PR, it's really excellent and we're excited to have it as a part of htmx 2 :)
This adds a new configuration option,
allowNestedOobSwaps
, which preserves existing behavior when set to the defaulttrue
, but when set tofalse
it changeshandleOutOfBandSwaps
to only process elements withhx-swap-oob
anddata-hx-swap-oob
that appear at the root of the response fragment, i.e. elements whose parent element isbody
. Elements deeper in the dom tree fragment are ignored and theoob-swap
attribute is stripped instead.Enabling this configuration option is helpful when you want to reuse a small html fragment to render both as as an element within a larger fragment (say a page) as well as more fine grained interactions where the small fragment is updated out of band. By setting
allowNestedOobSwaps: false
, you can placehx-swap-oob="true"
on the fragment and leave it: when a full page renders then the attribute ignored since the fragment is nested inside the main response, but if you do want to update the fragment out of band then render it adjacent to the to the root response element and htmx performs the oob swap as expected. However in this scenario ifallowNestedOobSwaps: true
(default) then the small fragment is removed from the root element first and then the root element is swapped leaving you confused about why the small fragment vanished.Here's an example of how this simplifies htmx usage in Go templates. In particular, note how this eliminates the conditional logic that controls whether
hx-swap-oob="true"
is included in the response or not: infogulch/go-htmx@ea227b6This PR implements the feature request in #1133