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

Fixes main content padding on various screens #2906

Merged
merged 11 commits into from
Dec 6, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import org.smartregister.fhircore.engine.R
import org.smartregister.fhircore.engine.configuration.navigation.NavigationMenuConfig
import org.smartregister.fhircore.engine.ui.theme.GreyTextColor
import org.smartregister.fhircore.engine.util.annotation.PreviewWithBackgroundExcludeGenerated

Expand All @@ -51,9 +52,16 @@ fun RegisterFooter(
previousButtonClickListener: () -> Unit,
nextButtonClickListener: () -> Unit,
modifier: Modifier = Modifier,
fabActions: List<NavigationMenuConfig>? = null,
) {
if (resultCount > 0) {
Row(modifier = modifier.fillMaxWidth().testTag(SEARCH_FOOTER_TAG).padding(bottom = 32.dp)) {
Row(
modifier =
modifier
.fillMaxWidth()
.testTag(SEARCH_FOOTER_TAG)
.padding(bottom = if (!fabActions.isNullOrEmpty() && fabActions.first().visible) 80.dp else 32.dp)
Copy link
Contributor

Choose a reason for hiding this comment

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

Convert these to constants with intuitive names e.g FOOTER_PADDING_WITH_FAB, FOOTER_PADDING_WITHOUT_FAB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should be a constant variable? for both

Copy link
Collaborator

Choose a reason for hiding this comment

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

See my suggested code in the comments. The idea is to define the padding values in 2 separate constants to be reused in all screens.

) {
Box(
modifier = modifier.weight(1f).padding(4.dp).wrapContentWidth(Alignment.Start),
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import androidx.compose.ui.test.performClick
import androidx.navigation.compose.rememberNavController
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import io.mockk.mockk
import io.mockk.spyk
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.asSharedFlow
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.navigation.NavController
import androidx.navigation.testing.TestNavHostController
import androidx.paging.Pager
import androidx.paging.PagingConfig
import androidx.test.core.app.ApplicationProvider
import io.mockk.mockk
import org.junit.Rule
import org.junit.Test
Expand All @@ -34,7 +36,7 @@ import org.smartregister.fhircore.quest.ui.report.measure.screens.SHOW_PROGRESS_
class MeasureReportListScreenTest {

@get:Rule(order = 0) val composeTestRule = createComposeRule()
private val navController: NavController = mockk(relaxUnitFun = true)
private val navController = TestNavHostController(ApplicationProvider.getApplicationContext())
private val dataList =
Pager(PagingConfig(10)) {
MeasureReportPagingSource(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fun ProfileScreen(
LaunchedEffect(Unit) {
snackStateFlow.hookSnackBar(scaffoldState, profileUiState.resourceData, navController)
}

val fabActions = profileUiState.profileConfiguration?.fabActions
Scaffold(
scaffoldState = scaffoldState,
topBar = {
Expand All @@ -118,8 +118,6 @@ fun ProfileScreen(
}
},
floatingActionButton = {
val fabActions = profileUiState.profileConfiguration?.fabActions

if (!fabActions.isNullOrEmpty() && fabActions.first().visible) {
ExtendedFab(
modifier = Modifier.testTag(FAB_BUTTON_TEST_TAG),
Expand Down Expand Up @@ -154,7 +152,10 @@ fun ProfileScreen(
color = MaterialTheme.colors.primary,
)
}
LazyColumn(state = lazyListState) {
LazyColumn(
state = lazyListState,
modifier = Modifier.padding(bottom = if (!fabActions.isNullOrEmpty() && fabActions.first().visible) 80.dp else 32.dp)
Copy link
Contributor

Choose a reason for hiding this comment

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

Convert these to constants with intuitive names e.g FOOTER_PADDING_WITH_FAB, FOOTER_PADDING_WITHOUT_FAB

) {
item(key = profileUiState.resourceData?.baseResourceId) {
ViewRenderer(
viewProperties = profileUiState.profileConfiguration?.views ?: emptyList(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ fun RegisterCardList(
resultCount = pagingItems.itemCount,
currentPage = currentPage.value.plus(1),
pagesCount = registerUiState.pagesCount,
fabActions = registerUiState.registerConfiguration?.fabActions,
previousButtonClickListener = { onEvent(RegisterEvent.MoveToPreviousPage) },
nextButtonClickListener = { onEvent(RegisterEvent.MoveToNextPage) },
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import androidx.navigation.findNavController
import androidx.navigation.fragment.navArgs
import dagger.hilt.android.AndroidEntryPoint
import org.smartregister.fhircore.engine.ui.theme.AppTheme
import org.smartregister.fhircore.quest.ui.profile.ProfileViewModel

@AndroidEntryPoint
class MeasureReportFragment : Fragment() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import androidx.navigation.navArgument
import org.smartregister.fhircore.quest.R
import org.smartregister.fhircore.quest.navigation.MeasureReportNavigationScreen
import org.smartregister.fhircore.quest.navigation.NavigationArg
import org.smartregister.fhircore.quest.ui.profile.ProfileViewModel
import org.smartregister.fhircore.quest.ui.report.measure.screens.MeasureReportListScreen
import org.smartregister.fhircore.quest.ui.report.measure.screens.MeasureReportResultScreen
import org.smartregister.fhircore.quest.ui.report.measure.screens.MeasureReportSubjectsScreen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import androidx.paging.compose.collectAsLazyPagingItems
import kotlinx.coroutines.flow.Flow
import org.smartregister.fhircore.engine.configuration.report.measure.ReportConfiguration
import org.smartregister.fhircore.quest.R
import org.smartregister.fhircore.quest.ui.profile.ProfileUiState
import org.smartregister.fhircore.quest.ui.report.measure.components.MeasureReportRow

@Composable
Expand All @@ -58,7 +59,6 @@ fun MeasureReportListScreen(
showProgressIndicator: Boolean = false,
) {
val lazyReportItems = dataList.collectAsLazyPagingItems().itemSnapshotList.groupBy { it?.module }

Scaffold(
topBar = {
TopAppBar(
Expand Down Expand Up @@ -92,7 +92,13 @@ fun MeasureReportListScreen(
}
}
} else {
LazyColumn(modifier = modifier.background(Color.White).fillMaxSize()) {
LazyColumn(
modifier =
modifier
.background(Color.White)
.fillMaxSize()
.padding(bottom = 32.dp)
) {
lazyReportItems.keys.forEach { key ->
item {
key?.let { it1 ->
Expand Down
Loading