Android 16のLive Update(ProgressStyle通知)に対応#5387
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAndroid向けの新規ネイティブKotlinモジュール LiveUpdateModule とそれを呼ぶ TypeScript ブリッジを追加し、APIレベル36以上で進捗通知の開始・更新・停止を行えるようにした。compileSdk の下限を 36 に設定し、通知アイコンを追加、フック側でネイティブ呼び出しを統合。 変更内容
シーケンス図sequenceDiagram
participant React as React Component
participant Hook as useUpdateLiveActivities
participant Bridge as liveUpdateModule (TS)
participant Native as LiveUpdateModule (Kotlin)
participant System as Android Notification System
React->>Hook: マウント/開始トリガー
Hook->>Bridge: startLiveUpdate(state)
Bridge->>Native: NativeModules.LiveUpdateModule.startLiveUpdate(state)
Native->>Native: API >= 36 チェック
Native->>Native: 通知チャネルを作成/確保
Native->>System: 進捗通知を投稿
System-->>Native: 表示完了
React->>Hook: 進捗更新イベント
Hook->>Bridge: updateLiveUpdate(state)
Bridge->>Native: NativeModules.LiveUpdateModule.updateLiveUpdate(state)
Native->>System: 通知内容・進捗を更新
React->>Hook: アンマウント/停止
Hook->>Bridge: stopLiveUpdate()
Bridge->>Native: NativeModules.LiveUpdateModule.stopLiveUpdate()
Native->>System: 通知をキャンセル
推定コードレビュー工数🎯 3 (Moderate) | ⏱️ ~20 minutes 関連の可能性があるPR
ウサギの詩
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/utils/native/android/liveUpdateModule.ts (1)
22-26:startLiveUpdateの引数が省略可能になっていますが、Kotlin 側は必須です。
state?: LiveUpdateStateと省略可能にしていますが、Kotlin 側のstartLiveUpdate(state: ReadableMap)は引数を必須としています。stateなしで呼び出すと、Kotlin 側でnullが渡され、state.getString()などでNullPointerExceptionが発生する可能性があります。実際の呼び出し元 (
useUpdateLiveActivities.ts) では常にactivityStateを渡しているため、現時点では問題ありませんが、型の整合性のために修正を検討してください。🔧 修正案
-export const startLiveUpdate = (state?: LiveUpdateState) => { +export const startLiveUpdate = (state: LiveUpdateState) => { if (IS_LIVE_UPDATE_ELIGIBLE_PLATFORM) { LiveUpdateModule?.startLiveUpdate?.(state); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/native/android/liveUpdateModule.ts` around lines 22 - 26, The startLiveUpdate TypeScript wrapper declares an optional parameter (state?: LiveUpdateState) but the native Kotlin signature startLiveUpdate(state: ReadableMap) requires a non-null argument, so calling with undefined can lead to a NullPointerException; make the TS parameter required (change startLiveUpdate to accept state: LiveUpdateState) and/or add an explicit runtime guard that throws a clear error if state is missing before calling LiveUpdateModule.startLiveUpdate, referencing the startLiveUpdate function in this file and the Kotlin startLiveUpdate(state: ReadableMap) to ensure type/runtime consistency.android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt (1)
74-76:@Suppress("NewApi")の使用は適切ですが、代替案も検討してください。
@Suppress("NewApi")で API 36+ のコードを許可していますが、呼び出し元 (startLiveUpdate,updateLiveUpdate) で既に API レベルチェックを行っているため、問題ありません。ただし、より明示的なアプローチとして
@RequiresApi(36)を使用し、呼び出し元でのチェックを強制する方法もあります。現状のままでも機能しますが、将来的なメンテナンス性を考慮すると良いかもしれません。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt` around lines 74 - 76, Replace the `@Suppress`("NewApi") on postProgressNotification with a more explicit `@RequiresApi`(36) annotation (and remove the suppress) so callers are required to perform the API-level check; update or confirm the callers startLiveUpdate and updateLiveUpdate perform the API 36+ check before invoking postProgressNotification to satisfy the new requirement and avoid adding runtime suppressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt`:
- Around line 104-108: The Kotlin code appends "方面" to boundStationName causing
double suffixes or mixed-language output; change LiveUpdateModule.kt so it no
longer blindly appends "方面" — either (a) remove this append and trust the
TypeScript producer (useUpdateLiveActivities.ts) to supply the correctly
localized string, or (b) if you must append here, only do so for Japanese locale
and only when boundStationName does not already end with "方面" (check
boundStationName.endsWith("方面")); update the logic around the boundStationName
variable accordingly to avoid producing "渋谷方面方面" or adding "方面" to English names
like "Shibuya".
---
Nitpick comments:
In `@android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt`:
- Around line 74-76: Replace the `@Suppress`("NewApi") on postProgressNotification
with a more explicit `@RequiresApi`(36) annotation (and remove the suppress) so
callers are required to perform the API-level check; update or confirm the
callers startLiveUpdate and updateLiveUpdate perform the API 36+ check before
invoking postProgressNotification to satisfy the new requirement and avoid
adding runtime suppressions.
In `@src/utils/native/android/liveUpdateModule.ts`:
- Around line 22-26: The startLiveUpdate TypeScript wrapper declares an optional
parameter (state?: LiveUpdateState) but the native Kotlin signature
startLiveUpdate(state: ReadableMap) requires a non-null argument, so calling
with undefined can lead to a NullPointerException; make the TS parameter
required (change startLiveUpdate to accept state: LiveUpdateState) and/or add an
explicit runtime guard that throws a clear error if state is missing before
calling LiveUpdateModule.startLiveUpdate, referencing the startLiveUpdate
function in this file and the Kotlin startLiveUpdate(state: ReadableMap) to
ensure type/runtime consistency.
android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt`:
- Around line 57-67: React Native may pass undefined as null to the Kotlin
bridge, so the non-nullable parameter in startLiveUpdate(state: ReadableMap) and
updateLiveUpdate(state: ReadableMap) can cause NPEs when
postProgressNotification reads values; change both method signatures to accept a
nullable ReadableMap? and guard against null (either return early or handle it
inside postProgressNotification), ensuring any calls like state.getString(...)
are only performed after a null-check; reference startLiveUpdate,
updateLiveUpdate, and postProgressNotification when applying the nullability
checks.
- Around line 46-54: createContentIntent() must guard against
getLaunchIntentForPackage(...) returning null: change createContentIntent to
return a nullable PendingIntent (PendingIntent?) and check the result of
reactApplicationContext.packageManager.getLaunchIntentForPackage(...); if intent
is null return null, otherwise call PendingIntent.getActivity(...) with the
non-null intent; update any callers that call createContentIntent() (e.g.,
notification builder code that calls setContentIntent) to handle a null return
by not calling setContentIntent when contentIntent is null so you don't pass a
null Intent into PendingIntent.getActivity.
android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt`:
- Around line 78-87: Replace direct ReadableMap getters in LiveUpdateModule
(where stationName, nextStationName, approaching, stopped, lineName, lineColor,
progress, trainTypeName, boundStationName, passingStationName are read) with
defensive extension helpers: add optString, optBoolean, optDouble extension
functions on ReadableMap that check hasKey and isNull and return a default, then
use state.optString("..."), state.optBoolean("...") and state.optDouble("...")
instead of getString/getBoolean/getDouble so missing or null keys won’t crash.
android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt (1)
43-49: 通知チャンネルの表示文言はリソース化推奨。
ユーザー表示文字列なので、strings.xmlへ移してローカライズ可能にしておくと保守性が上がります。♻️ 例
- val channel = NotificationChannel( - CHANNEL_ID, - "運行情報", - NotificationManager.IMPORTANCE_DEFAULT - ).apply { - description = "現在の運行状況をリアルタイムで表示します" + val channel = NotificationChannel( + CHANNEL_ID, + reactApplicationContext.getString(R.string.live_update_channel_name), + NotificationManager.IMPORTANCE_DEFAULT + ).apply { + description = reactApplicationContext.getString(R.string.live_update_channel_desc)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt` around lines 43 - 49, The hardcoded Japanese title "運行情報" and description "現在の運行状況をリアルタイムで表示します" in the NotificationChannel creation should be moved to resources: add entries to strings.xml (e.g., notification_channel_title and notification_channel_description), then update the NotificationChannel instantiation in LiveUpdateModule (where NotificationChannel and CHANNEL_ID are used) to use context.getString(R.string.notification_channel_title) and context.getString(R.string.notification_channel_description) (or requireContext().getString(...) / reactContext.getString(...) depending on available context variable) instead of string literals so the texts are localizable and maintainable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt`:
- Around line 43-49: The hardcoded Japanese title "運行情報" and description
"現在の運行状況をリアルタイムで表示します" in the NotificationChannel creation should be moved to
resources: add entries to strings.xml (e.g., notification_channel_title and
notification_channel_description), then update the NotificationChannel
instantiation in LiveUpdateModule (where NotificationChannel and CHANNEL_ID are
used) to use context.getString(R.string.notification_channel_title) and
context.getString(R.string.notification_channel_description) (or
requireContext().getString(...) / reactContext.getString(...) depending on
available context variable) instead of string literals so the texts are
localizable and maintainable.
|
@coderabbitai nitpickの件をi18n対応として起票してください |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 25941
🏁 Script executed: Length of output: 49
Issue Live Update通知チャンネルのハードコードされた文言(「運行情報」「現在の運行状況をリアルタイムで表示します」)を 🧠 Learnings used |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt (1)
106-111: 通知コンテンツの日本語文字列もi18n対応の検討を
"通過中"と"まもなく"がハードコードされています。Issue#5388でチャンネル文言のi18n対応が追跡されていますが、これらの通知コンテンツ文字列も同様にstrings.xmlへ移動することを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt` around lines 106 - 111, The notification content currently hardcodes Japanese fragments ("通過中" and "まもなく") when building contentTitle in LiveUpdateModule.kt (using passingStationName, stopped, approaching, stationName, nextStationName); extract these texts into strings.xml (e.g. notification_passing="%1$s 通過中" and notification_approaching="まもなく %1$s" or separate short labels) and replace the hardcoded literals with resource lookups via the module's Context (getString(R.string.notification_passing, passingStationName) / getString(R.string.notification_approaching, nextStationName)) or equivalent formatting so the strings become localizable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@android/app/src/main/java/me/tinykitten/trainlcd/LiveUpdateModule.kt`:
- Around line 106-111: The notification content currently hardcodes Japanese
fragments ("通過中" and "まもなく") when building contentTitle in LiveUpdateModule.kt
(using passingStationName, stopped, approaching, stationName, nextStationName);
extract these texts into strings.xml (e.g. notification_passing="%1$s 通過中" and
notification_approaching="まもなく %1$s" or separate short labels) and replace the
hardcoded literals with resource lookups via the module's Context
(getString(R.string.notification_passing, passingStationName) /
getString(R.string.notification_approaching, nextStationName)) or equivalent
formatting so the strings become localizable.
637d42a to
d8c7229
Compare
https://claude.ai/code/session_016hTeobTKcEKjphKCN5p9nz
Summary by CodeRabbit
新機能
Chores