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

Update custom view width calculation for TextViews #686

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Update custom view width calculation for TextViews #686

merged 1 commit into from
Aug 20, 2024

Conversation

CzarMatt
Copy link

Overview

Resolved missing horizontal padding when calculating width for custom layouts.

Details

When showing a Balloon with a custom layout, the custom view tree is walked and checked for instances of TextView.

Any TextView widgets, and by extension, descendants of TextView (e.g., Button), has its text measured for calculating width.

The measurement heuristics adds horizontal padding while using compound drawables, but not otherwise. This works great when a custom layout's TextView has no horizontal padding, but constrains text when horizontal padding is present.

Since padding is inside the bounds of the TextView, we would like the resulting maxWidth calculation to contain this extra space information to render the full text.

Here are some example screenshots showing the behavior with and without horizontal padding, before and after this PR's changes:

Before changes with zero horizontal padding

Before changes with 10dp paddingHorizontal

After changes with 10dp paddingHorizontal


Reproduction steps

  1. Create a new project and surface a Balloon via:
    val balloon = Balloon.Builder(context)
      .setLayout(R.layout.custom_layout)
      .build()
    
    balloon.showAlignBottom(someAnchorView)

where custom_layout is:

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
  android:layout_width="match_parent"
  android:layout_height="wrap_content">

  <TextView
    android:id="@+id/textview"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:ellipsize="end"
    android:maxLines="1"
    android:paddingHorizontal="20dp"
    android:text="Hello, there!" />

</LinearLayout>

Note: can also use padingLeft/paddingStart and paddingRight/paddingEnd to demonstrate the issue.

Resolution

Add overall horizontal compound padding to TextView's maxWidth measurement always. Also updated the Balloon#measureTextWidth kdoc.

Copy link
Owner

@skydoves skydoves left a comment

Choose a reason for hiding this comment

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

Looks very good to me! Thank you so much for your contribution 👍

@skydoves skydoves merged commit c459890 into skydoves:main Aug 20, 2024
3 checks passed
@CzarMatt CzarMatt deleted the mmossman/balloon/custom_layout_width_bug branch August 20, 2024 00:47
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