Add Animora: interactive Jetpack Compose animation sample #1637
Conversation
Baidaidai-GFWD-origin
requested review from
dturner,
riggaroo and
simona-anomis
as code owners
Summary of ChangesHello @Baidaidai-GFWD-origin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates a new sample application, Animora, into the repository. Animora is a Jetpack Compose project designed to be an educational resource for understanding and experimenting with various animation APIs. It provides interactive demonstrations, particularly for spring animations, and is structured to be clear and accessible for developers looking to learn Compose animations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Animora, a new Jetpack Compose sample project focused on animations. The project is well-structured and provides a great learning resource. I've identified several areas for improvement, including critical issues in the build configuration and networking code, as well as opportunities to enhance code quality, correctness, and maintainability throughout the new codebase. My feedback covers dependency management, CI/CD practices, code style, and architectural patterns.
| animationFiles = { infiniteTransition.animateColor() } | ||
| ), | ||
| AnimationDatas( | ||
| id = 14, | ||
| name = "AnimateFloat", | ||
| shortInfo = R.string.animateFloat_shortInfo, | ||
| details = "markdown/animateFloat.md", | ||
| animationFiles = { infiniteTransition.animateFloat() } |
There was a problem hiding this comment.
The animationFiles lambda for AnimateColor and AnimateFloat does not match the expected (Boolean) -> Unit signature defined in the AnimationDatas data class. This will cause a compilation error. You should update the lambdas to accept the boolean parameter, even if it's unused.
| animationFiles = { infiniteTransition.animateColor() } | |
| ), | |
| AnimationDatas( | |
| id = 14, | |
| name = "AnimateFloat", | |
| shortInfo = R.string.animateFloat_shortInfo, | |
| details = "markdown/animateFloat.md", | |
| animationFiles = { infiniteTransition.animateFloat() } | |
| animationFiles = { _ -> infiniteTransition.animateColor() } | |
| ), | |
| AnimationDatas( | |
| id = 14, | |
| name = "AnimateFloat", | |
| shortInfo = R.string.animateFloat_shortInfo, | |
| details = "markdown/animateFloat.md", | |
| animationFiles = { _ -> infiniteTransition.animateFloat() } |
| suspend fun fetch(url: String,options:HTTPOptions.() -> Unit = {}):String{ | ||
| var responseBody:String | ||
| val httpOptions = HTTPOptions() | ||
| httpOptions.options() | ||
|
|
||
| println(httpOptions.headers) | ||
|
|
||
| val request = Request.Builder() | ||
| .url(url) | ||
| .headers(httpOptions.headers) | ||
| .method( | ||
| method = httpOptions.methods, | ||
| body = httpOptions.body | ||
| ) | ||
| .build() | ||
|
|
||
| clinit.newCall(request) | ||
| .execute() | ||
| .use { | ||
| responseBody = it.body?.string() ?: "Body is Void" | ||
| } | ||
|
|
||
| return responseBody | ||
| } No newline at end of file |
There was a problem hiding this comment.
This fetch function is a suspend function, but it's making a blocking network call using execute(). Blocking calls on coroutines can lead to thread starvation and make your application unresponsive. You should use the asynchronous enqueue() method with a suspendCancellableCoroutine to properly integrate with coroutines.
Additionally, the println statement on line 48 should be removed or replaced with a proper logging framework for production code.
| run: ./gradlew assembleDebug --no-daemon | ||
|
|
||
| - name: Upload Release | ||
| uses: softprops/action-gh-release@v2 | ||
| if: github.ref_type == 'tag' | ||
| with: | ||
| files: app/build/outputs/apk/debug/app-debug.apk |
There was a problem hiding this comment.
The CI/CD workflow is configured to build and upload a debug APK (assembleDebug, app-debug.apk) when a release tag is pushed. For a public release, you should always build and distribute a signed release APK. Debug builds are not optimized, are insecure, and are not suitable for distribution.
run: ./gradlew assembleRelease --no-daemon
- name: Upload Release
uses: softprops/action-gh-release@v2
if: github.ref_type == 'tag'
with:
files: app/build/outputs/apk/release/app-release.apk| <img src="/publicAsset/images/AnimoraSplash.png" width="32%" /> | ||
| <img src="/publicAsset/images/AnimoraHome.png" width="32%" /> | ||
| <img src="/publicAsset/images/AnimoraInfo.png" width="32%" /> | ||
| <img src="/publicAsset/images/AnimoraList.png" width="32%" /> | ||
| <img src="/publicAsset/images/AnimoraBlur.png" width="32%" /> | ||
| <img src="/publicAsset/images/AnimoraStudio.png" width="32%" /> |
There was a problem hiding this comment.
The image paths in the preview section appear to be incorrect. They start with a leading slash (/), which makes them root-relative paths. This will likely result in broken images when viewed on GitHub. To fix this, you should use relative paths from the README.md file's location or absolute URLs to the images in the repository.
| <img src="/publicAsset/images/AnimoraSplash.png" width="32%" /> | |
| <img src="/publicAsset/images/AnimoraHome.png" width="32%" /> | |
| <img src="/publicAsset/images/AnimoraInfo.png" width="32%" /> | |
| <img src="/publicAsset/images/AnimoraList.png" width="32%" /> | |
| <img src="/publicAsset/images/AnimoraBlur.png" width="32%" /> | |
| <img src="/publicAsset/images/AnimoraStudio.png" width="32%" /> | |
| <img src="publicAsset/images/AnimoraSplash.png" width="32%" /> | |
| <img src="publicAsset/images/AnimoraHome.png" width="32%" /> | |
| <img src="publicAsset/images/AnimoraInfo.png" width="32%" /> | |
| <img src="publicAsset/images/AnimoraList.png" width="32%" /> | |
| <img src="publicAsset/images/AnimoraBlur.png" width="32%" /> | |
| <img src="publicAsset/images/AnimoraStudio.png" width="32%" /> |
|
|
||
| buildTypes { | ||
| release { | ||
| isMinifyEnabled = false |
There was a problem hiding this comment.
For release builds, it's highly recommended to enable minification by setting isMinifyEnabled = true. This helps reduce the size of your APK and obfuscates the code, which is a good practice for production releases.
isMinifyEnabled = true| import androidx.compose.runtime.Composable | ||
| import androidx.compose.runtime.CompositionLocalProvider | ||
| import androidx.compose.ui.test.junit4.createComposeRule | ||
| import androidx.compose.ui.test.onNodeWithContentDescription | ||
| import androidx.compose.ui.test.onNodeWithText | ||
| import androidx.compose.ui.test.performClick | ||
| import androidx.lifecycle.viewmodel.compose.viewModel | ||
| import androidx.navigation.compose.rememberNavController | ||
| import androidx.test.platform.app.InstrumentationRegistry | ||
| import androidx.test.ext.junit.runners.AndroidJUnit4 | ||
| import com.baidaidai.animora.components.StartScreen.components.NecessaryComponents | ||
| import com.baidaidai.animora.shared.viewModel.animationDatasViewModel | ||
|
|
||
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
|
|
||
| import org.junit.Assert.* | ||
| import org.junit.Rule |
There was a problem hiding this comment.
There are several unused imports in this file. It's good practice to remove them to keep the code clean and maintainable.
| import androidx.compose.runtime.Composable | |
| import androidx.compose.runtime.CompositionLocalProvider | |
| import androidx.compose.ui.test.junit4.createComposeRule | |
| import androidx.compose.ui.test.onNodeWithContentDescription | |
| import androidx.compose.ui.test.onNodeWithText | |
| import androidx.compose.ui.test.performClick | |
| import androidx.lifecycle.viewmodel.compose.viewModel | |
| import androidx.navigation.compose.rememberNavController | |
| import androidx.test.platform.app.InstrumentationRegistry | |
| import androidx.test.ext.junit.runners.AndroidJUnit4 | |
| import com.baidaidai.animora.components.StartScreen.components.NecessaryComponents | |
| import com.baidaidai.animora.shared.viewModel.animationDatasViewModel | |
| import org.junit.Test | |
| import org.junit.runner.RunWith | |
| import org.junit.Assert.* | |
| import org.junit.Rule | |
| import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi | |
| import androidx.compose.runtime.CompositionLocalProvider | |
| import androidx.compose.ui.test.junit4.createComposeRule | |
| import androidx.compose.ui.test.onNodeWithContentDescription | |
| import androidx.compose.ui.test.onNodeWithText | |
| import androidx.compose.ui.test.performClick | |
| import androidx.lifecycle.viewmodel.compose.viewModel | |
| import androidx.test.ext.junit.runners.AndroidJUnit4 | |
| import com.baidaidai.animora.shared.viewModel.animationDatasViewModel | |
| import org.junit.Test | |
| import org.junit.runner.RunWith | |
| import org.junit.Rule |
| modifier = Modifier | ||
| .padding(contentPaddingValues) | ||
| .scrollable( | ||
| state = rememberScrollableState { 1f }, | ||
| orientation = Orientation.Vertical | ||
| ) |
There was a problem hiding this comment.
The Column has conflicting scroll modifiers. You are using both the low-level .scrollable() modifier and the high-level .verticalScroll() on different parts of the layout. This can lead to unexpected scrolling behavior.
To fix this, you should apply a single .verticalScroll() modifier to the parent Column and remove the redundant .scrollable() modifier to ensure a single, consistent scroll behavior for the entire screen content.
Column(
modifier = Modifier
.padding(contentPaddingValues)
.verticalScroll(rememberScrollState())
.padding(horizontal = 30.dp)
){| */ | ||
| @Composable | ||
| fun AnimateTo(blueState: Boolean){ | ||
| var alphaValue = remember { Animatable(1f) } |
There was a problem hiding this comment.
The alphaValue variable is declared with var but is never reassigned. It's best practice to use val for variables that are not reassigned. This applies to other Animatable instances in this file as well.
| var alphaValue = remember { Animatable(1f) } | |
| val alphaValue = remember { Animatable(1f) } |
| blueState: Boolean, | ||
| springSpecStudioViewModel: springSpecStudioViewModel | ||
| ){ | ||
| var widthValue = remember { Animatable(100f) } |
There was a problem hiding this comment.
The widthValue variable is declared with var but is never reassigned. It should be declared with val for better code clarity and to prevent accidental reassignment.
| var widthValue = remember { Animatable(100f) } | |
| val widthValue = remember { Animatable(100f) } |
| import androidx.annotation.StringRes | ||
| import androidx.compose.runtime.Composable | ||
|
|
||
| final data class AnimationDatas( |
There was a problem hiding this comment.
The final keyword is redundant on a data class in Kotlin, as data classes are final by default. You can safely remove it to simplify the code.
| final data class AnimationDatas( | |
| data class AnimationDatas( |
|
Hello @Baidaidai-GFWD-origin, if you'd like to create new compose-samples, we encourage you to create these as separate repositories on Github and not within the android/compose-samples repository. This way you'll have direct ownership and ability to make updates. |
2 participants
Overview
This PR introduces Animora, a Jetpack Compose sample showcasing animation APIs through small, focused demos.
The sample includes an interactive Spring animation example that allows real-time parameter adjustments to observe animation behavior.
Highlights
Motivation
The goal is to complement existing Compose animation samples by offering an interactive way to explore animation parameters and outcomes.
Scope