Skip to content

Commit fc6e4e2

Browse files
bmartybmarty
authored
Merge pull request #6195 from element-hq/feature/bma/callButtonColor
Fix call button color and ensure call can always be declined from the notification
2 parents 6bdb9c7 + 459e70f commit fc6e4e2

File tree

9 files changed

+89
-31
lines changed

9 files changed

+89
-31
lines changed

‎features/call/impl/src/main/kotlin/io/element/android/features/call/impl/receivers/DeclineCallBroadcastReceiver.kt‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class DeclineCallBroadcastReceiver : BroadcastReceiver() {
2929
companion object {
3030
const val EXTRA_NOTIFICATION_DATA = "EXTRA_NOTIFICATION_DATA"
3131
}
32+
3233
@Inject
3334
lateinit var activeCallManager: ActiveCallManager
3435

@@ -40,7 +41,13 @@ class DeclineCallBroadcastReceiver : BroadcastReceiver() {
4041
?: return
4142
context.bindings<CallBindings>().inject(this)
4243
appCoroutineScope.launch {
43-
activeCallManager.hungUpCall(callType = CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
44+
activeCallManager.hangUpCall(
45+
callType = CallType.RoomCall(
46+
sessionId = notificationData.sessionId,
47+
roomId = notificationData.roomId,
48+
),
49+
notificationData = notificationData,
50+
)
4451
}
4552
}
4653
}

‎features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class CallScreenPresenter(
100100
)
101101
}
102102
onDispose {
103-
appCoroutineScope.launch { activeCallManager.hungUpCall(callType) }
103+
appCoroutineScope.launch { activeCallManager.hangUpCall(callType) }
104104
}
105105
}
106106

‎features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/IncomingCallActivity.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ class IncomingCallActivity : AppCompatActivity() {
118118
private fun onCancel() {
119119
val activeCall = activeCallManager.activeCall.value ?: return
120120
appCoroutineScope.launch {
121-
activeCallManager.hungUpCall(callType = activeCall.callType)
121+
activeCallManager.hangUpCall(callType = activeCall.callType)
122122
}
123123
}
124124
}

‎features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/IncomingCallScreen.kt‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ import io.element.android.libraries.matrix.api.core.SessionId
5151
import io.element.android.libraries.matrix.api.core.UserId
5252
import io.element.android.libraries.ui.strings.CommonStrings
5353

