Skip to content

Make sure HeaderFooterPage can contents be scrolled #4704

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
+52 −11
Open

Make sure HeaderFooterPage can contents be scrolled #4704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
22bd75e
Make sure `HeaderFooterPage` can contents be scrolled
jmartinesp May 12, 2025
9586102
Center `SecureBackupEnterRecoveryKeyView` in screen when the IME is s…
jmartinesp May 12, 2025
af52512
Since `FlowStepPage` can't have a `TopAppBar` title, make it transpar…
jmartinesp May 12, 2025
668838a
Update screenshots
ElementBot May 12, 2025
2932242
Fix the scrollable content not taking the whole available space
jmartinesp May 12, 2025
33db77f
Update screenshots
ElementBot May 12, 2025
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
36 changes: 35 additions & 1 deletion ...n/io/element/android/features/securebackup/impl/enter/SecureBackupEnterRecoveryKeyView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,24 @@

package io.element.android.features.securebackup.impl.enter

import androidx.compose.foundation.ExperimentalFoundationApi
import androidx.compose.foundation.layout.ColumnScope
import androidx.compose.foundation.layout.ExperimentalLayoutApi
import androidx.compose.foundation.layout.WindowInsets
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.isImeVisible
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.relocation.BringIntoViewRequester
import androidx.compose.foundation.relocation.bringIntoViewRequester
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.onFocusChanged
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.PreviewParameter
import androidx.compose.ui.unit.dp
Expand All @@ -25,6 +38,9 @@ import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.designsystem.theme.components.Button
import io.element.android.libraries.ui.strings.CommonStrings
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import kotlin.time.Duration.Companion.milliseconds

@Composable
fun SecureBackupEnterRecoveryKeyView(
Expand Down Expand Up @@ -55,12 +71,30 @@ fun SecureBackupEnterRecoveryKeyView(
}
}

@OptIn(ExperimentalFoundationApi::class, ExperimentalLayoutApi::class)
@Composable
private fun Content(
state: SecureBackupEnterRecoveryKeyState,
) {
val bringIntoViewRequester = remember { BringIntoViewRequester() }
Copy link
Member

Choose a reason for hiding this comment

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

FocusRequester is not enough?

Copy link
Member Author

Choose a reason for hiding this comment

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

That made the textfield visible, but only half of it (the other half was under the IME).

var isFocused by remember { mutableStateOf(false) }
val isImeVisible = WindowInsets.isImeVisible
val coroutineScope = rememberCoroutineScope()
LaunchedEffect(isImeVisible, isFocused) {
// When the keyboard is shown, we want to scroll the text field into view
if (isImeVisible && isFocused) {
coroutineScope.launch {
// Delay to ensure the keyboard is fully shown
delay(100.milliseconds)
bringIntoViewRequester.bringIntoView()
}
}
}
RecoveryKeyView(
modifier = Modifier.padding(top = 52.dp, bottom = 32.dp),
modifier = Modifier
.onFocusChanged { isFocused = it.isFocused }
.bringIntoViewRequester(bringIntoViewRequester)
.padding(top = 52.dp, bottom = 32.dp),
state = state.recoveryKeyViewState,
onClick = null,
onChange = {
Expand Down
3 changes: 3 additions & 0 deletions ...em/src/main/kotlin/io/element/android/libraries/designsystem/atomic/pages/FlowStepPage.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ import androidx.compose.foundation.layout.ColumnScope
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.TopAppBarDefaults
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import io.element.android.compound.theme.ElementTheme
import io.element.android.compound.tokens.generated.CompoundIcons
Expand Down Expand Up @@ -63,6 +65,7 @@ fun FlowStepPage(
}
},
title = {},
colors = TopAppBarDefaults.topAppBarColors(containerColor = Color.Transparent)
)
},
header = {
Expand Down
8 changes: 6 additions & 2 deletions ...rc/main/kotlin/io/element/android/libraries/designsystem/atomic/pages/HeaderFooterPage.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ package io.element.android.libraries.designsystem.atomic.pages

import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.IntrinsicSize
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.calculateEndPadding
import androidx.compose.foundation.layout.calculateStartPadding
import androidx.compose.foundation.layout.consumeWindowInsets
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.imePadding
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.rememberScrollState
Expand Down Expand Up @@ -94,17 +96,19 @@ fun HeaderFooterPage(
.run {
if (isScrollable) {
verticalScroll(rememberScrollState())
// Make sure the scrollable content takes the full available height
.height(IntrinsicSize.Max)
} else {
Modifier
}
}
// Apply insets here so if the content is scrollable it can get below the top app bar if needed
.padding(contentInsetsPadding)
.weight(1f),
.weight(1f, fill = true),
) {
// Header
header()
Box(modifier = Modifier.weight(1f)) {
Box {
content()
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should add some padding between the footer and the content? This looks a bit weird without

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm... maybe, but maybe that should be added for the content on each screen? Otherwise we need to remember we have 10-16.dp here and substract it from whatever we have in the designs.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ganfra just to remind you of this question: should we add a default padding for all screens and then diff with the designs or leave it at 0 and just apply the padding to each content component so it matches the designs 1:1?

}
}
Expand Down
4 changes: 2 additions & 2 deletions ...images/features.securebackup.impl.reset.root_ResetIdentityRootView_Day_0_en.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions ...images/features.securebackup.impl.reset.root_ResetIdentityRootView_Day_1_en.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions ...ages/features.securebackup.impl.reset.root_ResetIdentityRootView_Night_0_en.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions ...ages/features.securebackup.impl.reset.root_ResetIdentityRootView_Night_1_en.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading