Accessibility: Fix talkback after EoY modal bottom sheet is hidden #630
Conversation
ashiagr
changed the title
| LaunchedEffect(Unit) { | ||
| snapshotFlow { sheetState.currentValue } | ||
| .collect { | ||
| if (sheetState.currentValue == ModalBottomSheetValue.Expanded) { | ||
| onExpanded.invoke() | ||
| } else if (sheetState.currentValue == ModalBottomSheetValue.Hidden) { | ||
| if (isSheetShown) { | ||
| /* Remove bottom sheet from parent view when bottom sheet is hidden | ||
| on dismiss or back action for talkback to function properly. */ | ||
| parent.removeAllViews() | ||
| } else { | ||
| if (!sheetState.isVisible && shouldShow) { | ||
| /* Show bottom sheet when it is hidden on initial set up */ | ||
| displayBottomSheet(coroutineScope, sheetState) | ||
| isSheetShown = true | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Why do we need to create a snapshot flow instead of just passing sheetState.currentValue as the argument to the LaunchedEffect instead of unit?
LaunchedEffect(sheetState.currentValue) {
if (sheetState.currentValue == ModalBottomSheetValue.Expanded) {
// ...
}
}What you have here works fine, so I don't think this needs to change. I'm just asking because I suspect I would have used the LaunchedEffect-with-a-key approach, and I'm curious if maybe there's a gotcha I'm overlooking.
There was a problem hiding this comment.
snapshotFlow allowed me to observe bottom sheet visibility changes and react to show/hide actions without having to cancel and re-launch LaunchEffect. Your suggestion works as well, and tbh I don't have a clear answer at this point about which one has a clear advantage.
There was a problem hiding this comment.
Just came across this discussion, I haven't tested it though:
LaunchedEffect(someMutableState.value) will cause a custom Layout() in the same @composable function to remeasure every time the value of the key changes
generally snapshotFlow is more efficient for observing state in coroutines because it lets you avoid recompositions.
There was a problem hiding this comment.
That is interesting. Sounds like the gist of it is that reading a value in the key field of a LaunchedEffect does make that composable scope depend on that value, and it will recompose that entire scope anytime the key changes, even if the LaunchedEffect does not generate any UI.
Thanks for pointing that out. I still have so much to learn with compose!
Description
This PR attempts to fix talkback issues after End of Year launch modal bottom sheet is shown.
From the issue description:
It felt strange that we could navigate through talkback by swiping on the x-axis, which means after the modal was dismissed, talkback focus was moved from the bottom sheet to the screen elements. I wonder if this behavior was due to one of the compose modal bottom sheet issues (1, 2).
As a workaround, I remove EoY compose modal bottom sheet once it is hidden. And re-add it when it is needed again with minimum changes to the existing logic.
Fixes #621
Testing Instructions
Screenshots or Screencast
talkback.mp4