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

Update re-attached widget removes to use detach #2510

Merged
merged 3 commits into from
Dec 16, 2024

Conversation

JakeWharton
Copy link
Collaborator

@JakeWharton JakeWharton commented Dec 13, 2024

This ensures that the host-side maintains their instances to correctly support re-attach.

Closes #1902

movable.webm

  • CHANGELOG.md's "Unreleased" section has been updated, if applicable.

This ensures that the host-side maintains their instances to correctly support re-attach.
@JakeWharton JakeWharton force-pushed the jw.movable-content.2024-11-15 branch from 1920eef to 3b50c8b Compare December 13, 2024 17:21
Copy link
Member

@colinrtwhite colinrtwhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

}
}

assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful!

)

state = 1
assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ValueChange instances all target the new Column here, right?

state = 1
assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
// Box from Row
ChildrenChange.Remove(Id(1), ChildrenTag(1), 0, detach = true),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YASS

)
}

@Test fun multipleMovableContentButOnlyOneReused() = runTest {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So cool

@squarejesse
Copy link
Contributor

You made it look easy

…protocol/guest/DefaultGuestProtocolAdapter.kt
@JakeWharton
Copy link
Collaborator Author

You made it look easy

Extracting like 8 infrastructure PRs out from before it certainly helped!

@JakeWharton JakeWharton enabled auto-merge (squash) December 16, 2024 22:48
@JakeWharton JakeWharton merged commit 7af60e8 into trunk Dec 16, 2024
11 checks passed
@JakeWharton JakeWharton deleted the jw.movable-content.2024-11-15 branch December 16, 2024 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support movableContentOf across protocol
3 participants