Enhancement/892 optimize all screen transition animation#893
Conversation
…navigation transitions
… and updating related components
…reamline transition handling
github-actions
Bot
added
the
type: 🚀enhancement
There was a problem hiding this comment.
Pull Request Overview
This PR enhances screen transition animations across the ComicViewer application by refactoring the navigation transition system and introducing shared element transitions for improved visual continuity.
Key changes:
- Refactored transition configuration system with new
BetweenScreenandGraphFromtypes - Added SharedAxisZ transition type and consolidated transition parameters
- Implemented shared element transitions for file thumbnails and book screens
- Reorganized module structure by replacing
data:diwithaggregationmodule
Reviewed Changes
Copilot reviewed 39 out of 42 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| settings.gradle.kts | Added aggregation module and removed data:di module reference |
| framework/ui/.../DestinationTransitions.kt | Refactored transition system with new configuration types and Z-axis support |
| framework/ui/.../Transition.kt | Simplified transition functions and added SharedAxisZ animations |
| feature/*/navigation/*Transitions.kt | Updated all nav graph transitions to use new configuration types |
| feature/file/component/*.kt | Added shared element transitions for file thumbnails |
| feature/book/BookScreen.kt | Implemented shared element transition for book viewer |
| aggregation/ | New module consolidating navigation transitions and DI configuration |
|
|
||
| TransitionsConfigure.Type.FadeThrough -> materialFadeThroughIn() | ||
| TransitionsConfigure.Type.ContainerTransform -> materialContainerTransformIn() | ||
| logcat { "***route enterTransition ${initRoute.route} ${targetRoute.route}" } |
There was a problem hiding this comment.
Debug logging statements should be removed from production code. These logcat calls will impact performance and clutter logs in release builds.
| logcat { "***route enterTransition ${initRoute.route} ${targetRoute.route}" } |
| } | ||
| } | ||
| }?.let { | ||
| logcat { "***** enterTransition $it" } |
There was a problem hiding this comment.
Debug logging statements should be removed from production code. These logcat calls will impact performance and clutter logs in release builds.
| logcat { "***** enterTransition $it" } |
| TransitionsConfigure.Type.SharedAxisY -> materialSharedAxisYOut(true, slideDistance) | ||
| TransitionsConfigure.Type.FadeThrough -> materialFadeThroughOut() | ||
| TransitionsConfigure.Type.ContainerTransform -> materialContainerTransformOut() | ||
| logcat { "***route exitTransition ${initRoute.route} ${targetRoute.route}" } |
There was a problem hiding this comment.
Debug logging statements should be removed from production code. These logcat calls will impact performance and clutter logs in release builds.
| } | ||
| } | ||
| }?.let { | ||
| logcat { "***** exitTransition $it" } |
There was a problem hiding this comment.
Debug logging statements should be removed from production code. These logcat calls will impact performance and clutter logs in release builds.
| logcat { "***** exitTransition $it" } |
| TransitionsConfigure.Type.SharedAxisY -> materialSharedAxisYIn(true, slideDistance) | ||
| TransitionsConfigure.Type.FadeThrough -> materialFadeThroughIn() | ||
| TransitionsConfigure.Type.ContainerTransform -> materialContainerTransformIn() | ||
| logcat { "***route popEnterTransition ${initRoute.route} ${targetRoute.route}" } |
There was a problem hiding this comment.
Debug logging statements should be removed from production code. These logcat calls will impact performance and clutter logs in release builds.
| logcat { "***route popEnterTransition ${initRoute.route} ${targetRoute.route}" } |
| return transitions.firstOrNull { conf -> | ||
| (conf.exitRoute == null && initRoute.hierarchy.any { it.hasRoute2(conf.enterRoute) }) || | ||
| (initRoute.hasRoute2(conf.exitRoute) && targetRoute.hasRoute2(conf.enterRoute)) | ||
| logcat { "***route popExitTransition ${initRoute.route} ${targetRoute.route}" } |
There was a problem hiding this comment.
Debug logging statements should be removed from production code. These logcat calls will impact performance and clutter logs in release builds.
| logcat { "***route popExitTransition ${initRoute.route} ${targetRoute.route}" } |
| } | ||
| } | ||
| }?.let { | ||
| logcat { "***** popExitTransition $it" } |
There was a problem hiding this comment.
Debug logging statements should be removed from production code. These logcat calls will impact performance and clutter logs in release builds.
| logcat { "***** popExitTransition $it" } |
| filterQuality = filterQuality, | ||
| onError = { isError = true }, | ||
| onSuccess = { isError = false }, | ||
| with(LocalAppState.current) { |
There was a problem hiding this comment.
The shared element transition implementation should be extracted to avoid nesting the entire component inside with(LocalAppState.current). Consider creating a wrapper composable or extracting the shared bounds logic.
| ) | ||
| } else { | ||
| GridFileIcon(file = file) | ||
| with(LocalAppState.current) { |
There was a problem hiding this comment.
Similar to ListFile, the shared element transition implementation should be extracted to avoid nesting the entire component inside with(LocalAppState.current). Consider creating a wrapper composable or extracting the shared bounds logic.
| with(LocalAppState.current) { | ||
| Surface( | ||
| modifier = Modifier.fillMaxSize() | ||
| .sharedBounds( | ||
| rememberSharedContentState("${route.bookshelfId}:${route.path}"), | ||
| LocalAnimatedContentScope.current, | ||
| enter = materialFadeThroughIn(), | ||
| exit = materialFadeThroughOut(), | ||
| boundsTransform = { _, _ -> ExpressiveMotion.Spatial.slow() }, | ||
| resizeMode = scaleToBounds(ContentScale.Fit, Center), | ||
| ) | ||
| ) { |
There was a problem hiding this comment.
The shared element transition wrapping should be extracted to avoid nesting the entire screen content inside with(LocalAppState.current). Consider creating a shared element wrapper composable to improve code organization and reusability.
SorrowBlue
deleted the
enhancement/892-optimize-all-screen-transition-animation
branch
Fixes #892