Skip to content

Fix modal contents overlapping screen lock pin #2692 #2874

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

Merged
merged 4 commits into from
May 21, 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
44 changes: 26 additions & 18 deletions app/src/main/kotlin/io/element/android/x/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,26 @@
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalUriHandler
import androidx.core.splashscreen.SplashScreen.Companion.installSplashScreen
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import com.bumble.appyx.core.integration.NodeHost
import com.bumble.appyx.core.integrationpoint.NodeActivity
import com.bumble.appyx.core.plugin.NodeReadyObserver
import io.element.android.compound.theme.ElementTheme
import io.element.android.compound.theme.Theme
import io.element.android.compound.theme.isDark
import io.element.android.compound.theme.mapToTheme
import io.element.android.features.lockscreen.api.LockScreenEntryPoint
import io.element.android.features.lockscreen.api.LockScreenLockState
import io.element.android.features.lockscreen.api.LockScreenService
import io.element.android.features.lockscreen.api.handleSecureFlag
import io.element.android.features.lockscreen.api.isLocked
import io.element.android.libraries.architecture.bindings
import io.element.android.libraries.core.log.logger.LoggerTag
import io.element.android.libraries.designsystem.utils.snackbar.LocalSnackbarDispatcher
import io.element.android.x.di.AppBindings
import io.element.android.x.intent.SafeUriHandler
import kotlinx.coroutines.launch
import timber.log.Timber

private val loggerTag = LoggerTag("MainActivity")
Expand All @@ -59,27 +65,13 @@
installSplashScreen()
super.onCreate(savedInstanceState)
appBindings = bindings()
appBindings.lockScreenService().handleSecureFlag(this)
setupLockManagement(appBindings.lockScreenService(), appBindings.lockScreenEntryPoint())

Check warning on line 68 in app/src/main/kotlin/io/element/android/x/MainActivity.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/kotlin/io/element/android/x/MainActivity.kt#L68

Added line #L68 was not covered by tests
enableEdgeToEdge()
setContent {
MainContent(appBindings)
}
}

@Deprecated("")
override fun onBackPressed() {
// If the app is locked, we need to intercept onBackPressed before it goes to OnBackPressedDispatcher.
// Indeed, otherwise we would need to trick Appyx backstack management everywhere.
// Without this trick, we would get pop operations on the hidden backstack.
if (appBindings.lockScreenService().isLocked) {
// Do not kill the app in this case, just go to background.
moveTaskToBack(false)
} else {
@Suppress("DEPRECATION")
super.onBackPressed()
}
}

@Composable
private fun MainContent(appBindings: AppBindings) {
val theme by remember {
Expand All @@ -96,8 +88,8 @@
) {
Box(
modifier = Modifier
.fillMaxSize()
.background(MaterialTheme.colorScheme.background),
.fillMaxSize()
.background(MaterialTheme.colorScheme.background),

Check warning on line 92 in app/src/main/kotlin/io/element/android/x/MainActivity.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/kotlin/io/element/android/x/MainActivity.kt#L91-L92

Added lines #L91 - L92 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

We need to find a solution to avoid such change

) {
if (migrationState.migrationAction.isSuccess()) {
MainNodeHost()
Expand Down Expand Up @@ -131,6 +123,22 @@
}
}

private fun setupLockManagement(
lockScreenService: LockScreenService,
lockScreenEntryPoint: LockScreenEntryPoint
) {
lockScreenService.handleSecureFlag(this)
lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.RESUMED) {
lockScreenService.lockState.collect { state ->

Check warning on line 133 in app/src/main/kotlin/io/element/android/x/MainActivity.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/kotlin/io/element/android/x/MainActivity.kt#L130-L133

Added lines #L130 - L133 were not covered by tests
if (state == LockScreenLockState.Locked) {
startActivity(lockScreenEntryPoint.pinUnlockIntent(this@MainActivity))

Check warning on line 135 in app/src/main/kotlin/io/element/android/x/MainActivity.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/kotlin/io/element/android/x/MainActivity.kt#L135

Added line #L135 was not covered by tests
}
Copy link
Member

Choose a reason for hiding this comment

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

This is working, but can we try to use only things from the api module?
Maybe expose a getIntent(): Intent? api and do nothing when null is returned?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's ok for me to use impl in the app module as it's the glue, but I can still expose something in the EntryPoint if u prefer.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

}
}
}
}

