[MaterialSwitch] Amendments to thumb icon size support

[pubiqq]

This PR:

• Adds custom thumb icon size support for API < 23.
• Replaces thumbIconWidth and thumbIconHeight with thumbIconSize (as in other components such as MaterialButton, FloatingActionButton, Chip, Card, and so on).
• Adds a default thumb icon size (16dp, according to specs).

[drchen]

Thanks for providing a solution on API < 23.

For other parts of the change:

1. I don't think only supporting size makes sense, especially considering the provided icon can actually have different width/height. Setting the value separately provides client more flexibility.

2. Unfortunately changing the default size to 16dp breaks the existing usages - for backward compatibility we need to keep using intrinsic sizes as the default.

[pubiqq]

1. I wonder why it suddenly bothered your team? 🤣 Well, if you wish, I can keep the width and height attributes, but the size should stay too, by analogy with:

• BottomNavigationView (app:itemIconSize)
• Chip (app:chipIconSize, app:closeIconSize)
• FloatingActionButton (app:iconSize)
• MaterialButton (app:iconSize)
• MaterialCard (app:checkedIconSize)
• NavigationRail (app:itemIconSize)
• NavigationView (app:itemIconSize)

Will that suit your team?

2. Umm... it's a bugfix. The component was out of spec and this change fixes that. Your team is constantly making changes like this, including at the cost of "breaking the existing usage".
   I would still consider adopting this change, especially since it's unlikely to affect many users.

[drchen]

1. I guess the point is what API makes more sense? (And let history be history...)
2. I won't call it a "bug fix". : ) We don't really have a default implementation of the switch icon, and if we do, it will be 16dp in w/h according to the spec. For clients' implementation, I don't feel it makes too much sense to set a fixed global default size and scale whatever drawables provided by clients. It won't create a good UX anyways in a lot of cases.

[pubiqq]

I'm afraid I have no idea what you mean at all.

---

I guess the point is what API makes more sense?

Both ways make sense. I would just like to keep the naming uniformity, and that's why I would like to keep *iconSize attribute. If you also want to have separate attributes for thumb icon width and height, I can keep them too.

We don't really have a default implementation of the switch icon ...

We have an implementation of the switch icon and it's already supported by MaterialSwitch.

... and if we do, it will be 16dp in w/h according to the spec.

We already have an implementation of the icon and its size doesn't meet the specification. The proposal fixes this flaw.

For clients' implementation, I don't feel it makes too much sense to set a fixed global default size and scale whatever drawables provided by clients.

We have to do this and all material components are doing this, because it's part of the specification. If the client wants to change the icon size, they can use the appropriate attribute or method.

[drchen]

I mean, MaterialSwitch doesn't provide any default icons or any default styles with icons. Also icons on a switch is more like an optional decoration, instead of an integral part of the overall design.

You can imagine that the iconSize you see in the spec is supposed to be applied to the default icons if we are providing any, however, Widget.Material3.CompoundButton.MaterialSwitch doesn't really provide any icons. Any icons provided by clients are "customized", and I don't think it makes sense to enforce a default size on customized drawables.

And - I think separating width/height makes more sense than a single size attribute because it provides better flexibility. And if we have width/height, it doesn't make sense to have size as well since it's redundant.

No matter what, it's just my personal opinion. If you still disagree let's involve @dsn5ft to hear more thoughts from a different perspective here. : )

[pubiqq]

You can imagine that the iconSize you see in the spec is supposed to be applied to the default icons if we are providing any, however, Widget.Material3.CompoundButton.MaterialSwitch doesn't really provide any icons.

• Widget.Material3.NavigationRailView
• Widget.Material3.BottomNavigationView
• Widget.Material3.NavigationView
• Widget.Material3.Chip.*
• Widget.Material3.Button
• Widget.Material3.ExtendedFloatingActionButton.*

also don't provide any icons, but do provide default size for them.

I understand your point and agree with it: having thumbIconWidth and thumbIconHeight attributes is indeed more flexible than having only thumbIconSize, and also it's not necessary to impose default styles on icons.
But also, as a consumer, I expect some sort of consistent experience from the library. I expect that since *iconSize is available in buttons, cards, chips, and other components with icons, it should also be available in the switch. And since the default attribute values of all components match the values from the spec, this principle should be followed for the switch as well.

Anyway, yeah, let's get @dsn5ft's opinion.

[hunterstich]

Hi @pubiqq,

I'm filling in for @dsn5ft at the moment. @drchen and I spoke about this and I think we're going to work on getting this in before 1.10 goes to beta. It's a little unfortunate to have to cherry pick this in to our release so late but we agree on thumbIconSize being more consistent, having support for <M, and having a default icon size.

I'm going to try to resolve the breaking changes internally and will get back to you.

Hunter

[paulfthomas]

This requires API >= 23, see https://developer.android.com/reference/android/graphics/drawable/DrawableWrapper so cannot be used here

[pubiqq]

This code is executed only in API >= 23:

material-components-android/lib/java/com/google/android/material/drawable/DrawableUtils.java

Lines 289 to 294 in 58f5439

	if (VERSION.SDK_INT >= VERSION_CODES.M) {
	drawable = new LayerDrawable(new Drawable[] {bottomLayerDrawable, topLayerDrawable});
	drawable.setLayerSize(1, topLayerNewWidth, topLayerNewHeight);
	drawable.setLayerGravity(1, Gravity.CENTER);
	} else {

[paulfthomas]

I'm talking about ScaledDrawableWrapper which extends DrawableWrapper which is only API >= 23, there seems to be a DrawableWrapperCompat though

[pubiqq]

Yes, that was the compat DrawableWrapper:

material-components-android/lib/java/com/google/android/material/drawable/ScaledDrawableWrapper.java

Line 5 in 58f5439

import androidx.appcompat.graphics.drawable.DrawableWrapper;

Anyway, I've rebased the PR and updated this class.