Merge pull request #54 from niscy-eudiw/refactor/ui_fixes

Refactor/UI fixes
eu-digital-identity-wallet · Feb 24, 2025 · f52260b · f52260b
2 parents 2839f84 + b64f44d
commit f52260b
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 93 deletions.
diff --git a/...-ui-sdk/src/main/java/eu/europa/ec/eudi/rqesui/presentation/ui/component/SelectionItem.kt b/...-ui-sdk/src/main/java/eu/europa/ec/eudi/rqesui/presentation/ui/component/SelectionItem.kt
@@ -22,6 +22,7 @@ import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.shape.RoundedCornerShape
+import androidx.compose.material3.HorizontalDivider
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
@@ -40,6 +41,7 @@ import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.HSpacer
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.SIZE_SMALL
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.SPACING_EXTRA_SMALL
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.SPACING_MEDIUM
+import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.VSpacer
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.wrap.WrapCard
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.wrap.WrapIcon
 import eu.europa.ec.eudi.rqesui.presentation.ui.options_selection.Event
@@ -48,6 +50,7 @@ import eu.europa.ec.eudi.rqesui.presentation.ui.options_selection.Event
 internal fun <T : ViewEvent> SelectionItem(
     modifier: Modifier = Modifier,
     selectionItemData: SelectionOptionUi<T>,
+    showDividerAbove: Boolean,
     onClick: ((T) -> Unit),
 ) {
     WrapCard(
@@ -57,6 +60,14 @@ internal fun <T : ViewEvent> SelectionItem(
         shape = RoundedCornerShape(SIZE_SMALL.dp),
         enabled = selectionItemData.enabled
     ) {
+        if (showDividerAbove) {
+            HorizontalDivider(
+                modifier = Modifier.fillMaxWidth(),
+                color = MaterialTheme.colorScheme.outlineVariant
+            )
+            VSpacer.Medium()
+        }
+
         Row(
             modifier = Modifier
                 .fillMaxWidth()
@@ -152,6 +163,7 @@ private fun SelectionItemPreview(
                 leadingIcon = AppIcons.StepOne,
                 trailingIcon = AppIcons.KeyboardArrowRight,
             ),
+            showDividerAbove = false,
             onClick = {}
         )
     }

diff --git a/...src/main/java/eu/europa/ec/eudi/rqesui/presentation/ui/component/content/ContentScreen.kt b/...src/main/java/eu/europa/ec/eudi/rqesui/presentation/ui/component/content/ContentScreen.kt
@@ -24,7 +24,6 @@ import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.RowScope
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
-import androidx.compose.foundation.layout.padding
 import androidx.compose.material3.DropdownMenu
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.material3.FabPosition
@@ -57,7 +56,6 @@ import eu.europa.ec.eudi.rqesui.presentation.ui.component.loader.LoadingIndicato
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.preview.PreviewTheme
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.preview.ThemeModePreviews
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.MAX_TOOLBAR_ACTIONS
-import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.SPACING_EXTRA_SMALL
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.SPACING_SMALL
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.TopSpacing
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.Z_STICKY
@@ -215,48 +213,26 @@ private fun DefaultToolBar(
     keyboardController: SoftwareKeyboardController?,
     toolbarConfig: ToolbarConfig?,
 ) {
-
     val isTitlePresent = !toolbarConfig?.title.isNullOrEmpty()
 
-    val topAppBar: @Composable (
-        modifier: Modifier,
-        title: @Composable () -> Unit,
-        navigationIcon: @Composable () -> Unit,
-        actions: @Composable RowScope.() -> Unit,
-        colors: TopAppBarColors
-    ) -> Unit = if (isTitlePresent) {
-        { modifier, title, navigationIcon, actions, colors ->
-            MediumTopAppBar(
-                modifier = modifier,
-                title = title,
-                navigationIcon = navigationIcon,
-                actions = actions,
-                colors = colors
-            )
-        }
-    } else {
-        { modifier, title, navigationIcon, actions, colors ->
-            TopAppBar(
-                modifier = modifier,
-                title = title,
-                navigationIcon = navigationIcon,
-                actions = actions,
-                colors = colors
-            )
-        }
-    }
-
     val modifier = Modifier
-        .shadow(elevation = SPACING_SMALL.dp)
-        .takeIf { toolbarConfig?.hasShadow == true } ?: Modifier
+        .fillMaxWidth()
+        .then(
+            if (toolbarConfig?.hasShadow == true) {
+                Modifier.shadow(elevation = SPACING_SMALL.dp)
+            } else {
+                Modifier
+            }
+        )
 
     val colors = TopAppBarDefaults.topAppBarColors().copy(
         containerColor = MaterialTheme.colorScheme.surface
     )
 
-    topAppBar(
-        modifier,
-        {
+    AdaptiveTopAppBar(
+        modifier = modifier,
+        hasTitle = isTitlePresent,
+        title = {
             if (isTitlePresent) {
                 Text(
                     text = toolbarConfig.title,
@@ -266,41 +242,68 @@ private fun DefaultToolBar(
                 )
             }
         },
-        {
+        navigationIcon = {
             if (navigatableAction != ScreenNavigateAction.NONE) {
-                val icon = when (navigatableAction) {
+                val navigationIcon = when (navigatableAction) {
                     ScreenNavigateAction.CANCELABLE -> AppIcons.Close
                     else -> AppIcons.ArrowBack
                 }
-                Row(modifier = Modifier.padding(start = SPACING_EXTRA_SMALL.dp)) {
-                    WrapIconButton(
-                        iconData = icon,
+
+                ToolbarIcon(
+                    toolbarAction = ToolbarAction(
+                        icon = navigationIcon,
                         onClick = {
                             onBack?.invoke()
                             keyboardController?.hide()
-                        },
-                        customTint = MaterialTheme.colorScheme.onSurface
+                        }
                     )
-                }
+                )
             }
         },
-        {
+        actions = {
             ToolBarActions(toolBarActions = toolbarConfig?.actions)
         },
-        colors
+        colors = colors
     )
 }
 
+@OptIn(ExperimentalMaterial3Api::class)
+@Composable
+private fun AdaptiveTopAppBar(
+    modifier: Modifier,
+    hasTitle: Boolean,
+    title: @Composable () -> Unit,
+    navigationIcon: @Composable () -> Unit,
+    actions: @Composable RowScope.() -> Unit,
+    colors: TopAppBarColors,
+) {
+    if (hasTitle) {
+        MediumTopAppBar(
+            modifier = modifier,
+            title = title,
+            navigationIcon = navigationIcon,
+            actions = actions,
+            colors = colors
+        )
+    } else {
+        TopAppBar(
+            modifier = modifier,
+            title = title,
+            navigationIcon = navigationIcon,
+            actions = actions,
+            colors = colors
+        )
+    }
+}
+
 @Composable
 private fun ToolBarActions(
     toolBarActions: List<ToolbarAction>?,
     maxActionsShown: Int = MAX_TOOLBAR_ACTIONS,
 ) {
     toolBarActions?.let { actions ->
 
-        var dropDownMenuExpanded by remember {
-            mutableStateOf(false)
-        }
+        var dropDownMenuExpanded by remember { mutableStateOf(false) }
 
         // Show first [MAX_TOOLBAR_ACTIONS] actions.
         actions
@@ -313,12 +316,12 @@ private fun ToolBarActions(
         // Check if there are more actions to show.
         if (actions.size > maxActionsShown) {
             Box {
-                val iconMore = AppIcons.VerticalMore
-                WrapIconButton(
-                    onClick = { dropDownMenuExpanded = !dropDownMenuExpanded },
-                    iconData = iconMore,
-                    enabled = true,
-                    customTint = MaterialTheme.colorScheme.primary
+                ToolbarIcon(
+                    toolbarAction = ToolbarAction(
+                        icon = AppIcons.VerticalMore,
+                        onClick = { dropDownMenuExpanded = !dropDownMenuExpanded },
+                        enabled = true,
+                    )
                 )
                 DropdownMenu(
                     expanded = dropDownMenuExpanded,
@@ -339,7 +342,7 @@ private fun ToolBarActions(
 @Composable
 private fun ToolbarIcon(toolbarAction: ToolbarAction) {
     val customIconTint = toolbarAction.customTint
-        ?: MaterialTheme.colorScheme.primary
+        ?: MaterialTheme.colorScheme.onSurface
 
     if (toolbarAction.clickable) {
         WrapIconButton(

diff --git a/...-ui-sdk/src/main/java/eu/europa/ec/eudi/rqesui/presentation/ui/component/wrap/WrapCard.kt b/...-ui-sdk/src/main/java/eu/europa/ec/eudi/rqesui/presentation/ui/component/wrap/WrapCard.kt
@@ -50,7 +50,6 @@ internal fun WrapCard(
     )
     val cardModifier = Modifier
         .clip(cardShape)
-        .then(modifier)
         .then(
             if (enabled && onClick != null) {
                 when (throttleClicks) {
@@ -64,6 +63,7 @@ internal fun WrapCard(
                 }
             } else Modifier.clickable(enabled = false, onClick = {})
         )
+        .then(modifier)
 
     Card(
         modifier = cardModifier,

diff --git a/...java/eu/europa/ec/eudi/rqesui/presentation/ui/options_selection/OptionsSelectionScreen.kt b/...java/eu/europa/ec/eudi/rqesui/presentation/ui/options_selection/OptionsSelectionScreen.kt
@@ -19,14 +19,13 @@ package eu.europa.ec.eudi.rqesui.presentation.ui.options_selection
 import androidx.compose.animation.AnimatedVisibility
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.PaddingValues
+import androidx.compose.foundation.layout.calculateStartPadding
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.rememberScrollState
 import androidx.compose.foundation.verticalScroll
 import androidx.compose.material3.ExperimentalMaterial3Api
-import androidx.compose.material3.HorizontalDivider
-import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.SheetState
 import androidx.compose.material3.rememberModalBottomSheetState
 import androidx.compose.runtime.Composable
@@ -35,6 +34,7 @@ import androidx.compose.runtime.getValue
 import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.platform.LocalContext
+import androidx.compose.ui.platform.LocalLayoutDirection
 import androidx.compose.ui.unit.dp
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import androidx.navigation.NavController
@@ -56,7 +56,6 @@ import eu.europa.ec.eudi.rqesui.presentation.ui.component.preview.PreviewTheme
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.preview.ThemeModePreviews
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.OneTimeLaunchedEffect
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.SPACING_LARGE
-import eu.europa.ec.eudi.rqesui.presentation.ui.component.utils.VSpacer
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.wrap.BottomSheetTextData
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.wrap.BottomSheetWithOptionsList
 import eu.europa.ec.eudi.rqesui.presentation.ui.component.wrap.DialogBottomSheet
@@ -161,27 +160,37 @@ private fun Content(
     val context = LocalContext.current
     val coroutineScope = rememberCoroutineScope()
     val scrollState = rememberScrollState()
+    val innerComponentsModifier = Modifier
+        .fillMaxWidth()
+        .padding(horizontal = paddingValues.calculateStartPadding(LocalLayoutDirection.current))
 
     Column(
         modifier = Modifier
             .fillMaxSize()
-            .padding(paddingValues)
+            .padding(
+                start = 0.dp,
+                end = 0.dp,
+                top = paddingValues.calculateTopPadding(),
+                bottom = paddingValues.calculateBottomPadding()
+            )
             .verticalScroll(scrollState)
     ) {
 
         safeLet(
             state.documentSelectionItem,
             state.documentSelectionItem?.event
         ) { safeSelectionItem, selectionItemEvent ->
-            SelectionItemWithDivider(
+            SelectionItem(
+                modifier = innerComponentsModifier,
                 selectionItemData = safeSelectionItem,
                 showDividerAbove = false,
                 onClick = onEventSend,
             )
         }
 
         state.qtspServiceSelectionItem?.let { safeSelectionItem ->
-            SelectionItemWithDivider(
+            SelectionItem(
+                modifier = innerComponentsModifier,
                 selectionItemData = safeSelectionItem,
                 showDividerAbove = true,
                 onClick = onEventSend,
@@ -190,7 +199,8 @@ private fun Content(
 
         AnimatedVisibility(visible = state.certificateDataList.isNotEmpty()) {
             state.certificateSelectionItem?.let { safeSelectionItem ->
-                SelectionItemWithDivider(
+                SelectionItem(
+                    modifier = innerComponentsModifier,
                     selectionItemData = safeSelectionItem,
                     showDividerAbove = true,
                     onClick = onEventSend,
@@ -306,35 +316,6 @@ private fun OptionsSelectionSheetContent(
     }
 }
 
-@Composable
-private fun <T : Event> SelectionItemWithDivider(
-    selectionItemData: SelectionOptionUi<T>,
-    showDividerAbove: Boolean,
-    onClick: (T) -> Unit,
-) {
-    Column(modifier = Modifier.fillMaxWidth()) {
-        if (showDividerAbove) {
-            ListDivider()
-            VSpacer.Medium()
-        }
-
-        SelectionItem(
-            modifier = Modifier.fillMaxWidth(),
-            selectionItemData = selectionItemData,
-            onClick = onClick
-        )
-    }
-}
-
-
-@Composable
-private fun ListDivider() {
-    HorizontalDivider(
-        modifier = Modifier.fillMaxWidth(),
-        color = MaterialTheme.colorScheme.outlineVariant
-    )
-}
-
 @OptIn(ExperimentalMaterial3Api::class)
 @ThemeModePreviews
 @Composable