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

compose 1.6.8, compose compiler 1.5.4, kotlin 1.9.20 #1216

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rjrjr
Copy link
Contributor

@rjrjr rjrjr commented Jul 31, 2024

No description provided.

When `DialogCollator` has to reorder, it did so by calling `Dialog.dismiss()` and `Dialog.show()`. #1213 fixed a problem that caused for `ComposeView`, but that fix is undone by Compose 1.6.8. Ngoc Vu dug in to see why and discovered:

> The `onClick` in `Modifier.clickable` is passed all the way down to `pointerInputHandler` in `SuspendingPointerInputModifierNodeImpl` (when the screen is responsive, you should see `pointerInputHandler()` being called in `SuspendingPointerInputModifierNodeImpl#onPointerEvent`).
>
> After shuffling the overlays, even though the `ClickableNode` is found and `SuspendingPointerInputModifierNodeImpl#onPointerEvent` is called, `pointerInputHandler()` is never called  because the `pointerInputJob` in `SuspendingPointerInputModifierNodeImpl` was launched from a canceled coroutineScope. Tracking down that coroutineScope shows that it was canceled when `DialogCollator` calls `Dialog.dismiss` -> `onViewDetachedFromWindow`  -> `Recomposer.cancel()`. So it makes sense why destroying/rebuilding instead of dismissing and reusing the dialog fixes this issue.

Still need to put together a non-workflow repro sample and submit a bug to the Compose team, but this unblocks our move to Compose 1.6.8 in the meantime.
@rjrjr rjrjr force-pushed the ray/dialogamageddon branch 2 times, most recently from 497496e to fae88ea Compare July 31, 2024 23:04
Base automatically changed from ray/dialogamageddon to main July 31, 2024 23:23
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.

1 participant