bigskysoftware / idiomorph

A DOM-merging algorithm
BSD 2-Clause "Simplified" License
641 stars 32 forks source link

More sophisticated ignoreActive, or ability to intercept. #5

Closed rskuipers closed 7 months ago

rskuipers commented 1 year ago

Context

I'm using idiomorph together with Hotwire Turbo to morph the DOM instead of replacing the DOM. Primarily to be able to use ignoreActive so that when a user is typing in an <input type="text"> element, the user won't lose focus/its cursor while the dom is being morphed.

I've tried keeping the input element out of the DOM that's being morphed/replaced, but that's not an option, so ignoreActive is great.

Problem

ignoreActive also applies when clicking a link or button, which creates interesting scenarios. In my particular situation my pagination gets all fucky. See screenshot below with the 7 button showing up twice because it kept the active element.

image

Proposed solution

Allow stopping a morph through the beforeNodeMorphed callback. So we can do our own check for active element and add a oldNode.nodeName === 'INPUT' condition for example.

Or introduce a new callback specifically for this.

During development I've simply made this change so I can continue developing:

                if (ctx.ignoreActive && oldNode === document.activeElement && oldNode.nodeName === 'INPUT') {
                    // don't morph focused element

Or perhaps I'm silly and it's already possible in the existing code, then please do tell me how I can achieve this.

I'm not a javascript dev but happy to PR the above proposal if you agree.

Thank you for HTMX and Idiomorph, big fan of the library and movement :)

rskuipers commented 1 year ago

I also have another use-case, I want it to ignore a particular node, in my case the <turbo-frame> node. But I do need it to morph the children of this node. Just not touch any of its attributes.

So my hotfix for this was:

if (oldNode.nodeName !== 'TURBO-FRAME') {
    syncNodeFrom(newContent, oldNode);
}
morphChildren(newContent, oldNode, ctx);

This could also be turned into a callback possibility, again I'd be happy to PR this if you agree.

1cg commented 1 year ago

hi @rskuipers would this PR address your needs: https://github.com/bigskysoftware/idiomorph/pull/6

rskuipers commented 1 year ago

It fixes the activeElement issue, but I'm not sure if I can get my second scenario to work with those callbacks. I'd be able to stop <turbo-frame> itself from being processed but I can't reach in and call the morphChildren() function to continue morphing what's inside the <turbo-frame> node.

My proposal would be to add a callback to syncNodeFrom() as well, to stop attributes from being synced. But I don't know if this is too specific of a usecase or if there's a significant performance hit.

owickstrom commented 1 year ago

I'll just add that what @rskuipers is suggesting seems like it would help me in #7 as well. I'd like to avoid morphing particular element/attributes pairs (e.g. details and the open attribute).

owickstrom commented 1 year ago

Here's a commit that solves my problem, and possibly also @rskuipers's: https://github.com/squidlerio/idiomorph/commit/0629ddd44ae94776e96d1c222ef89947a947b6bd#diff-f270a015ef1a07532bccf37cf54015dc1c9e9fb71da35a480843562956b02fbe

I can make into a PR if you want.

pbowyer commented 8 months ago

@owickstrom Yes, please make a PR!

1cg commented 7 months ago

@owickstrom I'm concerned about the perf implications of an attribute lookup on every attribute morph.

What do you think about a generalized callback that, if present, is called

Then you can implement that sort of logic for your use case, but people don't have to pay for it in the general case?

1cg commented 7 months ago

I have a proof of concept here:

https://github.com/bigskysoftware/idiomorph/commit/abf6c948ed60098b5471d474b5fd0c5b254b258b

with minimal perf implications. Can you all please take a look at it and let me know if it is flexible enough for what you want?

owickstrom commented 7 months ago

@1cg callback approach makes sense, as you say. How would this integrate with hx-swap? I mean, how would one set up the callback in that case?

1cg commented 7 months ago

I am wondering about this too. I can extend the syntax in the swap attribute to access it:

  hx-swap="morph:{ignoreActive:true, ... calbacks : { ... }"

i also wonder if we should suppport an Idiomorph.config so you can set global defaults for the library.

In your case it sounds like you want to set defaults for all morph swaps, is that correct?

owickstrom commented 7 months ago

@1cg yeah, it would be application-wide as I can see it now, for my use case. It could get messy if you would need contextual rules, but I suppose you could implement them in the callback in that case (ignoring the pain of central coupling).

1cg commented 7 months ago

have a change in dev that allows globally setting the morph configuration:

https://github.com/bigskysoftware/idiomorph/commit/f9833458b3c6566f46cd1c8af2c4fe7635eabfde

working on a change that will allow the htmx extension to override settings as well

owickstrom commented 7 months ago

Looks good. So we just need the beforeAttributeUpdated callback in there, as in your first POC?

1cg commented 7 months ago

Supported in latest version of idiomorph (v0.2.0) in the beforeAttributeUpdated callback.