Enhance/transition #295
Enhance/transition #295
Conversation
WalkthroughSharedTransitionLayout と CompositionLocal（LocalSharedTransitionScope / LocalAnimatedVisibilityScope）を導入し、NavHost 周りでスコープを提供。アイコンに対してスコープ存在時のみ共有要素トランジションを条件適用。Scaffold/TopBar の onBack シグネチャを nullable 化し、Main にネストナビゲーションを導入しました。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as AppScreen (SharedTransitionLayout)
participant Local as CompositionLocalProvider (Shared/Animated scopes)
participant Nav as NavHost / Nested NavController
participant List as AppList / AppListItem
participant Detail as DetailScreen / DetailContent
Note over App,Nav: 起点 — SharedTransitionLayout を生成しスコープを提供
App->>Local: Provide LocalSharedTransitionScope / LocalAnimatedVisibilityScope
App->>Nav: NavHost 起動（Main/Targets/Detail）
Nav->>List: Targets 画面を表示
List->>Local: LocalSharedTransitionScope を参照
alt スコープが存在
List->>List: アイコンに sharedElement 修飾子を適用 (key)
else スコープが null
List->>List: 通常のアイコン Modifier を使用
end
List->>Nav: アイテム選択 -> Detail ルートへ遷移（args: app）
Nav->>Detail: DetailScreen 表示
Detail->>Local: LocalSharedTransitionScope / LocalAnimatedVisibilityScope を参照
alt スコープが存在
Detail->>Detail: アイコンに sharedElement 修飾子を適用（遷移発火）
else
Detail->>Detail: 通常のアイコン Modifier を使用
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 分
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt (1)
131-144: sharedElement 適用の分岐は安全で良い（null ガードあり）／DRY は検討余地
同じ.then(if (scope != null) sharedElement else Modifier)がAppListItemにもあるので、拡張関数化して重複削減すると保守が楽です。AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/util/CompositionLocals.kt (1)
7-8:staticCompositionLocalOf採用を検討（提供値が基本固定なら）
提供するスコープがルート配下で基本的に変化しない前提なら、compositionLocalOfよりstaticCompositionLocalOfの方が意図に合うことが多いです。AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/AppList.kt (1)
75-88: sharedElement の付与条件・キーは妥当／重複コードは共通化推奨
DetailScreen.ktと同一パターンなので、Modifier.appIconSharedElement(...)のように共通化すると変更耐性が上がります。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/AppList.kt(4 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/app/AppScreen.kt(3 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt(3 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/util/CompositionLocals.kt(1 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/util/Sample.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (9)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt (3)
104-107: named arguments への変更は可読性が上がっていて LGTM
121-123: Local の取り出し位置はここで妥当（AppInfo 内に閉じていて影響範囲が小さい）
45-46: SharedTransition API のバージョン要件を確認してくださいSharedTransitionLayout や sharedElement は androidx.compose.animation 1.7.0-alpha07 以降で導入された実験的 API です。実装自体は正しく（AnimatedVisibilityScope を正しく渡し、null チェックで安全に制御）、AnimatedVisibilityScope の型も適切です。プロジェクトの
build.gradleで compose-animation が 1.7.0 以上に設定されていることを確認してください。AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/AppList.kt (3)
26-27: 遷移コンテキストを Local 経由で受け取る方針は一貫していて良い
41-46: itemContent の明示化は読みやすくなっていて良い
57-60: スコープ取得は最小限で OKAndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/app/AppScreen.kt (3)
82-98: SharedTransitionLayout で NavHost を包む構成は目的に合っていて良い
100-106: CompositionLocalProvider の供給範囲が適切（目的地ごとに閉じている）Also applies to: 113-116
123-123: About ルート追加はシンプルで良い
| import androidx.compose.animation.SharedTransitionLayout | ||
| import androidx.compose.animation.slideInHorizontally | ||
| import androidx.compose.animation.slideOutHorizontally | ||
| import androidx.compose.runtime.Composable | ||
| import androidx.compose.runtime.CompositionLocalProvider |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/app/AppScreen.kt | head -130Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 6222
🏁 Script executed:
# Find build.gradle files to check dependencies
find . -name "build.gradle*" -o -name "*.toml" | grep -E "(gradle|toml)" | head -20Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 375
🏁 Script executed:
# Search for composition/dependency information
find . -type f \( -name "build.gradle" -o -name "build.gradle.kts" -o -name "*.toml" \) | xargs grep -l "androidx.compose\|androidx.navigation" 2>/dev/null | head -5Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 223
🏁 Script executed:
cat AndroidApp/gradle/libs.versions.tomlRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 4381
🏁 Script executed:
cat AndroidApp/ui/build.gradle.ktsRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 1065
🏁 Script executed:
# Find where LocalAnimatedVisibilityScope is defined
find . -type f -name "*.kt" | xargs grep -l "LocalAnimatedVisibilityScope" | head -5Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 436
🏁 Script executed:
cat -n ./AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/util/CompositionLocals.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 521
🌐 Web query:
androidx.navigation:navigation-compose 2.9.6 composable lambda scope AnimatedVisibilityScope AnimatedContentScope
💡 Result:
Short answer: In navigation-compose 2.9.6 the destination content lambda is invoked with an AnimatedContentScope receiver (which is a sealed interface that extends AnimatedVisibilityScope). The NavHost/composable transition lambdas use AnimatedContentTransitionScope. See the API docs:
- NavHost / NavGraphBuilder.composable signatures showing content: @composable AnimatedContentScope.(NavBackStackEntry) -> Unit and transition parameters: AnimatedContentTransitionScope.() -> Enter/ExitTransition. [1]
- AnimatedVisibilityScope (and that AnimatedContentScope extends it) — explains available transition/animateEnterExit helpers. [2]
- Compose source showing AnimatedContentScope : AnimatedVisibilityScope. [3]
Sources:
[1] Android NavHost / composable API reference.
[2] AnimatedVisibilityScope API reference.
[3] Compose AnimatedContent source (AnimatedContentScope implements AnimatedVisibilityScope).
🌐 Web query:
SharedTransitionLayout androidx.compose.animation 2025 SharedElement sharedElement scope requirements
💡 Result:
Short answer — requirements and rules for SharedTransitionLayout / SharedTransitionScope and Modifier.sharedElement:
- Any composable that will participate in a shared-element/shared-bounds transition must be a descendant (direct or indirect) of a SharedTransitionLayout or of a SharedTransitionScope. [1][2]
- Keys: shared elements are matched by the same key (Any) — use the same key on source and target. [1]
- If you use SharedTransitionScope (the variant that does NOT create a new Layout) you MUST apply the Modifier provided by the scope to the first/top-most child in your scope content — that Modifier obtains root coordinates and creates the overlay used to render transitioning elements. If the first child isn’t the top-most (highest zIndex), prefer SharedTransitionLayout instead. [2]
- SharedElement behavior: when a matched sharedElement starts a transition it is rendered into the scope’s overlay (to avoid parent clipping/fade). Any clipping/fade that should affect the element during transition must be applied after the sharedElement modifier (i.e., to the child inside the sharedElement). Parent clipping is applied only if there is a parent sharedBounds with clipping defined. [2][3]
References: Android Compose Animation docs (SharedTransitionLayout / SharedTransitionScope / shared elements). [1][2][3]
Sources
[1] API docs — SharedTransitionLayout / overview.
[2] API docs — SharedTransitionScope / sharedElement details.
[3] Guide — Shared elements with Navigation / AnimatedContent examples.
Gson からの null 返却を処理してください
行 109-112 の Gson().fromJson() は JSON 解析に失敗したり、引数が null/空の場合に null を返しますが、null チェックがないため DetailScreen に null を渡す可能性があります。以下のように修正してください:
composable(Screen.Detail.route) {
val appJson = it.arguments?.getString("app")
val app = appJson?.let { json -> Gson().fromJson(json, InstalledApp::class.java) }
if (app != null) {
CompositionLocalProvider(
LocalSharedTransitionScope provides this@SharedTransitionLayout,
LocalAnimatedVisibilityScope provides this@composable
) {
DetailScreen(navController = navController, app = app)
}
}
}SharedTransitionLayout と AnimatedVisibilityScope の型は互換性があります（AnimatedContentScope は AnimatedVisibilityScope を継承）。
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/app/AppScreen.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main/MainScreen.kt (1)
103-110: バックナビゲーションロジックに不整合があります
BackHandler（103-110行）がネストされたナビゲーション状態を考慮していません。タブ1の詳細画面にいる場合、バックボタンを押すとアプリが終了してしまいますが、代わりにターゲット画面に戻るべきです。100-102行で定義された
onBackロジックは使用されていません。🔎 BackHandler をネストされたナビゲーションに対応させる修正案：
BackHandler(true) { snackbarHostState.currentSnackbarData?.dismiss() - if (pagerState.currentPage == 0) { + if (pagerState.currentPage == 0 && isTab1RootScreen) { activity?.finish() + } else if (pagerState.currentPage == 0 && !isTab1RootScreen) { + tab1NavController.popBackStack() } else { scope.launch { pagerState.scrollToPage(0, 0f) } } }
🧹 Nitpick comments (3)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/app/AppScreen.kt (1)
115-121:Screen.Detailが重複して定義されています
Screen.Main.Detail(行117) とScreen.Detail(行120) の両方が存在します。どちらか一方のみ使用されるべきではないでしょうか？MainScreen.ktではScreen.Main.Detailが使用されているため、トップレベルのScreen.Detail(行120) は不要かもしれません。#!/bin/bash # Screen.Detail の使用箇所を確認 rg -n "Screen\.Detail" --type=kotlinAndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main/MainScreen.kt (2)
98-102: バックナビゲーションロジックを明確にすることを検討してください
currentBackStackEntryAsState().valueがnullの場合、isTab1RootScreenはfalseになります。これは意図した動作でしょうか？初期状態やバックスタックが空の場合の処理を明示的にすることをお勧めします。🔎 null 処理を明示的にする提案：
val isTab1RootScreen = - tab1NavController.currentBackStackEntryAsState().value?.destination?.route == Screen.Main.Targets.route + tab1NavController.currentBackStackEntryAsState().value?.destination?.route.let { + it == null || it == Screen.Main.Targets.route + }これにより、バックスタックエントリーが
nullの場合もルート画面として扱われます。
57-85: 型安全なナビゲーション引数への移行を検討してください現在の実装では
Gsonを使用してナビゲーション引数をシリアライズ/デシリアライズしていますが、このアプローチは実行時エラーが発生する可能性があります。プロジェクトは既にNavigation Compose 2.9.6を使用しており、型安全なナビゲーションに対応しています。以下の代替案をご検討ください：
- Kotlin Serializationを使用した型安全なルーティング（Navigation Compose 2.8.0以降推奨）
- Parcelableを使用した引数の受け渡し
- ViewModelや状態管理を通じた画面間のデータ共有
型安全なアプローチはコンパイル時の型チェックを提供し、本番環境でのランタイムクラッシュを防ぎます。
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/AppList.kt(4 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/AppScaffold.kt(1 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/TopBar.kt(2 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/app/AppScreen.kt(4 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt(3 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/license/LicenseScreen.kt(0 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main/MainScreen.kt(5 hunks)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/util/Extensions.kt(2 hunks)AndroidApp/ui/src/screenshotTest/kotlin/me/nya_n/notificationnotifier/ScreenshotTest.kt(0 hunks)
💤 Files with no reviewable changes (2)
- AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/license/LicenseScreen.kt
- AndroidApp/ui/src/screenshotTest/kotlin/me/nya_n/notificationnotifier/ScreenshotTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/AppList.kt
🧰 Additional context used
🧬 Code graph analysis (5)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/util/Extensions.kt (1)
AndroidApp/model/src/main/kotlin/me/nya_n/notificationnotifier/model/InstalledApp.kt (1)
tableName(9-17)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/TopBar.kt (1)
AndroidApp/ui/src/screenshotTest/kotlin/me/nya_n/notificationnotifier/ScreenshotTest.kt (1)
SubContentTopBarComponentScreenshotTest(228-237)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt (2)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/Category.kt (1)
Category(23-50)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/GrayScaleAppIcon.kt (1)
GrayScaleAppIcon(16-41)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main/MainScreen.kt (3)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/target/TargetScreen.kt (1)
TargetScreen(23-45)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt (1)
DetailScreen(51-77)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/AppScaffold.kt (1)
AppScaffold(18-54)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/app/AppScreen.kt (3)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main/MainScreen.kt (1)
MainScreen(49-121)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/license/LicenseScreen.kt (1)
LicenseScreen(17-36)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/about/AboutScreen.kt (1)
AboutScreen(23-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (8)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/util/Extensions.kt (1)
18-19: LGTM!共有要素遷移用のキー生成ロジックがシンプルで適切です。
packageNameはInstalledAppのPrimaryKeyであるため、一意性が保証されています。AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/AppScaffold.kt (1)
21-27: LGTM!
hasBackContentとonBackを単一のnullableonBackパラメータに統合したのは良いAPI設計です。冗長性が排除され、使用時の間違いも減ります。AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt (2)
108-131: LGTM!共有要素遷移の条件付き適用が適切に実装されています。
LocalSharedTransitionScopeとLocalAnimatedVisibilityScopeの両方が利用可能な場合のみ遷移を適用し、プレビューや遷移スコープ外では通常のModifierにフォールバックします。
87-97: レイアウト構造の変更を確認
DetailContentがAppScaffoldからColumnに変更されています。これはDetailScreenがMainScreenのネストされたNavHost内でレンダリングされ、TopBarは親のMainScreenが提供するためと理解しています。意図した変更であれば問題ありません。AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/app/AppScreen.kt (1)
79-106: SharedTransitionLayout の実装は適切です
SharedTransitionLayoutでナビゲーション全体をラップし、CompositionLocalProviderでLocalSharedTransitionScopeを提供する実装は正しいです。なお、過去のレビューコメントで指摘されている
Gson().fromJson()の null ハンドリングの問題はMainScreen.kt(行69-82) に存在します。そちらの修正も確認してください。AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/common/TopBar.kt (1)
20-41: レビューコメントに誤りがあります。hasBackContentパラメータはコード内に存在せず、スクリーンショットテストはすでに新しいTopBar APIと互換性があります。実際のスクリーンショットテストの呼び出しを確認したところ：
TopBar()- デフォルト値を使用（互換性あり）TopBar(onBack = { })- 新しいシグネチャと互換性ありテストの更新は不要です。
Likely an incorrect or invalid review comment.
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main/MainScreen.kt (2)
22-46: LGTM: 必要なインポートが追加されていますネストされたナビゲーションと共有遷移のために必要なインポートが適切に追加されています。
125-131: LGTM: MainContentのシグネチャ変更が適切に伝播されています
onBackパラメータの追加と、すべての呼び出し元（MainScreenとプレビュー）での更新が一貫して行われています。Also applies to: 198-204
| CompositionLocalProvider( | ||
| LocalAnimatedVisibilityScope provides this@composable |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "MainScreen.kt" -type fRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 169
🏁 Script executed:
cat -n ./AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main/MainScreen.kt | head -100Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 5417
🏁 Script executed:
find . -name "*.kt" -type f | xargs grep -l "LocalAnimatedVisibilityScope" | head -5Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 438
🏁 Script executed:
cat -n ./AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/util/CompositionLocals.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 521
🏁 Script executed:
cat -n ./AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt | head -80Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 3880
🏁 Script executed:
cat -n ./AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt | sed -n '79,120p'Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 1372
🏁 Script executed:
cat -n ./AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/detail/DetailScreen.kt | sed -n '115,140p'Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 1293
🏁 Script executed:
rg -A5 "fun composable" --type=kotlin | grep -A3 "receiver\|NavGraphBuilder" | head -20Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 61
🏁 Script executed:
find . -name "*.kt" -type f | xargs grep -l "sealed class Screen\|object Screen" | head -5Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 167
🏁 Script executed:
cat -n ./AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/app/AppScreen.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 6153
🏁 Script executed:
rg -n "fun composable" --type=kotlin -A2 | grep -A2 "androidx.navigation.compose" | head -10Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 61
🏁 Script executed:
python3 <<'EOF'
# Check if there are compilation errors or if this is a logic issue
import subprocess
import os
os.chdir('./AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main')
# Try to find the actual type of 'this' inside composable lambda
result = subprocess.run(['grep', '-n', '@Composable', 'MainScreen.kt'],
capture_output=True, text=True)
print("Composable functions in MainScreen.kt:")
print(result.stdout)
EOFRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 165
composableブロック内でのthis@composableは不適切な型です
composable()関数のレシーバー型はNavGraphBuilderであり、AnimatedVisibilityScopeではありません。そのため、this@composableをLocalAnimatedVisibilityScopeに提供することは型の不一致となります。AppScreen.kt の this@SharedTransitionLayout の例のように、正しい型のスコープ内でのみ提供してください。
ネストされたナビゲーション内で共有要素トランジションを使用する必要がある場合は、AnimatedVisibilityでコンテンツをラップするか、CompositionLocalProviderを削除してLocalAnimatedVisibilityScopeをnullのままにしてください。
🤖 Prompt for AI Agents
In
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main/MainScreen.kt
around lines 62-63, you are providing this@composable to
LocalAnimatedVisibilityScope but this@composable is a NavGraphBuilder, not an
AnimatedVisibilityScope; remove the incorrect CompositionLocalProvider or supply
a real AnimatedVisibilityScope instead. Fix by either (A) wrap the UI that needs
shared transitions in an AnimatedVisibility (or other composable that exposes
AnimatedVisibilityScope) and provide that scope using
this@AnimatedVisibilityScope inside CompositionLocalProvider, or (B) simply
remove the CompositionLocalProvider line and leave LocalAnimatedVisibilityScope
null (and rely on AnimatedVisibility-wrapped content for transitions); ensure
the provided value matches AnimatedVisibilityScope type exactly.
| val app = Gson().fromJson( | ||
| it.arguments?.getString("app"), | ||
| InstalledApp::class.java | ||
| ) |
There was a problem hiding this comment.
Gson.fromJson()のnull安全性を確保してください
Gson.fromJson()は、引数が無効な場合やJSONの解析に失敗した場合にnullを返す可能性があります。現在のコードでは、appがnullの場合にDetailScreenがクラッシュする可能性があります。
🔎 null チェックとエラーハンドリングを追加する修正案：
composable(Screen.Main.Detail.route) {
val app = Gson().fromJson(
it.arguments?.getString("app"),
InstalledApp::class.java
- )
+ ) ?: run {
+ // エラーハンドリング: 前の画面に戻る
+ tab1NavController.popBackStack()
+ return@composable
+ }
CompositionLocalProvider(
LocalAnimatedVisibilityScope provides this@composable
) {
DetailScreen(
navController = tab1NavController,
app = app
)
}
}または、Kotlinの型安全なナビゲーション引数を使用することを検討してください（例：Navigation Compose の型安全なルーティング）。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val app = Gson().fromJson( | |
| it.arguments?.getString("app"), | |
| InstalledApp::class.java | |
| ) | |
| val app = Gson().fromJson( | |
| it.arguments?.getString("app"), | |
| InstalledApp::class.java | |
| ) ?: run { | |
| // エラーハンドリング: 前の画面に戻る | |
| tab1NavController.popBackStack() | |
| return@composable | |
| } |
🤖 Prompt for AI Agents
In
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/main/MainScreen.kt
around lines 72-75, Gson.fromJson(...) may return null and the current code
passes a possibly null InstalledApp to DetailScreen causing a crash; change the
code to safely handle null by checking the result of fromJson, and if null
either navigate back / show an error UI (or log and return) instead of calling
DetailScreen with a null value, or use requireNotNull with a clear message to
fail-fast; ensure downstream code expects a non-null InstalledApp only after
this null-check or fallback has been taken.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
AndroidApp/ui/src/screenshotTest/kotlin/me/nya_n/notificationnotifier/ScreenshotTest.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
| //package me.nya_n.notificationnotifier | ||
| // | ||
| //import androidx.compose.foundation.pager.rememberPagerState | ||
| //import androidx.compose.material.icons.Icons | ||
| //import androidx.compose.material.icons.automirrored.rounded.List | ||
| //import androidx.compose.material.icons.outlined.NotificationsActive | ||
| //import androidx.compose.material.icons.outlined.Settings | ||
| //import androidx.compose.material3.SnackbarHostState | ||
| //import androidx.compose.runtime.Composable | ||
| //import androidx.compose.runtime.remember | ||
| //import androidx.compose.ui.tooling.preview.Preview | ||
| //import me.nya_n.notificationnotifier.model.InstalledApp | ||
| //import me.nya_n.notificationnotifier.ui.common.AppList | ||
| //import me.nya_n.notificationnotifier.ui.common.AppOutlinedButton | ||
| //import me.nya_n.notificationnotifier.ui.common.AppScaffold | ||
| //import me.nya_n.notificationnotifier.ui.common.Category | ||
| //import me.nya_n.notificationnotifier.ui.common.CommonDialog | ||
| //import me.nya_n.notificationnotifier.ui.common.EmptyView | ||
| //import me.nya_n.notificationnotifier.ui.common.RequireNotificationPermissionDialog | ||
| //import me.nya_n.notificationnotifier.ui.common.RequirePackageVisibilityDialog | ||
| //import me.nya_n.notificationnotifier.ui.common.TopBar | ||
| //import me.nya_n.notificationnotifier.ui.screen.detail.DetailContent | ||
| //import me.nya_n.notificationnotifier.ui.screen.main.MainContent | ||
| //import me.nya_n.notificationnotifier.ui.screen.main.TabItem | ||
| //import me.nya_n.notificationnotifier.ui.screen.selection.SelectionContent | ||
| //import me.nya_n.notificationnotifier.ui.screen.settings.SettingsContent | ||
| //import me.nya_n.notificationnotifier.ui.screen.target.TargetContent | ||
| //import me.nya_n.notificationnotifier.ui.theme.AppTheme | ||
| //import me.nya_n.notificationnotifier.ui.util.Sample | ||
| // | ||
| //class ScreenshotTest { | ||
| // | ||
| // // region: 画面レベル | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun MainContentScreenshotTest() { | ||
| // val snackbarHostState = remember { SnackbarHostState() } | ||
| // val tabItems = listOf( | ||
| // TabItem("タブ1", Icons.Outlined.NotificationsActive), | ||
| // TabItem("タブ2", Icons.AutoMirrored.Rounded.List), | ||
| // TabItem("タブ3", Icons.Outlined.Settings), | ||
| // ) | ||
| // val pagerState = rememberPagerState(pageCount = { tabItems.size }) | ||
| // AppTheme { | ||
| // MainContent( | ||
| // snackbarHostState = snackbarHostState, | ||
| // tabItems = tabItems, | ||
| // pagerState = pagerState | ||
| // ) { } | ||
| // } | ||
| // } | ||
| // | ||
| // | ||
| // @Preview(backgroundColor = 0xFFC7B5A8, showBackground = true) | ||
| // @Composable | ||
| // fun TargetContentScreenshotTest() { | ||
| // val items = listOf( | ||
| // InstalledApp("Sample App", "me.nya_n.notificationnotifier"), | ||
| // InstalledApp("Sample App", "me.nya_n.notificationnotifier"), | ||
| // InstalledApp("Sample App", "me.nya_n.notificationnotifier"), | ||
| // ) | ||
| // AppTheme { | ||
| // TargetContent( | ||
| // items = items, | ||
| // isLoading = false, | ||
| // onAppSelected = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview(backgroundColor = 0xFFC7B5A8, showBackground = true) | ||
| // @Composable | ||
| // fun DetailContentScreenshotTest() { | ||
| // val snackbarHostState = remember { SnackbarHostState() } | ||
| // AppTheme { | ||
| // DetailContent( | ||
| // snackbarHostState = snackbarHostState, | ||
| // app = InstalledApp("Sample App Name", "example.sample.test"), | ||
| // condition = "^.*$", | ||
| // onBack = { }, | ||
| // onDeleteApp = { }, | ||
| // onConditionChanged = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview(backgroundColor = 0xFFC7B5A8, showBackground = true) | ||
| // @Composable | ||
| // fun SelectionContentScreenshotTest() { | ||
| // AppTheme { | ||
| // SelectionContent( | ||
| // items = Sample.items, | ||
| // isLoading = false, | ||
| // onAppSelected = { }, | ||
| // initQuery = "", | ||
| // onQueryInputted = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview(backgroundColor = 0xFFC7B5A8, showBackground = true) | ||
| // @Composable | ||
| // fun SettingsContentScreenshotTest() { | ||
| // AppTheme { | ||
| // SettingsContent( | ||
| // address = "192.168.11.2:5555", | ||
| // versionCode = 1, | ||
| // versionName = "1.0", | ||
| // onValueChange = { }, | ||
| // onNotifyTest = { }, | ||
| // onExportData = { }, | ||
| // onImportData = { }, | ||
| // onLicense = { }, | ||
| // onAboutDeveloper = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // // endregion | ||
| // | ||
| // // region: コンポーネントレベル | ||
| // | ||
| // @Preview(backgroundColor = 0xFFC7B5A8, showBackground = true) | ||
| // @Composable | ||
| // fun AppListComponentScreenshotTest() { | ||
| // val items = listOf( | ||
| // InstalledApp("Sample App", "me.nya_n.notificationnotifier"), | ||
| // InstalledApp( | ||
| // "Sample App Name So Looooooooooooooooong", | ||
| // "me.nya_n.notificationnotifier" | ||
| // ), | ||
| // ) | ||
| // AppTheme { | ||
| // AppList( | ||
| // items = items, | ||
| // onAppSelected = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview(backgroundColor = 0xFFC7B5A8, showBackground = true) | ||
| // @Composable | ||
| // fun EmptyAppListComponentScreenshotTest() { | ||
| // AppTheme { | ||
| // AppList( | ||
| // items = emptyList(), | ||
| // onAppSelected = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun AppOutlinedButtonComponentScreenshotTest() { | ||
| // AppTheme { | ||
| // AppOutlinedButton("text") { } | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun AppScaffoldComponentScreenshotTest() { | ||
| // val snackbarHostState = remember { SnackbarHostState() } | ||
| // AppTheme { | ||
| // AppScaffold(snackbarHostState) { } | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun CategoryComponentScreenshotTest() { | ||
| // AppTheme { | ||
| // Category("カテゴリ") | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun CommonDialogComponentScreenshotTest() { | ||
| // AppTheme { | ||
| // CommonDialog( | ||
| // message = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.", | ||
| // positiveButtonText = "YES", | ||
| // negativeButtonText = "NO", | ||
| // onPositiveDismissRequest = { }, | ||
| // onNegativeDismissRequest = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun RequireNotificationPermissionDialogComponentScreenshotTest() { | ||
| // AppTheme { | ||
| // RequireNotificationPermissionDialog( | ||
| // onDismissRequest = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun RequirePackageVisibilityDialogComponentScreenshotTest() { | ||
| // AppTheme { | ||
| // RequirePackageVisibilityDialog( | ||
| // onDismissRequest = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun EmptyViewComponentScreenshotTest() { | ||
| // AppTheme { | ||
| // EmptyView(message = "empty") | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun TopBarComponentScreenshotTest() { | ||
| // AppTheme { | ||
| // TopBar() | ||
| // } | ||
| // } | ||
| // | ||
| // @Preview | ||
| // @Composable | ||
| // fun SubContentTopBarComponentScreenshotTest() { | ||
| // AppTheme { | ||
| // TopBar( | ||
| // onBack = { } | ||
| // ) | ||
| // } | ||
| // } | ||
| // | ||
| // // endregion | ||
| //} |
There was a problem hiding this comment.
スクリーンショットテストの無効化による品質リスク
ファイル全体をコメントアウトすることで、すべてのスクリーンショットテストのカバレッジが失われています。SharedTransitionLayoutと新しいCompositionLocals（LocalSharedTransitionScope/LocalAnimatedVisibilityScope）の導入により、既存のPreview関数が動作しなくなったと推測されます。
以下のいずれかの対応を推奨します：
- テストを更新する: CompositionLocalProviderを使用してモックスコープを提供し、テストを再有効化する
- TODOコメントを追加する: 技術的負債として追跡可能にする
- ファイルを削除する: コメントアウトされたコードをリポジトリに残さない
コメントアウトされたコードを残すと、メンテナンス性が低下し、将来的に混乱を招く可能性があります。
🔎 提案: テストを更新するか、追跡用のTODOを追加
テストを維持する場合の例：
// CompositionLocalを提供するヘルパー関数を作成
@Composable
private fun TestWrapper(content: @Composable () -> Unit) {
SharedTransitionLayout {
AnimatedVisibility(visible = true) {
CompositionLocalProvider(
LocalSharedTransitionScope provides this@SharedTransitionLayout,
LocalAnimatedVisibilityScope provides this
) {
content()
}
}
}
}または、削除してIssueで追跡する場合：
#!/bin/bash
# コメントアウトされたテストファイルの状態を確認
echo "=== Checking commented test file ==="
head -50 AndroidApp/ui/src/screenshotTest/kotlin/me/nya_n/notificationnotifier/ScreenshotTest.kt
# 他のスクリーンショットテストが存在するか確認
echo "=== Other screenshot tests in the project ==="
fd -e kt . AndroidApp/ui/src/screenshotTest -x head -20 {}🤖 Prompt for AI Agents
In
AndroidApp/ui/src/screenshotTest/kotlin/me/nya_n/notificationnotifier/ScreenshotTest.kt
lines 1-239: the entire file is commented out which removes screenshot test
coverage; restore tests by creating a small TestWrapper composable that sets up
SharedTransitionLayout and provides the required CompositionLocals
(LocalSharedTransitionScope and LocalAnimatedVisibilityScope) via
CompositionLocalProvider, then wrap each @Preview composable with that
TestWrapper and uncomment the preview functions; if you cannot fix now, replace
the commented file with a one-line TODO comment explaining why tests are
disabled and create an issue to track re-enabling them.
Summary by CodeRabbit
新機能
改善 / リファクタ
雑務
✏️ Tip: You can customize this high-level summary in your review settings.