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

Crash inside AnimatedVisibility() #710

Closed
fuadenergyom opened this issue May 16, 2024 · 4 comments
Closed

Crash inside AnimatedVisibility() #710

fuadenergyom opened this issue May 16, 2024 · 4 comments

Comments

@fuadenergyom
Copy link

fuadenergyom commented May 16, 2024

How to reproduce

I searched through closed issues and this was addressed in this issue: #200
I have created a CartesianChartHost that works fine inside any layout. However, when put inside an AnimatedVisibility(), it crashes when the visibility becomes true. In the old issue you mentioned it would get fixed in 1.6.5, and I am currently using 2.0.0-alpha.19

How to Reproduce:
Create a CartesianChartHost with CartesianChart with LineCartesianLayer. Use it inside an AnimatedVisibility() block.

Observed behavior

Crashes as soon as visibility becomes true. coerceIn() method causes the crash, because of 0 value of bounds.height().toInt() (in BaseDynamicShader.kt:43).

Expected behavior

Normally render the chart, even with initial height of 0.

Vico version(s)

Latest unstable version

Android version(s)

Android 14 (Virtual Device)

Additional information

@patrickmichalik
Copy link
Member

patrickmichalik commented May 17, 2024

Hello! Thanks for the report. We’ve followed the reproduction instructions and tried several other setups, but unfortunately, we haven’t been able to reproduce the crash. Could you share a minimal reproducible example? Two extra notes:

  • The problem reported in issue Crash when graph get's resized to zero height #200 was resolved in Vico 1.6.5. Something else seems to be happening here.
  • At least by default, AnimatedVisibility appears to clip its content, not resize it. The CartesianChartHost’s height should remain unaffected, and this was the case in our testing.

@fuadenergyom
Copy link
Author

fuadenergyom commented May 17, 2024

Hey Patrick! Thank you very much for taking the time to look at this. I rolled back to yesterday's version of the app and tried to find what was causing the issue, and I figured it out. This is an extremely specific case: my AnimatedVisibility is inside a Column, which is inside a Box, which is inside another Column, and the whole thing is inside the content: @Composable (PaddingValues) -> Unit block of BottomSheetScaffold. When I use the PaddingValues in the parent Column, or the Box that's the only child of it, it would cause this crash. Another thing worth considering is that I measure content's height to determine sheetPeekHeight for BottomSheetScaffold.
I don't know if you think this is worth addressing or not, but here is the minimal reproduction:

BottomSheetScaffold(
...
) { paddingValues ->
Column(
Modifier
.fillMaxSize()
.padding(paddingValues) // The crash doesn't happen if this is removed.
) {
Box(
Modifier
.fillMaxWidth() // We can also put padding(paddingValues) here and it would crash.
.background(Color.White)
.onGloballyPositioned { coordinates ->
val height = with(localDensity) {
coordinates.size.height.toDp()
}
sheetHeight = (screenHeight - height) - 20.dp
}
) {
Column(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 20.dp)
) {
HomeViewSwitch( // This is just to show/hide the chart
modifier = Modifier,
mapViewSelected = mapViewSelected,
onClick = {
screenModel.changeHomeScreenView()
}
)
AnimatedVisibility(
visible = !mapViewSelected,
enter = fadeIn(tween(300)),
exit = fadeOut(tween(300))
) {
LineChartImpl( // This is a wrapper around CartesianChartHost
dataPoints = listOf(10f, 20f, 3f, 4f, 6f)
)
}
}
}
}
}

@Gowsky
Copy link
Member

Gowsky commented May 22, 2024

Hello @fuadenergyom, please provide a self-contained MRE. The code you've shared contains unknown references.

@Gowsky
Copy link
Member

Gowsky commented May 29, 2024

Closing due to no MRE.

@Gowsky Gowsky closed this as not planned Won't fix, can't repro, duplicate, stale May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants