-
Notifications
You must be signed in to change notification settings - Fork 97
Imagen redesign #81
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
Imagen redesign #81
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @lethargicpanda, 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 introduces a comprehensive redesign of the Imagen sample application, focusing on enhancing its user interface and improving code modularity. Key changes include updating to the latest Material3 library, extracting common UI components into a dedicated ui-component
module for reusability across samples, and a complete overhaul of the Imagen sample's screen layout and input handling. The aim is to provide a more modern, consistent, and maintainable user experience while streamlining development for future AI samples.
Highlights
- Material3 Dependency Updates: The Material3 library has been updated to version
1.5.0-alpha01
, and thematerial3-android
dependency has been removed across various samplebuild.gradle.kts
files. This signifies an upgrade to the latest Material Design components and potentially a simplification of dependency management. - UI Component Modularization: Common UI elements like
SeeCodeButton
,SampleDetailTopAppBar
,BackButton
, andTextInput
have been extracted or created within a newui-component
module. This promotes reusability and modularity across different AI samples, leading to a more consistent and maintainable codebase. - Imagen Sample UI Redesign: The
Imagen
sample's UI, specificallyImagenScreen.kt
andGeneratedContent.kt
, has undergone a significant redesign. This includes adopting the newly modularized UI components, updating the layout, and refining how image generation states and user input are handled, resulting in a fresh user experience. - TextInput Component API Update: The
TextInput
composable in theui-component
module has been refactored to utilizeTextFieldState
, a newer API for managing text input. This change improves state management and aligns with modern Jetpack Compose practices.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant and welcome redesign of the Imagen sample, creating a more polished and modern user interface. The extraction of common UI elements into a new :ui-component
module is an excellent architectural improvement that enhances reusability and maintainability across the project. The dependency updates and adoption of newer Compose APIs are also positive steps.
My review focuses on a few key areas to ensure the new components are robust and functional. I've identified a critical issue with the new SampleDetailTopAppBar
where the back button is non-functional, and a high-severity issue regarding improper side-effect handling for showing error messages in the ImagenScreen
. I've also included some medium-severity comments regarding dependency management and code cleanup. Overall, this is a great enhancement, and addressing these points will make it even better.
ai-catalog/ui-component/src/main/java/com/android/ai/uicomponent/SampleDetailTopAppBar.kt
Show resolved
Hide resolved
|
||
Spacer(modifier = Modifier.height(16.dp)) | ||
when (uiState) { | ||
is ImagenUIState.Error -> Toast.makeText(LocalContext.current, uiState.message, Toast.LENGTH_SHORT).show() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Showing a Toast
directly within the body of a when
statement is a side-effect that will be re-executed on every recomposition where the state is Error
. This can lead to the toast being shown multiple times unintentionally.
Side-effects in Compose should be managed within controlled environments like LaunchedEffect
. A better approach would be to hoist the side-effect out of the when
expression and trigger it from a LaunchedEffect
keyed to the error state. This also aligns with the style guide recommendation to handle UI events based on state changes in a controlled manner.1
Example:
val context = LocalContext.current
LaunchedEffect(uiState) {
if (uiState is ImagenUIState.Error) {
Toast.makeText(context, uiState.message, Toast.LENGTH_SHORT).show()
// To make this a one-off event, the ViewModel should then clear
// the error state after it has been handled.
}
}
when(uiState) {
// ... handle other states, but not the error toast
}
Style Guide References
Footnotes
-
The style guide recommends not sending events from the ViewModel to the UI. While this isn't a direct event from the ViewModel, handling side-effects like Toasts improperly in the UI layer goes against the principle of UDF and can lead to unpredictable behavior. The recommended
LaunchedEffect
pattern is a safer way to handle such one-time UI events based on state changes. (link) ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JolandaVerhoef what is the right approach to display a toast on error in a composable here?
ai-catalog/ui-component/src/main/java/com/android/ai/uicomponent/SeeCodeButton.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant redesign for the Imagen sample, extracting common UI elements into a new :ui-component
module. This is a great step towards improving code reusability and maintainability. The dependency updates and UI refactoring align well with modern Android development practices. My review focuses on a few key areas: ensuring dependency stability, correct handling of UI states and side-effects in Jetpack Compose, improving the functionality of reusable components, and general code cleanup. Overall, this is a solid improvement, and addressing the feedback will further enhance the quality of the codebase.
UI changes:
This is an early implementation of the screen. It has the following known issues that will be addressed in following PRs: