Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve pip and add feature flag. #3199

Merged
merged 5 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions features/call/impl/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ dependencies {
implementation(projects.libraries.architecture)
implementation(projects.libraries.core)
implementation(projects.libraries.designsystem)
implementation(projects.libraries.featureflag.api)
implementation(projects.libraries.matrix.impl)
implementation(projects.libraries.matrixui)
implementation(projects.libraries.network)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@

package io.element.android.features.call.impl.pip

import androidx.compose.ui.tooling.preview.PreviewParameterProvider

open class PictureInPictureStateProvider : PreviewParameterProvider<PictureInPictureState> {
override val values: Sequence<PictureInPictureState>
get() = sequenceOf(
aPictureInPictureState(supportPip = true),
aPictureInPictureState(supportPip = true, isInPictureInPicture = true),
)
}

fun aPictureInPictureState(
supportPip: Boolean = false,
isInPictureInPicture: Boolean = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
import io.element.android.libraries.core.bool.orFalse
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.di.ApplicationContext
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.featureflag.api.FeatureFlags
import kotlinx.coroutines.runBlocking
import javax.inject.Inject

interface PipSupportProvider {
Expand All @@ -34,9 +37,15 @@
@ContributesBinding(AppScope::class)
class DefaultPipSupportProvider @Inject constructor(
@ApplicationContext private val context: Context,
private val featureFlagService: FeatureFlagService,

Check warning on line 40 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipSupportProvider.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipSupportProvider.kt#L40

Added line #L40 was not covered by tests
) : PipSupportProvider {
override fun isPipSupported(): Boolean {
val hasSystemFeaturePip = context.packageManager?.hasSystemFeature(PackageManager.FEATURE_PICTURE_IN_PICTURE).orFalse()
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && hasSystemFeaturePip
val isSupportedByTheOs = Build.VERSION.SDK_INT >= Build.VERSION_CODES.O &&
context.packageManager?.hasSystemFeature(PackageManager.FEATURE_PICTURE_IN_PICTURE).orFalse()
return if (isSupportedByTheOs) {
runBlocking { featureFlagService.isFeatureEnabled(FeatureFlags.PictureInPicture) }

Check warning on line 46 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipSupportProvider.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipSupportProvider.kt#L46

Added line #L46 was not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is OK to use a runBlocking here, since at some point the FF will be removed.

} else {
false

Check warning on line 48 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipSupportProvider.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipSupportProvider.kt#L48

Added line #L48 was not covered by tests
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import io.element.android.compound.tokens.generated.CompoundIcons
import io.element.android.features.call.impl.R
import io.element.android.features.call.impl.pip.PictureInPictureEvents
import io.element.android.features.call.impl.pip.PictureInPictureState
import io.element.android.features.call.impl.pip.PictureInPictureStateProvider
import io.element.android.features.call.impl.pip.aPictureInPictureState
import io.element.android.features.call.impl.utils.WebViewWidgetMessageInterceptor
import io.element.android.libraries.architecture.AsyncData
Expand Down Expand Up @@ -81,7 +82,7 @@ internal fun CallScreenView(
title = { Text(stringResource(R.string.element_call)) },
navigationIcon = {
BackButton(
imageVector = CompoundIcons.Close(),
imageVector = if (pipState.supportPip) CompoundIcons.ArrowLeft() else CompoundIcons.Close(),
onClick = ::handleBack,
)
}
Expand Down Expand Up @@ -195,3 +196,15 @@ internal fun CallScreenViewPreview(
requestPermissions = { _, _ -> },
)
}

@PreviewsDayNight
@Composable
internal fun CallScreenPipViewPreview(
@PreviewParameter(PictureInPictureStateProvider::class) state: PictureInPictureState,
) = ElementPreview {
CallScreenView(
state = aCallScreenState(),
pipState = state,
requestPermissions = { _, _ -> },
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import androidx.appcompat.app.AppCompatActivity
import androidx.compose.runtime.mutableStateOf
import androidx.core.content.IntentCompat
import androidx.lifecycle.Lifecycle
import io.element.android.features.call.api.CallType
import io.element.android.features.call.impl.DefaultElementCallEntryPoint
import io.element.android.features.call.impl.di.CallBindings
Expand All @@ -41,6 +42,7 @@
import io.element.android.libraries.architecture.bindings
import io.element.android.libraries.designsystem.theme.ElementThemeApp
import io.element.android.libraries.preferences.api.store.AppPreferencesStore
import timber.log.Timber
import javax.inject.Inject

class ElementCallActivity : AppCompatActivity(), CallScreenNavigator {
Expand All @@ -63,6 +65,8 @@
private var isDarkMode = false
private val webViewTarget = mutableStateOf<CallType?>(null)

private var eventSink: ((CallScreenEvents) -> Unit)? = null

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

Expand Down Expand Up @@ -91,6 +95,7 @@
val pipState = pictureInPicturePresenter.present()
ElementThemeApp(appPreferencesStore) {
val state = presenter.present()
eventSink = state.eventSink

Check warning on line 98 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt#L98

Added line #L98 was not covered by tests
CallScreenView(
state = state,
pipState = pipState,
Expand All @@ -111,6 +116,11 @@
override fun onPictureInPictureModeChanged(isInPictureInPictureMode: Boolean, newConfig: Configuration) {
super.onPictureInPictureModeChanged(isInPictureInPictureMode, newConfig)
pictureInPicturePresenter.onPictureInPictureModeChanged(isInPictureInPictureMode)

if (!isInPictureInPictureMode && !lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED)) {
Timber.d("Exiting PiP mode: Hangup the call")

Check warning on line 121 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt#L121

Added line #L121 was not covered by tests
eventSink?.invoke(CallScreenEvents.Hangup)
}
}

override fun onNewIntent(intent: Intent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,11 @@ enum class FeatureFlags(
defaultValue = { true },
isFinished = false,
),
PictureInPicture(
key = "feature.pictureInPicture",
title = "Picture in Picture for Calls",
description = "Allow the Call to be rendered in PiP mode",
defaultValue = { it.buildType != BuildType.RELEASE },
isFinished = false,
),
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class KonsistPreviewTest {
"AsyncIndicatorLoadingPreview",
"BloomInitialsPreview",
"BloomPreview",
"CallScreenPipViewPreview",
"ColorAliasesPreview",
"DefaultRoomListTopBarWithIndicatorPreview",
"GradientFloatingActionButtonCircleShapePreview",
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading