[DRAFT] Add twoPass option for additional state retention
#72
Conversation
| callbacks: mergedConfig.callbacks, | ||
| head: mergedConfig.head | ||
| } | ||
| } | ||
|
|
||
| function createPantry() { | ||
| const pantry = document.createElement("div"); |
There was a problem hiding this comment.
maybe add style='display:none' to hide?
There was a problem hiding this comment.
also maybe throw an id on it? idiomorph-pantry?
There was a problem hiding this comment.
maybe add
style='display:none'to hide?
IDK, el.hidden = true on the next line seems to be a higher level way to express the same thing as style="display: none", and is just as good in terms of browser support https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/hidden#browser_compatibility
I'm not married to it, though. If you prefer the latter that's fine.
There was a problem hiding this comment.
also maybe throw an id on it?
idiomorph-pantry?
I actually had this at first, but ultimately decided to remove it, thinking that it could get weird with id collision if we have more than one instance of Idiomorph running on a page simultaneously, which seems plausible when used in the context of Turbo or htmx. I'm not imagining it buying us much, either, aside from perhaps someone being able to pick it out more easily in the dev tools html inspector while debugging. What do you think?
src/idiomorph.js
Outdated
| ctx.callbacks.afterNodeRemoved(tempNode); | ||
| } | ||
|
|
||
| function moveToPantry(node, ctx) { | ||
| if (!node) return; |
There was a problem hiding this comment.
rather than a recursive algo, do a query selector on [id] to grab all elements that have ids?
might be better for perf
There was a problem hiding this comment.
Agree, except is it possible to do bottom-up in that way? Maybe that's where the idSets can come into the picture?
src/idiomorph.js
Outdated
| Array.from(ctx.pantry.children).forEach(element => { | ||
| const matchElement = root.findElementById(element.id); | ||
| if (matchElement) { | ||
| matchElement.before(element); |
There was a problem hiding this comment.
looks reasonable, want to also have a moveBefore() branch
maybe we empty the children out of the old node when it is stuck in the pantry?
|
Okay, got something working with testing Anyways, I've just pushed with it failing so that the next commit will correct a trivial error, so that we can see it passing, thus demonstrating that it is indeed testing what we think it is testing. |
|
Just pushed a commit that makes |
Add Two-Pass option for retaining additional element state
Superseeds #61. Read that for more context, but the gist of it is that this is a stop-gap solution for retaining more element state within morphs until
moveBeforelands in browsers.This draft PR is just an initial spike, so there are some missing pieces that I'd like to resolve before considering this ready for merge:
morphStyle: innerHTMLmoveBeforesupport is still missingWhat else? Any other thoughts about this direction?