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

Balloon does not adapt to the height of a custom View on certain devices #171

Closed
cooltey opened this issue Mar 23, 2021 · 13 comments
Closed
Assignees
Labels
bug Something isn't working Released Released already on the latest version.

Comments

@cooltey
Copy link

cooltey commented Mar 23, 2021

Please complete the following information:

  • Library Version 1.3.3
  • Affected Device(s) Pixel 3 XL with Android 11 and Pixel 2 with Android 10

Describe the Bug:

Addressed here:
#148 (comment)

The previous fix works fine on Samsung S9 with Android 10, but not on Pixel 3 XL and Pixel 2 and possibly some other devices.

Expected Behavior:

Content should not be cutoff.

@skydoves
Copy link
Owner

skydoves commented Mar 24, 2021

Hi, @cooltey again.
Hmm, I guess it caused by the drawableStartCompat. If there is a TextView which is a single line and has a drawable (If the drawable's height is bigger than the TextView's height), the measured height will be adopted by TextView's one.
Can I see the full code of XML and Balloon.Builder related to this issue?

@skydoves
Copy link
Owner

1.3.4-SNAPSHOT is released!
Please try to use this snapshot.

@skydoves skydoves self-assigned this Mar 24, 2021
@skydoves skydoves added the bug Something isn't working label Mar 24, 2021
@cooltey
Copy link
Author

cooltey commented Mar 24, 2021

Hi, @cooltey again.
Hmm, I guess it caused by the drawableStartCompat. If there is a TextView which is a single line and has a drawable (If the drawable's height is bigger than the TextView's height), the measured height will be adopted by TextView's one.
Can I see the full code of XML and Balloon.Builder related to this issue?

Yes.
XML: https://github.com/wikimedia/apps-android-wikipedia/blob/master/app/src/main/res/layout/view_watchlist_page_tooltip.xml
Code: https://github.com/wikimedia/apps-android-wikipedia/blob/master/app/src/main/java/org/wikipedia/util/FeedbackUtil.kt#L217-L234

I'll try the SNAPSHOT version to see if it fixes the issue, thank you.

@skydoves
Copy link
Owner

Is it possible to use the VectorTextView from this library instead of the MaterialTextView?
And we have to use the app:drawableLeft attribute for putting the drawable with the text.

<com.skydoves.balloon.vectortext.VectorTextView
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:fontFamily="sans-serif-medium"
    android:text="@string/page_watchlist_overflow_menu_onboarding_tooltip_title"
    android:textAllCaps="true"
    android:textSize="14sp"
    android:textColor="@android:color/white"
    android:letterSpacing="0.04"
    android:layout_marginBottom="4dp"
    android:gravity="center_vertical"
    android:drawablePadding="8dp"
    app:drawableLeft="@drawable/ic_baseline_star_outline_24"
    app:drawableTint="@android:color/white"/>

@cooltey
Copy link
Author

cooltey commented Mar 25, 2021

The SNAPSHOT version works great on Samsung S9 and Pixel 3XL, but I am still waiting for my colleague to report whether the version works fine on other devices as well. If that didn't work, I'll follow your suggestion to use the VectorTextView to see if it helps.
wikimedia/apps-android-wikipedia#2247

@skydoves
Copy link
Owner

Thanks for checking!
But I highly recommend using the VectorTextView because it measures the size of the drawable.

@cooltey
Copy link
Author

cooltey commented Mar 26, 2021

Hi @skydoves
I've tried to use VectorTextView with the padding='16dp' on its parent view, turns out it will have a huge space on the bottom of the content. If I change to paddingTop='16dp' paddingStart='16dp' paddingEnd='8dp', some other devices will have the cutoff issue.

Also, the other test from my colleague, the bottom of the content has a slightly cutoff:
wikimedia/apps-android-wikipedia#2247 (comment)

@skydoves
Copy link
Owner

Hi, @cooltey

I've created a new branch for testing this issue, and I just checked about the VectorTextView using a lot of devices. Could you please check again on your device using this branch? 🙏

VectorTextVIew

vector

MaterialTextView and AppCompatTextView

non-vector

@cooltey
Copy link
Author

cooltey commented Mar 31, 2021

Hi @skydoves
Thanks for creating the test branch, I really appreciate it.
Simply change to the VectorTextView with AppCompatTextView does not solve the issue in the app, however, I added android:minHeight="24dp" to each VectorTextView to make sure they reserve the height of the drawable, and turned out it worked.

Also, I removed the setWidthRatio(if (isLandscape(context)) 0.4f else 0.8f) and replaced with BalloonSizeSpec.WRAP on width and height, and now the content looks great on multiple devices.

Now it looks good to me and I'll let you know when getting the test results from my colleagues. 👍

Thank you!

@skydoves
Copy link
Owner

skydoves commented Apr 1, 2021

Hi @cooltey
Wow.. that's awesome. I was thinking that measuring the single line textView and calculating it.
But simply can be resolved using android:minHeight. 😮

@skydoves skydoves added the Release Next This feature will be released on next version label Apr 2, 2021
@cooltey
Copy link
Author

cooltey commented Apr 2, 2021

Hi @skydoves
I have confirmed that the issue has been tested and resolved on multiple devices. Thanks for your help and really appreciate it.

@skydoves
Copy link
Owner

skydoves commented Apr 3, 2021

Hi, @cooltey
Thanks for your confirmation. 🙏
A new stable version 1.3.4 has been released!

@skydoves skydoves added Released Released already on the latest version. and removed Release Next This feature will be released on next version labels Apr 3, 2021
@cooltey
Copy link
Author

cooltey commented Apr 4, 2021

Thank you!

@skydoves skydoves closed this as completed Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Released Released already on the latest version.
Projects
None yet
Development

No branches or pull requests

2 participants