/**
* Called when:
* - the launcher icon is clicked (if the app is already running);
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/kotlin/io/element/android/x/di/AppBindings.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package io.element.android.x.di

import com.squareup.anvil.annotations.ContributesTo
import io.element.android.features.api.MigrationEntryPoint
import io.element.android.features.lockscreen.api.LockScreenEntryPoint
import io.element.android.features.lockscreen.api.LockScreenService
import io.element.android.features.preferences.api.store.AppPreferencesStore
import io.element.android.features.rageshake.api.reporter.BugReporter
Expand All @@ -38,4 +39,6 @@ interface AppBindings {
fun preferencesStore(): AppPreferencesStore

fun migrationEntryPoint(): MigrationEntryPoint

fun lockScreenEntryPoint(): LockScreenEntryPoint
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ import io.element.android.features.createroom.api.CreateRoomEntryPoint
import io.element.android.features.ftue.api.FtueEntryPoint
import io.element.android.features.ftue.api.state.FtueService
import io.element.android.features.ftue.api.state.FtueState
import io.element.android.features.lockscreen.api.LockScreenEntryPoint
import io.element.android.features.lockscreen.api.LockScreenLockState
import io.element.android.features.lockscreen.api.LockScreenService
import io.element.android.features.networkmonitor.api.NetworkMonitor
import io.element.android.features.networkmonitor.api.NetworkStatus
import io.element.android.features.preferences.api.PreferencesEntryPoint
Expand Down Expand Up @@ -100,8 +97,6 @@ class LoggedInFlowNode @AssistedInject constructor(
private val coroutineScope: CoroutineScope,
private val networkMonitor: NetworkMonitor,
private val ftueService: FtueService,
private val lockScreenEntryPoint: LockScreenEntryPoint,
private val lockScreenStateService: LockScreenService,
private val roomDirectoryEntryPoint: RoomDirectoryEntryPoint,
private val matrixClient: MatrixClient,
snackbarDispatcher: SnackbarDispatcher,
Expand All @@ -111,7 +106,7 @@ class LoggedInFlowNode @AssistedInject constructor(
savedStateMap = buildContext.savedStateMap,
),
permanentNavModel = PermanentNavModel(
navTargets = setOf(NavTarget.LoggedInPermanent, NavTarget.LockPermanent),
navTargets = setOf(NavTarget.LoggedInPermanent),
savedStateMap = buildContext.savedStateMap,
),
buildContext = buildContext,
Expand Down Expand Up @@ -189,9 +184,6 @@ class LoggedInFlowNode @AssistedInject constructor(
@Parcelize
data object LoggedInPermanent : NavTarget

@Parcelize
data object LockPermanent : NavTarget

@Parcelize
data object RoomList : NavTarget

Expand Down Expand Up @@ -235,11 +227,6 @@ class LoggedInFlowNode @AssistedInject constructor(
NavTarget.LoggedInPermanent -> {
createNode<LoggedInNode>(buildContext)
}
NavTarget.LockPermanent -> {
lockScreenEntryPoint.nodeBuilder(this, buildContext)
.target(LockScreenEntryPoint.Target.Unlock)
.build()
}
Copy link
Member

Choose a reason for hiding this comment

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

I think that Target.Unlock can be removed, since it's not used anymore.
Same for LockScreenFlowNode.NavTarget.Unlock and PinUnlockNode.Inputs since isInAppUnlockwill always be false.

NavTarget.RoomList -> {
val callback = object : RoomListEntryPoint.Callback {
override fun onRoomClicked(roomId: RoomId) {
Expand Down Expand Up @@ -430,15 +417,11 @@ class LoggedInFlowNode @AssistedInject constructor(
@Composable
override fun View(modifier: Modifier) {
Box(modifier = modifier) {
val lockScreenState by lockScreenStateService.lockState.collectAsState()
val ftueState by ftueService.state.collectAsState()
BackstackView()
if (ftueState is FtueState.Complete) {
PermanentChild(permanentNavModel = permanentNavModel, navTarget = NavTarget.LoggedInPermanent)
}
if (lockScreenState == LockScreenLockState.Locked) {
PermanentChild(permanentNavModel = permanentNavModel, navTarget = NavTarget.LockPermanent)
}
}
}

Expand Down
1 change: 1 addition & 0 deletions changelog.d/2692.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix modal contents overlapping screen lock pin.
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,8 @@ class FtueFlowNode @AssistedInject constructor(
lifecycleScope.launch { moveToNextStep() }
}
}
lockScreenEntryPoint.nodeBuilder(this, buildContext)
lockScreenEntryPoint.nodeBuilder(this, buildContext, LockScreenEntryPoint.Target.Setup)
.callback(callback)
.target(LockScreenEntryPoint.Target.Setup)
.build()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,19 @@

package io.element.android.features.lockscreen.api

import android.content.Context
import android.content.Intent
import com.bumble.appyx.core.modality.BuildContext
import com.bumble.appyx.core.node.Node
import com.bumble.appyx.core.plugin.Plugin
import io.element.android.libraries.architecture.FeatureEntryPoint

interface LockScreenEntryPoint : FeatureEntryPoint {
fun nodeBuilder(parentNode: Node, buildContext: BuildContext): NodeBuilder
fun nodeBuilder(parentNode: Node, buildContext: BuildContext, navTarget: Target): NodeBuilder
fun pinUnlockIntent(context: Context): Intent

interface NodeBuilder {
fun callback(callback: Callback): NodeBuilder
fun target(target: Target): NodeBuilder
fun build(): Node
}

Expand All @@ -37,6 +39,5 @@ interface LockScreenEntryPoint : FeatureEntryPoint {
enum class Target {
Settings,
Setup,
Unlock
}
}
25 changes: 25 additions & 0 deletions features/lockscreen/impl/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<!--
~ Copyright (c) 2024 New Vector Ltd
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->

<manifest xmlns:android="http://schemas.android.com/apk/res/android">

<application>
<activity
android:name="io.element.android.features.lockscreen.impl.unlock.activity.PinUnlockActivity"
android:configChanges="screenSize|screenLayout|orientation|keyboardHidden|keyboard|navigation|uiMode"/>
</application>

</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,20 @@

package io.element.android.features.lockscreen.impl

import android.content.Context
import android.content.Intent
import com.bumble.appyx.core.modality.BuildContext
import com.bumble.appyx.core.node.Node
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.features.lockscreen.api.LockScreenEntryPoint
import io.element.android.features.lockscreen.impl.unlock.activity.PinUnlockActivity
import io.element.android.libraries.architecture.createNode
import io.element.android.libraries.di.AppScope
import javax.inject.Inject

@ContributesBinding(AppScope::class)
class DefaultLockScreenEntryPoint @Inject constructor() : LockScreenEntryPoint {
override fun nodeBuilder(parentNode: Node, buildContext: BuildContext): LockScreenEntryPoint.NodeBuilder {
var innerTarget: LockScreenEntryPoint.Target = LockScreenEntryPoint.Target.Unlock
override fun nodeBuilder(parentNode: Node, buildContext: BuildContext, navTarget: LockScreenEntryPoint.Target): LockScreenEntryPoint.NodeBuilder {
val callbacks = mutableListOf<LockScreenEntryPoint.Callback>()

return object : LockScreenEntryPoint.NodeBuilder {
Expand All @@ -36,15 +38,9 @@
return this
}

override fun target(target: LockScreenEntryPoint.Target): LockScreenEntryPoint.NodeBuilder {
innerTarget = target
return this
}

override fun build(): Node {
val inputs = LockScreenFlowNode.Inputs(
when (innerTarget) {
LockScreenEntryPoint.Target.Unlock -> LockScreenFlowNode.NavTarget.Unlock
when (navTarget) {
LockScreenEntryPoint.Target.Setup -> LockScreenFlowNode.NavTarget.Setup
LockScreenEntryPoint.Target.Settings -> LockScreenFlowNode.NavTarget.Settings
}
Expand All @@ -54,4 +50,8 @@
}
}
}

override fun pinUnlockIntent(context: Context): Intent {
return PinUnlockActivity.newIntent(context)

Check warning on line 55 in features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/DefaultLockScreenEntryPoint.kt

View check run for this annotation

Codecov / codecov/patch

features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/DefaultLockScreenEntryPoint.kt#L55

Added line #L55 was not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import io.element.android.anvilannotations.ContributesNode
import io.element.android.features.lockscreen.api.LockScreenEntryPoint
import io.element.android.features.lockscreen.impl.settings.LockScreenSettingsFlowNode
import io.element.android.features.lockscreen.impl.setup.LockScreenSetupFlowNode
import io.element.android.features.lockscreen.impl.unlock.PinUnlockNode
import io.element.android.libraries.architecture.BackstackView
import io.element.android.libraries.architecture.BaseFlowNode
import io.element.android.libraries.architecture.NodeInputs
Expand All @@ -44,20 +43,17 @@ class LockScreenFlowNode @AssistedInject constructor(
@Assisted plugins: List<Plugin>,
) : BaseFlowNode<LockScreenFlowNode.NavTarget>(
backstack = BackStack(
initialElement = plugins.filterIsInstance(Inputs::class.java).first().initialNavTarget,
initialElement = plugins.filterIsInstance<Inputs>().first().initialNavTarget,
savedStateMap = buildContext.savedStateMap,
),
buildContext = buildContext,
plugins = plugins,
) {
data class Inputs(
val initialNavTarget: NavTarget = NavTarget.Unlock,
val initialNavTarget: NavTarget,
) : NodeInputs

sealed interface NavTarget : Parcelable {
@Parcelize
data object Unlock : NavTarget

@Parcelize
data object Setup : NavTarget

Expand All @@ -75,10 +71,6 @@ class LockScreenFlowNode @AssistedInject constructor(

override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
return when (navTarget) {
NavTarget.Unlock -> {
val inputs = PinUnlockNode.Inputs(isInAppUnlock = false)
createNode<PinUnlockNode>(buildContext, plugins = listOf(inputs))
}
NavTarget.Setup -> {
val callback = OnSetupDoneCallback(plugins())
createNode<LockScreenSetupFlowNode>(buildContext, plugins = listOf(callback))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,12 @@ class LockScreenSettingsFlowNode @AssistedInject constructor(
override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
return when (navTarget) {
NavTarget.Unlock -> {
val inputs = PinUnlockNode.Inputs(isInAppUnlock = true)
val callback = object : PinUnlockNode.Callback {
override fun onUnlock() {
backstack.newRoot(NavTarget.Settings)
}
}
createNode<PinUnlockNode>(buildContext, plugins = listOf(inputs, callback))
createNode<PinUnlockNode>(buildContext, plugins = listOf(callback))
}
NavTarget.SetupPin -> {
createNode<SetupPinNode>(buildContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import com.bumble.appyx.core.plugin.plugins
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import io.element.android.anvilannotations.ContributesNode
import io.element.android.libraries.architecture.NodeInputs
import io.element.android.libraries.architecture.inputs
import io.element.android.libraries.di.SessionScope

@ContributesNode(SessionScope::class)
Expand All @@ -40,12 +38,6 @@ class PinUnlockNode @AssistedInject constructor(
fun onUnlock()
}

data class Inputs(
val isInAppUnlock: Boolean
) : NodeInputs

private val inputs: Inputs = inputs()

private fun onUnlock() {
plugins<Callback>().forEach {
it.onUnlock()
Expand All @@ -62,7 +54,9 @@ class PinUnlockNode @AssistedInject constructor(
}
PinUnlockView(
state = state,
isInAppUnlock = inputs.isInAppUnlock,
// UnlockNode is only used for in-app unlock, so we can safely set isInAppUnlock to true.
// It's set to false in PinUnlockActivity.
isInAppUnlock = true,
modifier = modifier
)
}
Expand Down
Loading
Loading