maxkeppeler / sheets-compose-dialogs

✨ Enhancing Android UIs: A Jetpack Compose Library supporting a wide range of common use-cases with Material Design 3 Dialogs, Popups, and Bottom Sheets. ✨
https://maxkeppeler.github.io/sheets-compose-dialogs/
Apache License 2.0
807 stars 38 forks source link

fix: always set header horizontal padding #81

Closed Mystery00 closed 8 months ago

Mystery00 commented 8 months ago

When use with Header.Default, It

When using OptionView or StateView and setting the Header, there is a problem when handling the header spacing, which causes the header to be too close to the left border, as shown below:

image

Related codes: https://github.com/maxkeppeler/sheets-compose-dialogs/blob/02195faa75f0f35111f93d42fbdabab40670b51d/option/src/main/java/com/maxkeppeler/sheets/option/OptionView.kt#L74-L75 https://github.com/maxkeppeler/sheets-compose-dialogs/blob/02195faa75f0f35111f93d42fbdabab40670b51d/core/src/main/java/com/maxkeppeker/sheets/core/views/base/FrameBase.kt#L95-L100

Therefore, a fixed spacing is forced to be set for the header processing part in FrameBase to resolve this problem.

Follow-up solution: You can try to expose the content spacing setting in the header. Like the following:

abstract class Header {
    open val horizontalPadding: PaddingValues? = null

    /**
     * Standard implementation of a header.
     * @param icon The icon that is displayed above the title..
     * @param title The text that will be set as title.
     */
    data class Default(
        val title: String,
        val icon: IconSource? = null,
        override val horizontalPadding: PaddingValues? = null,
    ) : Header()

    /**
     * Custom implementation of a header.
     */
    data class Custom(
        override val horizontalPadding: PaddingValues? = null,
        val header: @Composable () -> Unit
    ) : Header()
}
Mystery00 commented 8 months ago

Without changing it, when using the list dialog, everything looks fine and looks as expected.

image

maxkeppeler commented 8 months ago

I designed the header without horizontal padding to avoid aligning its content with the padding of other views. This allows users to utilize the full width of the view or dialog for elements like cover images or other content.

Instead of designing the header without horizontal padding, I suggest a simpler approach: passing the default header's padding values to the custom header composable function.

header = Header.Custom { paddingValues -> }

This approach allows the developer to either use the padding values for their view or not. If they choose to use them, the content is automatically aligned horizontally with the other content. If not, it has the full width available for use.

maxkeppeler commented 8 months ago

I will close the PR, see the latest commit for the improvement. It will be in the next release.