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

enable hx-preserve handing for oob swaps #2934

Merged
merged 4 commits into from
Oct 3, 2024

Conversation

MichaelWest22
Copy link
Contributor

@MichaelWest22 MichaelWest22 commented Sep 27, 2024

TL;DR

As found in issue #2922, htmx does not process hx-preserve attributes properly when supplied inside a oob-swap response. Instead the preserved content is just overwritten with the new content from the oob response.
So I've found that simply wrapping the oobSwap() function with handle and restore preserved elements we can indeed support this with just these two lines.

Description

I also tried with simply moving the handlePreservedElements() funciton up before the oob swap handling code and this in fact does mostly work but it has some bad side effects as when it preserves the element into the fragment it removes the hx-swap-oob tags if done wrong and it can bring oob-swap tags from the page into the fragment which can then act as oob swaps in error. So to avoid this issue I moved the change to just wrap inside the oobSwap() function and it behaves much better.

What i've found is that with this change their is no way to support hx-preserve on the same element as the hx-swap-oob like:

<div id="foo" hx-swap-oob="true" hx-preserve>dummy data</div>

This just can't work for several reasons. I have failed to find a valid reason to even do this as you are asking it to swap foo back into foo with no changes so it is just a null update at best. Some of the other swap styles with additional targeting might do something meaningful but it not only breaks my brain trying to work out how it would work but it also breaks the code if you try and make it work. But the good news is that the code by default uses a findAll() to run a querySelectorAll() to find all children that match the hx-preserve attribute and this ignores the attribute on the root element of the oob swap. So this is a reasonable safe default to have where it prevents you doing the wrong kind of use case and just treats it as a hx-swap-oob with no preserve if you try this. Could add code to report an error and ignore the swap if needed but I think this is better fixed with correct documentation.

I also Found some interesting emergent behavior with how hx-preserve works. It base use case is to keep some important client side content from changing while the whole page gets updated and re-arranged around it and this does allow when doing full page reloads like boost's to re-parent the preserved elements to a new page location. And with standard htmx partial page updates if you include a hx-preserve item in your partial response it can in fact move the preserved item from another location on the page into the new target location allowing you to for example drag a video iframe to a different page location etc. And with oob swaps you also have this same new re-parenting super power:

Main page content<div id="topleft" hx-swap-oob="beforeend"><div id="video" hx-preserve></div></div>

With this simple response we can relocate video element to the end of the topleft div from anywhere on the page without it changing.

<p>Item 6 moved above item 3</p><div id="list-item-3" hx-swap-oob="beforebegin"><div id="list-item-6" hx-preserve></div></div>

And here you can move a preserved item up to above item 3 in some list without having to know update or change its contents.

https://michaelwest22.github.io/htmx/index.html here is a quick example of using this oob swaps in practice. it works best with experimental moveBefore enabled.

I think I just need to add some documentation to the hx-preserve.md page to highlight the basic behaviour when using hx-swap-oob.

Corresponding issue:
#2922

Testing

Added tests for the basic use cases and performed testing on both canary chrome with moveBefore and normal chrome to confirm oob swaps work as expected.

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

@irfinnew
Copy link

Thanks! I was intending to look at this myself at some point, but I'm not much of a Javascript dev, so I'm grateful someone else fixed it.

I tested master+PR in my project and I can confirm it solves my issue.

@Telroshan Telroshan added the bug Something isn't working label Sep 28, 2024
Copy link
Collaborator

@Telroshan Telroshan left a comment

Choose a reason for hiding this comment

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

Simple & effective fix, good catch!

(Note: I took the liberty of isolating a TL;DR part at the top of your PR description, just to maximize its chances of being read through upon review, hope you don't mind!)

@Telroshan Telroshan added the ready for review Issues that are ready to be considered for merging label Sep 28, 2024
@irfinnew
Copy link

irfinnew commented Sep 28, 2024

(this is a reply to a now deleted comment by someone who couldn't get oob-preserve-swapping to work)

You need to have div#will-be-oob-preserved in your fragment response as well, see the hx-preserve docs:

You must set an unchanging id on elements for hx-preserve to work. The response requires an element with the same id, but its type and other attributes are ignored.

Something like this should work. Original HTML:

<div id="will-be-oobed">
    This will change.
    <div id="will-be-oob-preserved" hx-preserve="true">
        This will stay untouched.
    </div>
</div>

OOB replacement:

<div id="will-be-oobed" hx-swap-oob="innerHTML">
    Changed!
    <div id="will-be-oob-preserved" hx-preserve="true">
        You should not see this!
    </div>
</div>

@MichaelWest22
Copy link
Contributor Author

@Telroshan if I wanted to add a few lines to hx-preserve.md documentation to make it clear how use oob swap with hx-preserve and its limitations should this be added to this PR or another separate PR? And if it is another one should it target dev or master as this shouldn't merge until this PR possibly gets merged to master with a full release.

@Telroshan
Copy link
Collaborator

@MichaelWest22 Since the documentation you're mentioning would depend on this PR to be merged first, and as it'd make even more clear what this PR is adding, I'd say it's totally fine & desired to add it to this very PR!

@gilesbradshaw
Copy link

(this is a reply to a now deleted comment by someone who couldn't get oob-preserve-swapping to work)

Thanks for your reply. I deleted my question in the hope no one had seen me make a fool of myself :P

@1cg
Copy link
Contributor

1cg commented Oct 3, 2024

@MichaelWest22 continues to crush

@1cg 1cg merged commit 4a81723 into bigskysoftware:dev Oct 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants