-
Notifications
You must be signed in to change notification settings - Fork 5
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
fix: [ANDROAPP-6744] correct bottom sheet issues and adapt for android 35 #347
base: develop
Are you sure you want to change the base?
Conversation
...gnsystem/src/commonMain/kotlin/org/hisp/dhis/mobile/ui/designsystem/component/BottomSheet.kt
Outdated
Show resolved
Hide resolved
7bb9783
to
d29a16b
Compare
@Composable | ||
fun Legend( | ||
legendData: LegendData, | ||
modifier: Modifier = Modifier, | ||
windowInsets: @Composable () -> WindowInsets = { BottomSheetDefaults.windowInsets }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing new params documentation
@@ -81,6 +87,8 @@ fun OrgBottomSheet( | |||
clearAllButtonText: String = provideStringResource("clear_all"), | |||
doneButtonText: String? = null, | |||
doneButtonIcon: ImageVector = Icons.Filled.Check, | |||
windowInsets: @Composable () -> WindowInsets = { BottomSheetDefaults.windowInsets }, | |||
bottomSheetLowerPadding: Dp = Spacing0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing bottomSheetLowerPadding docs
@@ -157,7 +165,10 @@ fun BottomSheetShell( | |||
subtitle: String? = null, | |||
description: String? = null, | |||
searchQuery: String? = null, | |||
showSectionDivider: Boolean = true, | |||
showTopSectionDivider: Boolean = true, | |||
showBottomSectionDivider: Boolean = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't you create a UIState class like:
data class BottomSheetShellUIState(
val title: String? = null,
val subtitle: String? = null,
val description: String? = null,
val searchQuery: String? = null,
val showTopSectionDivider: Boolean = true,
val showBottomSectionDivider: Boolean = true,
val bottomPadding: Dp = Spacing0,
val headerTextAlignment: TextAlign = TextAlign.Center,
val scrollableContainerMinHeight: Dp = Spacing0,
val scrollableContainerMaxHeight: Dp = InternalSizeValues.Size386,
val animateHeaderOnKeyboardAppearance: Boolean = true
)
Modify the BottomSheet sheel like:
@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun BottomSheetShell(
uiState: BottomSheetShellUIState,
content: @Composable (() -> Unit)? = null,
windowInsets: @Composable () -> WindowInsets = { BottomSheetDefaults.windowInsets },
contentScrollState: ScrollableState = rememberScrollState(),
icon: @Composable (() -> Unit)? = null,
buttonBlock: @Composable (() -> Unit)? = null,
modifier: Modifier = Modifier,
onSearchQueryChanged: ((String) -> Unit)? = null,
onSearch: ((String) -> Unit)? = null,
onDismiss: () -> Unit
) {
}
And then we can use it like:
val uiState = BottomSheetShellUIState(
title = "Title",
subtitle = "Subtitle",
description = "Description",
searchQuery = "Search",
showTopSectionDivider = true,
showBottomSectionDivider = true,
bottomPadding = Spacing0,
headerTextAlignment = TextAlign.Center,
scrollableContainerMinHeight = Spacing0,
scrollableContainerMaxHeight = InternalSizeValues.Size386,
animateHeaderOnKeyboardAppearance = true
)
BottomSheetShell(
uiState = uiState,
content = { },
onDismiss = { }
)
In this way we could add or remove parameters without generating breaking changes. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a great idea, I have updated the PR and usages with this, as well as deprecating the old one to avoid breaking changes
…alues, add new parameters to all internal components that use th BottomSheetShell.
…r bottom sheet, update usages
6e2f39e
to
f23f386
Compare
No description provided.