Material-components-android: ChipGroup - vertical spacing not working correctly

[johncazier]

Description:
After updating from 1.1.0-beta01 to 1.1.0 I noticed that the vertical spacing between chips in a chip group was suddenly much larger. I was using setChipSpacingResource with 8dp, so I tried separating that out into setChipSpacingHorizontalResource and setChipSpacingVerticalResource. Even with the vertical spacing set to 0dp it was still including a lot of vertical space. I ended up needing to set the vertical resource to -8dp in order to get it to look the same as it did before the update. The bug still exists on the latest alpha04.

Expected behavior:
I would expect vertical spacing of 0dp to have the chips touching each other

Source code:
ChipGroupVerticalSpacingBug.zip

Android API version:
29

Material Library version:
1.1.0, 1.2.0-alpha04

Device:
Pixel 2, Pixel 3 emulator

[johncazier]

I just tried different versions and it looks like it was introduced in version 1.1.0-rc01

[johncazier]

I checked out the code and found that if I added back in the "if (attrs == null) { return; }" into Chip.java initMinTouchTarget that it fixes the bug... not sure why that null check was removed in rc01?

[wcshi]

Hi @johncazier the change you sited (https://github.com/material-components/material-components-android/commit/39e62ec2203e69b9e5b146b51c6e681e54c680a9#diff-517b28ba515355e7ccfece81507a8cc1) was made in order to support a chip being programmatically created (i.e. no view inflation and therefore null attribute set) that has a custom min touch target value in its context's theme.

Did you turn off ensureMinTouchTargetSize? If not, it is true by default and may be the reason why your chips still have spacing in between.

[johncazier]

Setting ensureMinTouchTargetSize to false does fix the problem, thanks. I would expect ensureMinTouchTargetSize to affect the size of the chip, but why does it affect the margin? And if it should affect the margin, why does it only affect the vertical margin and not the horizontal margin?

[wcshi]

ensureMinTouchTargetSize affects both the vertical and horizontal margin of a chip to ensure that it is at least 48x48dp, your chips are probably longer than 48 dp already.

[johncazier]

That makes sense, thanks!