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

fix: always set header horizontal padding #81

Closed
wants to merge 1 commit into from

Conversation

Mystery00
Copy link

@Mystery00 Mystery00 commented Jan 28, 2024

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:

// Override content padding, spacing is within the scrollable container for display mode GRID_HORIZONTAL
horizontalContentPadding = PaddingValues(horizontal = 0.dp),

HeaderComponent(
header = header,
contentHorizontalPadding = PaddingValues(
start = horizontalContentPadding.calculateStartPadding(layoutDirection),
end = horizontalContentPadding.calculateEndPadding(layoutDirection),
)

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
Copy link
Author

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

image

@maxkeppeler
Copy link
Owner

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants