feat(Android): Add full screen incoming call #6977
Conversation
…display, streamline avatar loading logic, and add username field to VoipPayload for improved avatar handling. Update layout to reflect changes in avatar ImageView ID.
…able avatar sizes and refactor IncomingCallActivity to utilize new avatar URI method for improved avatar loading logic.
|
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:
WalkthroughAdds an Android incoming-call UI with resources and styles, Glide dependency, avatar URI sizing and caller factory in Ejson, VoipPayload extended with username/hostName, IncomingCallActivity avatar loading (Glide) and rounded corners, plus minor JS/CI formatting tweaks. Changes
Sequence DiagramsequenceDiagram
participant Activity as IncomingCallActivity
participant Payload as VoipPayload
participant Ejson as Ejson
participant Glide as Glide
participant Server as AvatarServer
Activity->>Payload: read username, hostName
Activity->>Activity: validate payload
Activity->>Ejson: forCallerAvatar(hostName, username)
Ejson-->>Activity: Ejson instance
Activity->>Ejson: getCallerAvatarUri(sizePx)
Ejson-->>Activity: avatar URI
Activity->>Glide: load(avatarUri) into ImageView
Glide->>Server: fetch image
Server-->>Glide: return image
Glide->>Glide: apply rounded corners
Glide-->>Activity: deliver bitmap (success/fail)
Activity->>Activity: show/hide avatar, update UI
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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: 4
🤖 Fix all issues with AI agents
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.kt`:
- Around line 70-71: The fromMap function currently returns null when
data["username"] is missing, causing payloads to be dropped; change fromMap in
VoipPayload.kt to treat username as optional and default to an empty string
(consistent with the username field default and fromBundle), i.e. replace the
early-return behavior in fromMap with assigning username = data["username"] ?:
"" (or equivalent) so the payload is not discarded when username is omitted;
keep hostName handling as-is and ensure the constructed VoipPayload uses this
defaulted username.
- Around line 16-27: The fromMap() implementation of VoipPayload currently
requires "username" (and treats it differently than fromBundle()), causing null
returns when the server omits it; update fromMap() in class VoipPayload to treat
username (and hostName) as optional by reading map["username"] as? String ?: ""
(and map["hostName"] as? String ?: "") instead of returning null, while keeping
host and type validation intact so only required fields still cause null.
In `@android/app/src/main/res/layout/activity_incoming_call.xml`:
- Around line 52-58: The layout currently hardcodes accessibility and default
texts — replace the literal "Caller avatar", "Unknown caller", and "Unknown
host" with string resources: add entries (e.g., caller_avatar, unknown_caller,
unknown_host) to strings_incoming_call.xml and update the layout attributes (for
the ImageView with android:id="@+id/avatar" use
android:contentDescription="@string/caller_avatar" and replace any hardcoded
text attributes in this XML that show "Unknown caller"/"Unknown host" with
`@string/unknown_caller` and `@string/unknown_host`) so localization and TalkBack
use resource strings.
In `@android/app/src/main/res/values-night/colors_incoming_call.xml`:
- Around line 5-8: The night theme color resource names don't match the ones
used by the layout, so rename the color entries in the night colors file: change
incoming_call_status_text -> incoming_call_header_text,
incoming_call_status_icon -> incoming_call_header_icon, and incoming_call_name
-> incoming_call_host_name so the night overrides are picked up; optionally keep
or remove incoming_call_org if unused (or rename it only if the layout actually
references a corresponding key).
🧹 Nitpick comments (8)
android/app/src/main/java/chat/rocket/reactnative/notification/Ejson.java (1)
112-121:forCallerAvatarcreates a partially initializedEjson— consider adding a guard or documenting the limitation.The factory sets only
hostandcaller.username, leaving all other fields (type,sender,rid, etc.) asnull. CallinggetAvatarUri()on this instance would NPE or returnnullsilently sincetypeisnull. Current usage only callsgetCallerAvatarUri(), so it works fine today, but a future caller could easily misuse it.A brief Javadoc note or an assertion would help prevent this.
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (4)
111-142: Consider importing Glide types instead of using fully-qualified class names inline.The
CustomTarget,Drawable, andTransitiontypes are referenced with fully-qualified names inside the anonymous object (lines 122-125, 132, 137). Importing them would improve readability.♻️ Proposed refactor — add imports and simplify
Add these imports at the top of the file:
import android.graphics.drawable.Drawable import com.bumptech.glide.request.target.CustomTarget import com.bumptech.glide.request.transition.TransitionThen simplify the callback:
Glide.with(this) .load(avatarUrl) - .into(object : com.bumptech.glide.request.target.CustomTarget<android.graphics.drawable.Drawable>(sizePx, sizePx) { + .into(object : CustomTarget<Drawable>(sizePx, sizePx) { override fun onResourceReady( - resource: android.graphics.drawable.Drawable, - transition: com.bumptech.glide.request.transition.Transition<in android.graphics.drawable.Drawable>? + resource: Drawable, + transition: Transition<in Drawable>? ) { imageView.visibility = View.VISIBLE imageView.setImageDrawable(resource) applyAvatarRoundCorners(imageView, cornerRadiusPx) } - override fun onLoadFailed(errorDrawable: android.graphics.drawable.Drawable?) { + override fun onLoadFailed(errorDrawable: Drawable?) { imageView.visibility = View.GONE } - override fun onLoadCleared(placeholder: android.graphics.drawable.Drawable?) { + override fun onLoadCleared(placeholder: Drawable?) { imageView.visibility = View.GONE } })
207-217: TODO: Decline call via REST API is unimplemented — the caller will keep ringing.Line 214 notes a TODO to call the REST API to decline the call. Without this, pressing "Decline" only dismisses the local UI and notification but does not inform the server, so the caller's phone will continue ringing until timeout. This will lead to a poor UX for both parties.
Would you like me to open an issue to track this? Or do you want me to help sketch out the REST API call implementation?
149-160: Nit: redundant local variableradius.Line 152 assigns
val radius = cornerRadiusPxbutcornerRadiusPxcould be used directly. The lambda captures the parameter just fine.♻️ Minor cleanup
private fun applyAvatarRoundCorners(imageView: ImageView, cornerRadiusPx: Float) { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) return imageView.post { - val radius = cornerRadiusPx imageView.outlineProvider = object : ViewOutlineProvider() { override fun getOutline(view: View, outline: android.graphics.Outline) { - outline.setRoundRect(0, 0, view.width, view.height, radius) + outline.setRoundRect(0, 0, view.width, view.height, cornerRadiusPx) } } imageView.clipToOutline = true } }
224-226: MigrateonBackPressed()toOnBackInvokedCallbackfor API 33+ compatibility.
onBackPressed()is deprecated starting with Android 13 (API 33). Since the app targets SDK 35, useOnBackInvokedCallbackwithgetOnBackInvokedDispatcher()instead. While the current code compiles and functions, modernizing this ensures consistency with Android's back gesture handling framework.Example migration for API 33+
private val onBackInvokedCallback = OnBackInvokedCallback { voipPayload?.let { handleDecline(it) } ?: finish() } override fun onStart() { super.onStart() onBackInvokedDispatcher.registerOnBackInvokedCallback( OnBackInvokedDispatcher.PRIORITY_DEFAULT, onBackInvokedCallback ) } override fun onStop() { onBackInvokedDispatcher.unregisterOnBackInvokedCallback(onBackInvokedCallback) super.onStop() }android/app/src/main/res/layout/activity_incoming_call.xml (3)
17-24:android:tintis deprecated — useapp:tintfor backward compatibility.
android:tint(line 24) was added in API 21 and has inconsistent behavior across vendors. The AppCompatapp:tintattribute (fromxmlns:app="http://schemas.android.com/apk/res-auto") provides consistent tinting viaAppCompatImageView.♻️ Proposed fix
Add the
appnamespace to the rootLinearLayout:xmlns:app="http://schemas.android.com/apk/res-auto"Then replace the tint attribute:
- android:tint="@color/incoming_call_header_icon" /> + app:tint="@color/incoming_call_header_icon" />Also consider changing the
ImageViewtoandroidx.appcompat.widget.AppCompatImageViewfor guaranteed compat tinting.
46-50: Negative margin (-60dp) can cause clipping on some OEM skins and older APIs.The
android:layout_marginTop="-60dp"on the avatarFrameLayoutshifts it upward to visually overlap with the header area. While this works on most stock Android devices, some OEM implementations (Samsung OneUI, Xiaomi MIUI) may clip content at parent boundaries when negative margins are used within aLinearLayout.Consider using a
ConstraintLayoutfor the center section to achieve the overlap effect with constraints instead of negative margins, or verify this renders correctly across target devices.
10-15: FixedpaddingTop="108dp"may not adapt well to different screen sizes.108dp is a significant fixed offset. On smaller devices (~480dp screen height), this consumes ~22% of the vertical space before any content appears. Combined with the bottom buttons'
marginBottom="48dp", the layout may run out of room on compact screens.Consider using a percentage-based offset (via
ConstraintLayoutguidelines) or a smaller top padding that scales better, especially since the PR notes that tablet support hasn't been reviewed yet.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
android/app/build.gradleandroid/app/src/main/java/chat/rocket/reactnative/notification/Ejson.javaandroid/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.ktandroid/app/src/main/res/drawable/bg_avatar_incoming_call.xmlandroid/app/src/main/res/drawable/bg_btn_accept.xmlandroid/app/src/main/res/drawable/bg_btn_reject.xmlandroid/app/src/main/res/drawable/ic_call.xmlandroid/app/src/main/res/drawable/ic_call_end.xmlandroid/app/src/main/res/layout/activity_incoming_call.xmlandroid/app/src/main/res/values-night/colors_incoming_call.xmlandroid/app/src/main/res/values/colors_incoming_call.xmlandroid/app/src/main/res/values/strings_incoming_call.xmlandroid/app/src/main/res/values/styles_incoming_call.xmlapp/sagas/deepLinking.jsapp/sagas/login.jsindex.js
🧰 Additional context used
🧬 Code graph analysis (3)
index.js (1)
app/lib/services/voip/MediaCallLogger.ts (1)
error(14-16)
android/app/src/main/java/chat/rocket/reactnative/notification/Ejson.java (4)
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (1)
TAG(28-109)android/app/src/main/java/chat/rocket/reactnative/notification/NotificationIntentHandler.kt (1)
TAG(16-186)android/app/src/main/java/chat/rocket/reactnative/notification/RCFirebaseMessagingService.kt (1)
TAG(18-79)android/app/src/main/java/chat/rocket/reactnative/notification/VideoConfBroadcast.kt (1)
TAG(14-75)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.kt (1)
app/definitions/Voip.ts (1)
VoipPayload(7-13)
🔇 Additional comments (15)
app/sagas/deepLinking.js (1)
49-49: Formatting-only change — looks good.Parenthesizing the single arrow-function parameter aligns with the project's style consistency effort across the other files in this PR.
index.js (1)
50-50: Formatting-only change — LGTM.app/sagas/login.js (1)
63-63: All formatting-only changes — LGTM.Consistent parenthesization of single arrow-function parameters across the WatermelonDB callbacks. No behavioral change.
Also applies to: 110-110, 282-282, 287-287, 364-364
android/app/build.gradle (1)
151-151: LGTM!Glide dependency addition looks correct. The version is properly externalized to
rootProject.ext.glideVersion.android/app/src/main/res/drawable/ic_call.xml (1)
1-9: LGTM!Standard vector drawable definition.
android/app/src/main/res/values/colors_incoming_call.xml (1)
1-15: LGTM!Light theme color definitions are well-structured with clear naming. See the comment on the night-theme file for a naming mismatch that needs to be resolved.
android/app/src/main/res/drawable/bg_avatar_incoming_call.xml (1)
1-6: LGTM!android/app/src/main/res/drawable/bg_btn_accept.xml (1)
1-6: LGTM!android/app/src/main/res/drawable/bg_btn_reject.xml (1)
1-6: LGTM!android/app/src/main/res/drawable/ic_call_end.xml (1)
1-9: LGTM!Standard vector drawable definition.
android/app/src/main/res/values/styles_incoming_call.xml (1)
7-7: LGTM!The window background now correctly references the theme-aware color resource, aligning with the
DayNightparent theme for consistent light/dark mode support.android/app/src/main/res/values/strings_incoming_call.xml (1)
1-6: LGTM!String resources are well-named and correctly use the Unicode ellipsis character. Layout references align with these IDs.
android/app/src/main/java/chat/rocket/reactnative/notification/Ejson.java (2)
60-76: LGTM —buildAvatarUrinull-safety and size parameterization.Good improvement: early return when
serverURLis null/empty prevents NPE downstream. The size parameter is cleanly integrated without breaking existing callers.
127-148: LGTM — overloadedgetCallerAvatarUriwith default size.Clean backward-compatible overload. The default delegates to the parameterized version correctly.
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (1)
80-102: LGTM — font loading with graceful fallback.Good approach: if Bold fails, it falls back to Regular; if Regular fails, the method returns early without crashing. The null-safe
?.setTypefacecalls also prevent NPE if a view ID is missing from the layout.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
android/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.kt
Outdated
Show resolved
Hide resolved
android/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.kt
Outdated
Show resolved
Hide resolved
…cket.Chat.ReactNative into feat.voip-fs-incoming-call
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/prettier.yml:
- Around line 49-50: The workflow currently pushes a formatting commit using the
git commit command which removed the "[skip ci]" token and thus retriggers the
workflow; update the commit invocation in .github/workflows/prettier.yml (the
git commit -m "chore: format code and fix lint issues" step) to include "[skip
ci]" in the message (e.g., "chore: format code and fix lint issues [skip ci]")
so the push does not trigger a redundant CI run, or alternatively reintroduce an
equivalent mechanism (adding "[ci skip]" or using git push options) to prevent
the workflow from self-triggering.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/prettier.yml
⏰ 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@android/app/src/main/res/layout/activity_incoming_call.xml`:
- Around line 44-59: The FrameLayout containing the ImageView (id "avatar")
lacks the fallback initial, so add a TextView inside the same FrameLayout as the
ImageView to display the caller's initial (centered, matching the 120dp size)
and use it as the visible default while the ImageView remains gone; ensure the
TextView has an id (e.g., "avatar_initial"), appropriate textAppearance/size,
centered gravity, accessible contentDescription, and that your loading logic
will swap visibility (hide "avatar_initial" and show "avatar" when the image
loads successfully).
🧹 Nitpick comments (1)
android/app/src/main/res/layout/activity_incoming_call.xml (1)
45-49: Negative top margin is fragile and may cause clipping.
android:layout_marginTop="-60dp"(line 48) is used to visually shift the avatar upward into the header area. Negative margins are not officially guaranteed to work consistently across all Android layout managers and can cause the view to be clipped by its parent. Consider using aConstraintLayoutwith vertical bias ortranslationYfor a more robust overlap effect.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.ktandroid/app/src/main/res/layout/activity_incoming_call.xmlandroid/app/src/main/res/values-night/colors_incoming_call.xmlandroid/app/src/main/res/values/strings_incoming_call.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- android/app/src/main/res/values/strings_incoming_call.xml
- android/app/src/main/res/values-night/colors_incoming_call.xml
- android/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.kt
⏰ 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: ESLint and Test / run-eslint-and-test
🔇 Additional comments (2)
android/app/src/main/res/layout/activity_incoming_call.xml (2)
83-175: Action buttons look good — adequate touch targets and proper accessibility.Both decline and accept buttons use
focusable,clickable,contentDescriptionvia string resources, and sufficient padding (16dp around 64dp icons) for comfortable touch targets. The spacer-based layout correctly pushes them to opposite sides.
17-23: Useapp:tintinstead of deprecatedandroid:tint.
android:tintonImageViewis flagged by AppCompat lint rules as deprecated. Useapp:tintfrom the AndroidX/AppCompat namespace for proper tinting. You'll need to addxmlns:app="http://schemas.android.com/apk/res-auto"to the root element.Proposed fix
Add namespace to root:
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" xmlns:tools="http://schemas.android.com/tools" + xmlns:app="http://schemas.android.com/apk/res-auto"Then in the ImageView:
- android:tint="@color/incoming_call_header_icon" /> + app:tint="@color/incoming_call_header_icon" />
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (1)
207-217:⚠️ Potential issue | 🟠 MajorTODO: Decline doesn't notify the server — call may ring indefinitely on other devices.
Line 214 has a TODO to call the REST API on decline. Without this, declining only stops the local ringtone and dismisses the notification. The caller and any other devices will continue ringing. Ensure this is tracked for follow-up.
Would you like me to open an issue to track implementing the server-side decline call?
🤖 Fix all issues with AI agents
In
`@android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt`:
- Around line 106-107: The current calls in IncomingCallActivity.kt that set
caller and host TextViews use payload.caller.ifEmpty { "" } and
payload.hostName.ifEmpty { "" }, which are no-ops; either assign payload.caller
and payload.hostName directly to the TextViews, or provide a real fallback
(e.g., use payload.caller.ifBlank { "Unknown Caller" } and
payload.hostName.ifBlank { "Unknown Host" } or another meaningful default) so
the UI shows a usable name; update the two assignments that call
findViewById<TextView>(...) to use the chosen approach.
🧹 Nitpick comments (5)
android/app/src/main/res/layout/activity_incoming_call.xml (2)
17-23:android:tintis deprecated — preferapp:tintwithAppCompatImageView.
android:tintonImageViewis deprecated. Useapp:tint(from theandroidx.appcompatnamespace) to ensure consistent tinting across API levels. You'll also need to add theappnamespace to the root element.Suggested fix
Add namespace to root:
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" xmlns:tools="http://schemas.android.com/tools" + xmlns:app="http://schemas.android.com/apk/res-auto"Then on the ImageView:
- android:tint="@color/incoming_call_header_icon" /> + app:tint="@color/incoming_call_header_icon" />
91-101: Ensure adequate touch target size for action buttons.The 64dp icon is wrapped in a
LinearLayoutwith 16dp padding, giving a reasonable touch area. However, on the reject button thelayout_marginStart="44dp"and on acceptlayout_marginEnd="44dp"push buttons close to edges on narrow screens. Verify this works well on small-screen devices (≤ 320dp width).Also applies to: 136-146
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (3)
122-141: Use imports instead of fully-qualified class names in theCustomTargetcallback.
com.bumptech.glide.request.target.CustomTarget,android.graphics.drawable.Drawable, andcom.bumptech.glide.request.transition.Transitionare spelled out inline. Import them at the top of the file for readability.Suggested fix
Add to imports:
import android.graphics.drawable.Drawable import com.bumptech.glide.request.target.CustomTarget import com.bumptech.glide.request.transition.TransitionThen simplify:
- Glide.with(this) - .load(avatarUrl) - .into(object : com.bumptech.glide.request.target.CustomTarget<android.graphics.drawable.Drawable>(sizePx, sizePx) { - override fun onResourceReady( - resource: android.graphics.drawable.Drawable, - transition: com.bumptech.glide.request.transition.Transition<in android.graphics.drawable.Drawable>? - ) { + Glide.with(this) + .load(avatarUrl) + .into(object : CustomTarget<Drawable>(sizePx, sizePx) { + override fun onResourceReady( + resource: Drawable, + transition: Transition<in Drawable>? + ) {And similarly for
onLoadFailed/onLoadClearedparameter types.
149-160: Remove unnecessary Lollipop guard and redundant variable.The project's
minSdkVersionis 24 (Android 7.0), making theBuild.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOPcheck on line 150 dead code. Additionally,val radius = cornerRadiusPxis a redundant alias — use the parameter directly in thesetRoundRectcall.Suggested fix
private fun applyAvatarRoundCorners(imageView: ImageView, cornerRadiusPx: Float) { - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) return imageView.post { - val radius = cornerRadiusPx imageView.outlineProvider = object : ViewOutlineProvider() { override fun getOutline(view: View, outline: android.graphics.Outline) { - outline.setRoundRect(0, 0, view.width, view.height, radius) + outline.setRoundRect(0, 0, view.width, view.height, cornerRadiusPx) } } imageView.clipToOutline = true } }
224-226:onBackPressed()is deprecated since API 33. UseOnBackPressedDispatcherwith anOnBackPressedCallbackinstead.Register the callback in
onCreateaftersetContentView:onBackPressedDispatcher.addCallback( this, object : OnBackPressedCallback(true) { override fun handleOnBackPressed() { voipPayload?.let { handleDecline(it) } ?: finish() } } )Then remove the
onBackPressed()override entirely.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.ktandroid/app/src/main/res/layout/activity_incoming_call.xml
⏰ 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (3)
android/app/src/main/res/layout/activity_incoming_call.xml (1)
1-176: Layout looks good overall for the incoming call screen.The three-section structure (header, center content, bottom actions) is clear, the spacer approach for button positioning works, and string/color resources are properly referenced. The
tools:textusage for preview-only defaults is correct.android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (2)
81-103: Font loading approach is reasonable with proper fallbacks.The try/catch pattern for font loading with fallback from Inter-Bold to Inter-Regular is well-handled. Early return if the base font fails is appropriate.
38-79:onCreateflow is well-structured.Good defensive checks: keyguard dismissal before content view, API-level branching for lock-screen flags, early exit on invalid payload. The flow is clear and handles edge cases properly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt
Outdated
Show resolved
Hide resolved
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
diegolmello
deployed
to
experimental_android_build
GitHub Actions
Active
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
upload_experimental_android
GitHub Actions
Waiting
|
Android Build Available Rocket.Chat Experimental 4.69.0.108258 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNQTOwiY8SZrVz5nXIqIfAn6bB9hPZE836IKDqE1EvTwzfFLkF4o1810S0ZIPP4GWnJcYWWnpia5zxmHx61f |
2 participants
Proposed changes
Adds a proper full screen incoming call to Voice.
Supports both light and dark themes.
Missing:
Issue(s)
https://rocketchat.atlassian.net/browse/VMUX-37
How to test or reproduce
Screenshots
locked_app_background.mov
locked_app_closed_dark_theme.mov
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Chores