-
Notifications
You must be signed in to change notification settings - Fork 439
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
Fix bug where Idiomorph sometimes ignores data-turbo-permanent #1321
base: main
Are you sure you want to change the base?
Fix bug where Idiomorph sometimes ignores data-turbo-permanent #1321
Conversation
a363174
to
49f4504
Compare
b0dbea1
to
78f6e45
Compare
I see bigskysoftware/idiomorph#72 was finally merged. Would incorporating the use of |
Hi @brunoprietog, yes, but not quite yet! There's one outstanding bug in v0.4.0 that breaks Turbo data-turbo-permanent: bigskysoftware/idiomorph#83 . Once that gets merged and released, upgrading Idiomorph will become possible for a much-improved morphing experience in Turbo. I'm hoping we'll get a v0.4.1 or a v0.5.0 out the door very soon. Just gotta wait for @1cg to carve out the time to review and release. |
I am going to review the PR tomorrow and will update this issue |
Thanks! |
9bce6eb
to
794de7c
Compare
@brunoprietog After months of work, we've just released Idiomorph v0.7.1 which is a big leap forward in many many things, including fixing this motivating bug. Morphing should be much, much more correct now across the board. So I've force-pushed this PR with three commits:
Lastly, note that there is one additional feature in v0.7.1 that may be of interest here. Active focus/selection restoration after morph. I know Turbo already does this for non-morphing renders, but now the morphing render does it too via Idiomorph. If you want to evaluate that as a separate addition, it can be disabled by passing Finally, Carson has designated me as the lead of Idiomorph, so moving forward, I'll be happy to work with Turbo on any morphing-related issues or desires. P.S. CI is red right now on |
9bce6eb
to
a7b9250
Compare
And, we're green! Ready for review. |
The new version looks fantastic. Let us test it a bit with our apps and I'll merge. Thank you so much for the amazing work. |
Hi folks, this is a redux of #1308 with an alternative strategy: fixing the issue in Idiomorph.
Motivating use-case
I'm building a collaborative issue tracking app, and one of the issue tracker app's primary use-cases has exposed a bug in the integration between Idiomorph and the
data-turbo-permanent
attribute. Each ticket contains adata-turbo-permanent
checkbox to keep track of the client-side state of whether or not the ticket is currently expanded for that client or not. We also allow tickets to be reordered via drag-and-drop. The issue is that thisdata-turbo-permanent
checkbox is not always preserved across morphs involving ticket reorders. There are other more serious ramifications of this bug (data loss!), but this is the simplest case to illustrate the issue.Diagnosis and resolution
I have reduced this scenario down to the failing test case in this PR, and identified the issue as being within Idiomorph, which I've opened a PR for here: bigskysoftware/idiomorph#61 . However, much smaller "no-brainer" PRs from other Turbo contributors have gone ignored for months, so I'm not optimistic at its odds of a timely merge and release. Perhaps Turbo should maintain its own Idiomorph branch? At the moment, this PR is pointing Idiomorph to my own botandrose Idiomorph branch, which I'm sure is not ideal.
Thoughts?