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 an issue where the primary button got extra wide on configuration changes. #6326

Merged
merged 4 commits into from
Mar 6, 2023

Conversation

jaynewstrom-stripe
Copy link
Collaborator

@jaynewstrom-stripe jaynewstrom-stripe commented Mar 6, 2023

Summary

When causing a configuration change (I did it by changing from light theme to dark theme), the primary button would lose it's margin. This change wraps the primary button in a FrameLayout so the layout params of the PrimaryButton aren't lost.

Motivation

It was a bug!

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

Before After
before screenshot after screenshot

@jaynewstrom-stripe jaynewstrom-stripe marked this pull request as ready for review March 6, 2023 21:54
@jaynewstrom-stripe jaynewstrom-stripe requested review from a team as code owners March 6, 2023 21:54
android:text="@string/stripe_paymentsheet_pay_button_label"
android:visibility="gone" />
android:layout_height="wrap_content">
<!-- Needs to be wrapped in a FrameLayout to prevent layout issues after configuration changes. -->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My guess is it's a compose bug. But I didn't look to far into it since the mitigation was low cost.

I've run into this before, which is why I suspected this and fixed it here. The layout inflator isn't taking into account the root view (from this XML file) layout params into account when inflating the view. An easy way around that is to not have any layout params in the root view, but keep them in the nested view, which is what I did here.

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Till Hellmund <[email protected]>
@jaynewstrom-stripe jaynewstrom-stripe merged commit b2bcb4c into master Mar 6, 2023
@jaynewstrom-stripe jaynewstrom-stripe deleted the jaynewstrom/wide-primary-button branch March 6, 2023 22:38
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.

2 participants