Conversation
Summary of ChangesHello, 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 represents a significant architectural overhaul, migrating the Jetnews application to the experimental Navigation 3 library. This transition fundamentally redefines how the application handles screen transitions, back stacks, and deep linking, moving towards a more robust and type-safe navigation paradigm. The changes also enhance the app's adaptability across various screen sizes, particularly for list-detail patterns, and introduce a more structured approach to managing individual screen states. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully migrates the Jetnews app to Navigation 3, which is a significant and well-executed undertaking. The changes introduce a type-safe navigation system using NavKey, a custom deep link handling mechanism, and a responsive list-detail layout for large screens using a custom SceneStrategy. The refactoring greatly simplifies the ViewModels by offloading navigation state management to the navigation library. The overall architecture is much cleaner and more robust. I have a few minor suggestions for improvement, mainly related to code style and removing debug logs.
JetNews/app/src/main/java/com/example/jetnews/ui/MainActivity.kt
Outdated
Show resolved
Hide resolved
524c611 to
d905525
Compare
d905525 to
e8d5f4f
Compare
|
|
||
| println(composeTestRule.onRoot().printToString()) | ||
| try { | ||
| composeTestRule.onAllNodes(hasText("3 min read", substring = true))[0].assertExists() |
There was a problem hiding this comment.
This text also shows up in the list screen, so the test didn't actually test that the article screen was opened to the correct article.
| /** | ||
| * Top bar for a Post when displayed next to the Home feed | ||
| */ | ||
| @Composable | ||
| private fun PostTopBar(isFavorite: Boolean, onToggleFavorite: () -> Unit, onSharePost: () -> Unit, modifier: Modifier = Modifier) { | ||
| Surface( | ||
| shape = RoundedCornerShape(8.dp), | ||
| border = BorderStroke(Dp.Hairline, MaterialTheme.colorScheme.onSurface.copy(alpha = .6f)), | ||
| modifier = modifier.padding(end = 16.dp), | ||
| ) { | ||
| Row(Modifier.padding(horizontal = 8.dp)) { | ||
| FavoriteButton(onClick = { /* Functionality not available */ }) | ||
| BookmarkButton(isBookmarked = isFavorite, onClick = onToggleFavorite) | ||
| ShareButton(onClick = onSharePost) | ||
| TextSettingsButton(onClick = { /* Functionality not available */ }) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Moved here from HomeScreens.kt
…e KDocs and README
Key Changes
NavKeyobjects and classes to replace theJetnewsDestinationsobject.ListDetailSceneStrategyto display the post detail view alongside the list of posts if the window width size class is at least expanded.NavigationStateandNavigatorclasses and helpers for storing and updating the app's navigation state.JetnewsNavGraphwithJetnewsNavDisplay.app_opensArticle(nowapp_opensPost) test so that it now actually tests that the post content is displayed.