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 compose view detach #299

Merged
merged 5 commits into from
May 25, 2021
Merged

Conversation

zsoltk
Copy link
Contributor

@zsoltk zsoltk commented May 7, 2021

Fixes #289

@zsoltk zsoltk added the bug Something isn't working label May 7, 2021
@zsoltk zsoltk requested a review from antonshilov May 7, 2021 10:55
// If there was already another child attached, then this one in
// MutableState was overwritten, and already removed from the composition as a result,
// so no further action is needed.
if (child == lastChildAttached) {
Copy link
Contributor

Choose a reason for hiding this comment

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

With the current implementation we have an important difference between AndroidView and ComposeView attach logic.
For AndroidView we add the child to the view group. So other children are still there when new one is attached.
But for ComposeView we are replacing the previous child in target:mutableState. So the default configuration essentially support only one child to be shown. Which is actually the reason of the bug we're fixing in this PR.

Is this difference in attach behavior intended? Users may expect ComposeView to behave like AndroidView, maybe we should make them the same, or document the differences.

@zsoltk zsoltk merged commit a942de6 into badoo:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compose views + back stack don't work together as expected
2 participants