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

[ShapeableImageView] Support contentPadding #1871

Closed
wants to merge 2 commits into from

Conversation

drewhamilton
Copy link
Contributor

Closes #1489. Adds contentPadding APIs to ShapeableImageView. This is achieved by communicating the sum of padding and contentPadding as a single value to the superclass (AppCompatImageView), but tracking them separately in this class. This maintains the existing behavior of ShapeableImageView padding, which pads the background, but takes advantage of the fact that AppCompatImageView does not pad the background.

Frankly the internal logic is a nightmare, and hard to wrap my head around even after writing it. But it's the only way I can get it to work while maintaining behavioral compatibility, and it seems to work as expected with some manual testing.

I'm happy to add tests but I can't figure out how they work in this project. Is there something special I need to do to get the IDE to recognize the test sources?

@google-cla google-cla bot added the cla: yes label Nov 14, 2020
@wcshi wcshi closed this in c4f7de1 Nov 17, 2020
@jackthebeast
Copy link

so this was closed for what reason? any way to achieve this?

@drewhamilton
Copy link
Contributor Author

@jackthebeast It was merged in c4f7de1! Via this repo's kinda-weird merge process. I wish someone had mentioned how to add tests first 😬 but I'll take it 😄

@jackthebeast
Copy link

jackthebeast commented Nov 20, 2020

@drewhamilton oh, that's weird, I was expecting a clear "merged" status... Anyway, I tried adding the latest alpha to gradle but couldn't find the new method. do you know when it'll become available?

@drewhamilton drewhamilton deleted the #1489 branch November 20, 2020 10:14
@drewhamilton
Copy link
Contributor Author

No, I don't know the release plans or cadence.

@jackthebeast
Copy link

thanks, I'll keep an eye on it

@jackthebeast
Copy link

jackthebeast commented Nov 30, 2020

@drewhamilton Hi, I saw it has been released. I was giving a try:
<com.google.android.material.imageview.ShapeableImageView android:layout_width="75dp" android:layout_height="75dp" android:background="@drawable/drawable_circle_check_quiz" app:contentPadding="15dp" app:shapeAppearanceOverlay="@style/CircleImageView" android:src="@drawable/ic_generic_user" />

<style name="CircleImageView" parent=""> <item name="cornerFamily">rounded</item> <item name="cornerSize">50%</item> </style>

which correctly adds padding to ic_generic_user. Now, however, ic_generic_user which is a squared vector drawable, is not cut to a circle anymore: example.

Any idea what's going on? did you manage to test such a case before?

@drewhamilton
Copy link
Contributor Author

This is expected; the shape clips the view itself. Any content that does not intersect the edges of the shape of the view is unaffected.

The example you linked was not a consideration when I implemented contentPadding—personally my use case was transparent-background icons. Perhaps manipulating the bitmap itself would be the way to go. Or making the green part of your image a MaterialCardView (with contentPadding) and the grey part a ShapeableImageView (with no contentPadding`). Or of course you could file a new feature request for ShapeableImageView.

@jackthebeast
Copy link

This is expected; the shape clips the view itself. Any content that does not intersect the edges of the shape of the view is unaffected.

I see, I didn't know that.

The example you linked was not a consideration when I implemented contentPadding—personally my use case was transparent-background icons. Perhaps manipulating the bitmap itself would be the way to go. Or making the green part of your image a MaterialCardView (with contentPadding) and the grey part a ShapeableImageView (with no contentPadding`). Or of course you could file a new feature request for ShapeableImageView.

Yeah I was already doing something similar so no big deal, just want to simplify it. I will file a feature request and see if anyone else is interested.

@Aleksandramilova69
Copy link

4100115253233934

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

Successfully merging this pull request may close these issues.

[ShapeableImageView] Padding is applied to background, unlike normal ImageView
3 participants