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

Fix bug where Idiomorph sometimes ignores data-turbo-permanent #1321

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

botandrose
Copy link
Contributor

@botandrose botandrose commented Sep 26, 2024

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 a data-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 this data-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?

@botandrose botandrose force-pushed the morph-reorder-with-data-permanent-children-redux branch from a363174 to 49f4504 Compare September 26, 2024 06:17
@botandrose botandrose force-pushed the morph-reorder-with-data-permanent-children-redux branch from 49f4504 to b162f48 Compare December 23, 2024 18:46
@botandrose botandrose force-pushed the morph-reorder-with-data-permanent-children-redux branch 3 times, most recently from 0b5715e to b0dbea1 Compare December 23, 2024 20:24
@botandrose botandrose force-pushed the morph-reorder-with-data-permanent-children-redux branch from b0dbea1 to 78f6e45 Compare December 24, 2024 21:46
@brunoprietog
Copy link
Collaborator

I see bigskysoftware/idiomorph#72 was finally merged. Would incorporating the use of twoPass help here?

@botandrose
Copy link
Contributor Author

botandrose commented Jan 8, 2025

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.

@1cg
Copy link

1cg commented Jan 8, 2025

I am going to review the PR tomorrow and will update this issue

@brunoprietog
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants