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

Added support for VectorDrawable #19

Merged
merged 7 commits into from
Sep 14, 2019

Conversation

amaslanka
Copy link
Contributor

Added checking drawable type in SegmentedButton. If the drawable is Vector, it will be converted into a bitmap. This allows to handle drawable tint/selected tint properly.

@addisonElliott
Copy link
Owner

Hello, thanks for the PR!

I'll hopefully spend some time checking this out in more detail to fully understand it. But, do you mind adding a "unit test" to demonstrate this feature? Essentially, just add another item to the sample activity that shows it working.

In addition, I would recommend testing the feature on a few different Android versions (I usually do API19, API21, and API26 or 27) just to be sure.

…works with vector drawable. Tested on API 19, 21, 27 & 28.
@amaslanka
Copy link
Contributor Author

Hi!

Take a look at the example which I provided in the sample activity. It's tested on different API versions.

@addisonElliott
Copy link
Owner

Can you explain why you took VectorDrawable and converted it into a BitmapDrawable?

I changed the code to just return a VectorDrawable and the sample works out to be the same. Tinting seems to be working fine with just the VectorDrawable. (Tested on API 19 & 21)

image

Everything seems to work with just utilizing the VectorDrawable
android:id="@+id/buttonGroup_vectorDrawable"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_margin="4dp"
Copy link
Owner

Choose a reason for hiding this comment

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

Oh yeah, the margin bottom will need to be 60dp here.

This was just so I could move the button to be up higher on the device screen for some reason.

@amaslanka
Copy link
Contributor Author

The bug appears on API 28. The selectedDrawableTint of SegmentedButton is always applied to its drawable if the drawable type is VectorDrawable. Take a look at the screenshot.

Zrzut ekranu 2019-09-14 o 23 20 38

After converting VectorDrawable to BitmapDrawable the tint is working fine on API 28.
On lower API versions the bug doesn't appear at all.

@addisonElliott
Copy link
Owner

Wow, what a crazy bug. It's gone on API 29, so I made adjustments to the code to only convert from vector to bitmap in API 28. My thought process is that keeping the vector as a vector drawable is more efficient (in terms of memory) than converting to a bitmap. Although, in the end, the performance/memory differences is likely minimal.

Everything looks good! I'll add the new sample GIF and release a new version. Thanks

@addisonElliott addisonElliott merged commit 9f7ce70 into addisonElliott:master Sep 14, 2019
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