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

Support movableContentOf across protocol #1902

Closed
JakeWharton opened this issue Mar 28, 2024 · 2 comments · Fixed by #2510
Closed

Support movableContentOf across protocol #1902

JakeWharton opened this issue Mar 28, 2024 · 2 comments · Fixed by #2510

Comments

@JakeWharton
Copy link
Collaborator

Currently, due to #1899, we crash when the detached subtree is re-added because children IDs clash. Even once that's fixed, we need to detect the fact that it's a reused node (probably with something like a removed=true boolean on the node) and upon re-insertion, walk its children and add them all to the map.

Once guest-side is fixed, we basically have the same problem host-side. It has the map leak that will cause the crash (#1900), but once that's fixed we need the same tree walk on re-attach.

It is unlikely that we will retain the host-side nodes across recompositions. If you are just moving the content in a single composition, however, we can send a new boolean in the remove op to signal that this is a detach with a future op performing re-attach via insertion. This will keep all nodes in the map, and only perform a re-parenting from the old children to the new children.

@JakeWharton
Copy link
Collaborator Author

JakeWharton commented Nov 15, 2024

Alright here's some observations:

Working on it!

@JakeWharton
Copy link
Collaborator Author

JakeWharton commented Dec 10, 2024

Plan:

  • Change the protocol to eliminate range removal. All removes will be a single element.
  • Defer removing widgets from the map until the change list is realized, probably through a list that keeps track of removed widgets. We remove a widget from the removed widget list when it's reused. This is a rare event, so a quick list scan should be fine.
  • Have protocol remove return an opaque token (read: change list index) which can be held by the widget. If the widget comes back through reuse in the same recomposition, use the token/index to update the original remove with detach=true.

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

Successfully merging a pull request may close this issue.

1 participant