54+
/**
55+
* Ref: https://www.figma.com/design/0MMNu7cTOzLOlWb7ctTkv3/Element-X?node-id=16501-5740
56+
*/
5457
@Composable
5558
internal fun IncomingCallScreen(
5659
notificationData: CallNotificationData,
@@ -94,11 +97,8 @@ internal fun IncomingCallScreen(
9497
)
9598
}
9699
Row(
97-
modifier = Modifier
98-
.fillMaxWidth()
99-
.padding(start = 24.dp, end = 24.dp, bottom = 64.dp),
100-
horizontalArrangement = Arrangement.SpaceBetween,
101-
verticalAlignment = Alignment.CenterVertically
100+
modifier = Modifier.padding(bottom = 64.dp),
101+
horizontalArrangement = Arrangement.spacedBy(48.dp),
102102
) {
103103
ActionButton(
104104
size = 64.dp,
@@ -108,7 +108,6 @@ internal fun IncomingCallScreen(
108108
backgroundColor = ElementTheme.colors.iconSuccessPrimary,
109109
borderColor = ElementTheme.colors.borderSuccessSubtle
110110
)
111-
112111
ActionButton(
113112
size = 64.dp,
114113
onClick = onCancel,
@@ -143,7 +142,7 @@ private fun ActionButton(
143142
onClick = onClick,
144143
colors = IconButtonDefaults.filledIconButtonColors(
145144
containerColor = backgroundColor,
146-
contentColor = Color.White,
145+
contentColor = ElementTheme.colors.iconOnSolidPrimary,
147146
)
148147
) {
149148
Icon(

‎features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/ActiveCallManager.kt‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,14 @@ interface ActiveCallManager {
7272
suspend fun registerIncomingCall(notificationData: CallNotificationData)
7373

7474
/**
75-
* Called when the active call has been hung up. It will remove any existing UI and the active call.
76-
* @param callType The type of call that the user hung up, either an external url one or a room one.
75+
* Called to hang up the active call. It will hang up the call and remove any existing UI and the active call.
76+
* @param callType The type of call that the user hangs up, either an external url one or a room one.
77+
* @param notificationData The data for the incoming call notification.
7778
*/
78-
suspend fun hungUpCall(callType: CallType)
79+
suspend fun hangUpCall(
80+
callType: CallType,
81+
notificationData: CallNotificationData? = null,
82+
)
7983

8084
/**
8185
* Called after the user joined a call. It will remove any existing UI and set the call state as [CallState.InCall].
@@ -192,12 +196,28 @@ class DefaultActiveCallManager(
192196
}
193197
}
194198

195-
override suspend fun hungUpCall(callType: CallType) = mutex.withLock {
196-
Timber.tag(tag).d("Hung up call: $callType")
199+
override suspend fun hangUpCall(
200+
callType: CallType,
201+
notificationData: CallNotificationData?,
202+
) = mutex.withLock {
203+
Timber.tag(tag).d("Hang up call: $callType")
204+
cancelIncomingCallNotification()
197205
val currentActiveCall = activeCall.value ?: run {
206+
// activeCall.value can be null if the application has been killed while the call was ringing
207+
// Build a currentActiveCall with the provided parameters.
208+
notificationData?.let {
209+
ActiveCall(
210+
callType = callType,
211+
callState = CallState.Ringing(
212+
notificationData = notificationData,
213+
)
214+
)
215+
}
216+
} ?: run {
198217
Timber.tag(tag).w("No active call, ignoring hang up")
199218
return@withLock
200219
}
220+
201221
if (currentActiveCall.callType != callType) {
202222
Timber.tag(tag).w("Call type $callType does not match the active call type, ignoring")
203223
return@withLock
@@ -208,9 +228,13 @@ class DefaultActiveCallManager(
208228
matrixClientProvider.getOrRestore(notificationData.sessionId).getOrNull()
209229
?.getRoom(notificationData.roomId)
210230
?.declineCall(notificationData.eventId)
231+
?.onFailure {
232+
Timber.e(it, "Failed to decline incoming call")
233+
}
234+
?: run {
235+
Timber.tag(tag).d("Couldn't find session or room to decline call for incoming call")
236+
}
211237
}
212-
213-
cancelIncomingCallNotification()
214238
if (activeWakeLock?.isHeld == true) {
215239
Timber.tag(tag).d("Releasing partial wakelock after hang up")
216240
activeWakeLock.release()
@@ -221,7 +245,6 @@ class DefaultActiveCallManager(
221245

222246
override suspend fun joinedCall(callType: CallType) = mutex.withLock {
223247
Timber.tag(tag).d("Joined call: $callType")
224-
225248
cancelIncomingCallNotification()
226249
if (activeWakeLock?.isHeld == true) {
227250
Timber.tag(tag).d("Releasing partial wakelock after joining call")

‎features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultActiveCallManagerTest.kt‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class DefaultActiveCallManagerTest {
155155
}
156156

157157
@Test
158-
fun `hungUpCall - removes existing call if the CallType matches`() = runTest {
158+
fun `hangUpCall - removes existing call if the CallType matches`() = runTest {
159159
setupShadowPowerManager()
160160
val notificationManagerCompat = mockk<NotificationManagerCompat>(relaxed = true)
161161
val manager = createActiveCallManager(notificationManagerCompat = notificationManagerCompat)
@@ -165,7 +165,7 @@ class DefaultActiveCallManagerTest {
165165
assertThat(manager.activeCall.value).isNotNull()
166166
assertThat(manager.activeWakeLock?.isHeld).isTrue()
167167

168-
manager.hungUpCall(CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
168+
manager.hangUpCall(CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
169169
assertThat(manager.activeCall.value).isNull()
170170
assertThat(manager.activeWakeLock?.isHeld).isFalse()
171171

@@ -192,13 +192,41 @@ class DefaultActiveCallManagerTest {
192192
val notificationData = aCallNotificationData(roomId = A_ROOM_ID)
193193
manager.registerIncomingCall(notificationData)
194194

195-
manager.hungUpCall(CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
195+
manager.hangUpCall(CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
196196

197197
coVerify {
198198
room.declineCall(notificationEventId = notificationData.eventId)
199199
}
200200
}
201201

202+
@Test
203+
fun `Decline event - Hangup on a unknown call should send a decline event`() = runTest {
204+
setupShadowPowerManager()
205+
val notificationManagerCompat = mockk<NotificationManagerCompat>(relaxed = true)
206+
207+
val room = mockk<JoinedRoom>(relaxed = true)
208+
209+
val matrixClient = FakeMatrixClient().apply {
210+
givenGetRoomResult(A_ROOM_ID, room)
211+
}
212+
val clientProvider = FakeMatrixClientProvider({ Result.success(matrixClient) })
213+
214+
val manager = createActiveCallManager(
215+
matrixClientProvider = clientProvider,
216+
notificationManagerCompat = notificationManagerCompat
217+
)
218+
219+
val notificationData = aCallNotificationData(roomId = A_ROOM_ID)
220+
// Do not register the incoming call, so the manager doesn't know about it
221+
manager.hangUpCall(
222+
callType = CallType.RoomCall(notificationData.sessionId, notificationData.roomId),
223+
notificationData = notificationData,
224+
)
225+
coVerify {
226+
room.declineCall(notificationEventId = notificationData.eventId)
227+
}
228+
}
229+
202230
@OptIn(ExperimentalCoroutinesApi::class)
203231
@Test
204232
fun `Decline event - Declining from another session should stop ringing`() = runTest {
@@ -269,7 +297,7 @@ class DefaultActiveCallManagerTest {
269297
}
270298

271299
@Test
272-
fun `hungUpCall - does nothing if the CallType doesn't match`() = runTest {
300+
fun `hangUpCall - does nothing if the CallType doesn't match`() = runTest {
273301
setupShadowPowerManager()
274302
val notificationManagerCompat = mockk<NotificationManagerCompat>(relaxed = true)
275303
val manager = createActiveCallManager(notificationManagerCompat = notificationManagerCompat)
@@ -278,11 +306,12 @@ class DefaultActiveCallManagerTest {
278306
assertThat(manager.activeCall.value).isNotNull()
279307
assertThat(manager.activeWakeLock?.isHeld).isTrue()
280308

281-
manager.hungUpCall(CallType.ExternalUrl("https://example.com"))
309+
manager.hangUpCall(CallType.ExternalUrl("https://example.com"))
282310
assertThat(manager.activeCall.value).isNotNull()
283311
assertThat(manager.activeWakeLock?.isHeld).isTrue()
284312

285-
verify(exactly = 0) { notificationManagerCompat.cancel(notificationId) }
313+
// The notification is always cancelled do not block the user
314+
verify(exactly = 1) { notificationManagerCompat.cancel(notificationId) }
286315
}
287316

288317
@OptIn(ExperimentalCoroutinesApi::class)

‎features/call/impl/src/test/kotlin/io/element/android/features/call/utils/FakeActiveCallManager.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
1717

1818
class FakeActiveCallManager(
1919
var registerIncomingCallResult: (CallNotificationData) -> Unit = {},
20-
var hungUpCallResult: (CallType) -> Unit = {},
20+
var hangUpCallResult: (CallType, CallNotificationData?) -> Unit = { _, _ -> },
2121
var joinedCallResult: (CallType) -> Unit = {},
2222
) : ActiveCallManager {
2323
override val activeCall = MutableStateFlow<ActiveCall?>(null)
@@ -26,8 +26,8 @@ class FakeActiveCallManager(
2626
registerIncomingCallResult(notificationData)
2727
}
2828

29-
override suspend fun hungUpCall(callType: CallType) = simulateLongTask {
30-
hungUpCallResult(callType)
29+
override suspend fun hangUpCall(callType: CallType, notificationData: CallNotificationData?) = simulateLongTask {
30+
hangUpCallResult(callType, notificationData)
3131
}
3232

3333
override suspend fun joinedCall(callType: CallType) = simulateLongTask {

‎tests/uitests/src/test/snapshots/images/features.call.impl.ui_IncomingCallScreen_Day_0_en.png‎

Lines changed: 2 additions & 2 deletions
Loading

‎tests/uitests/src/test/snapshots/images/features.call.impl.ui_IncomingCallScreen_Night_0_en.png‎

Lines changed: 2 additions & 2 deletions
Loading

0 commit comments

Comments
 (0)