Skip to content

Commit

Permalink
[ButtonGroup] Simplify MaterialButtonToggleGroup's checking logic
Browse files Browse the repository at this point in the history 
1. Consolidates single selected ID and multiple selected IDs to a single selected
   ID set.
2. Separates View states and internal checked states so we can focus and enforce
   policies much easier on only internal states.

PiperOrigin-RevId: 420087718
  • Loading branch information
@drchen @afohrman
drchen authored and afohrman committed Jan 6, 2022
1 parent ff97a68 commit 3db25be
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 96 deletions.
5 changes: 5 additions & 0 deletions lib/java/com/google/android/material/button/MaterialButton.java
View file
Expand Up @@ -1101,6 +1101,11 @@ public void setChecked(boolean checked) {
this.checked = checked;
refreshDrawableState();

// Report checked state change to the parent toggle group, if there is one
if (getParent() instanceof MaterialButtonToggleGroup) {
((MaterialButtonToggleGroup) getParent()).onButtonCheckedStateChanged(this, this.checked);
}

// Avoid infinite recursions if setChecked() is called from a listener
if (broadcasting) {
return;
Expand Down
153 changes: 57 additions & 96 deletions lib/java/com/google/android/material/button/MaterialButtonToggleGroup.java
View file
Expand Up @@ -48,9 +48,12 @@
import com.google.android.material.shape.CornerSize;
import com.google.android.material.shape.ShapeAppearanceModel;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;

Expand Down Expand Up @@ -142,7 +145,6 @@ public interface OnButtonCheckedListener {

private final List<CornerData> originalCornerData = new ArrayList<>();

private final CheckedStateTracker checkedStateTracker = new CheckedStateTracker();
private final PressedStateTracker pressedStateTracker = new PressedStateTracker();
private final LinkedHashSet<OnButtonCheckedListener> onButtonCheckedListeners =
new LinkedHashSet<>();
Expand Down Expand Up @@ -170,7 +172,9 @@ public int compare(MaterialButton v1, MaterialButton v2) {
private boolean singleSelection;
private boolean selectionRequired;

@IdRes private int checkedId;
@IdRes
private final int defaultCheckId;
private Set<Integer> checkedIds = new HashSet<>();

public MaterialButtonToggleGroup(@NonNull Context context) {
this(context, null);
Expand All @@ -191,8 +195,9 @@ public MaterialButtonToggleGroup(

setSingleSelection(
attributes.getBoolean(R.styleable.MaterialButtonToggleGroup_singleSelection, false));
checkedId =
attributes.getResourceId(R.styleable.MaterialButtonToggleGroup_checkedButton, View.NO_ID);
defaultCheckId =
attributes.getResourceId(
R.styleable.MaterialButtonToggleGroup_checkedButton, View.NO_ID);
selectionRequired =
attributes.getBoolean(R.styleable.MaterialButtonToggleGroup_selectionRequired, false);
setChildrenDrawingOrderEnabled(true);
Expand All @@ -204,10 +209,8 @@ public MaterialButtonToggleGroup(
@Override
protected void onFinishInflate() {
super.onFinishInflate();

// Checks the appropriate button as requested via XML
if (checkedId != View.NO_ID) {
checkForced(checkedId, true);
if (defaultCheckId != View.NO_ID) {
updateCheckedIds(Collections.singleton(defaultCheckId));
}
}

Expand All @@ -234,11 +237,8 @@ public void addView(View child, int index, ViewGroup.LayoutParams params) {
// Sets sensible default values and an internal checked change listener for this child
setupButtonChild(buttonChild);

// Reorders children if a checked child was added to this layout
if (buttonChild.isChecked()) {
updateCheckedStates(buttonChild.getId(), true);
setCheckedId(buttonChild.getId());
}
// Update button group's checked states
checkInternal(buttonChild.getId(), buttonChild.isChecked());

// Saves original corner data
ShapeAppearanceModel shapeAppearanceModel = buttonChild.getShapeAppearanceModel();
Expand Down Expand Up @@ -273,7 +273,6 @@ public void onViewRemoved(View child) {
super.onViewRemoved(child);

if (child instanceof MaterialButton) {
((MaterialButton) child).removeOnCheckedChangeListener(checkedStateTracker);
((MaterialButton) child).setOnPressedChangeListenerInternal(null);
}

Expand Down Expand Up @@ -327,11 +326,7 @@ public void onInitializeAccessibilityNodeInfo(@NonNull AccessibilityNodeInfo inf
* @see #getCheckedButtonId()
*/
public void check(@IdRes int id) {
if (id == checkedId) {
return;
}

checkForced(id, true);
checkInternal(id, true);
}

/**
Expand All @@ -344,7 +339,7 @@ public void check(@IdRes int id) {
* @see #getCheckedButtonId()
*/
public void uncheck(@IdRes int id) {
checkForced(id, false);
checkInternal(id, false);
}

/**
Expand All @@ -357,16 +352,7 @@ public void uncheck(@IdRes int id) {
* @see #getCheckedButtonId()
*/
public void clearChecked() {
skipCheckedStateTracker = true;
for (int i = 0; i < getChildCount(); i++) {
MaterialButton child = getChildButton(i);
child.setChecked(false);

dispatchOnButtonChecked(child.getId(), false);
}
skipCheckedStateTracker = false;

setCheckedId(View.NO_ID);
updateCheckedIds(new HashSet<Integer>());
}

/**
Expand All @@ -385,7 +371,7 @@ public void clearChecked() {
*/
@IdRes
public int getCheckedButtonId() {
return singleSelection ? checkedId : View.NO_ID;
return singleSelection && !checkedIds.isEmpty() ? checkedIds.iterator().next() : View.NO_ID;
}

/**
Expand All @@ -402,15 +388,15 @@ public int getCheckedButtonId() {
*/
@NonNull
public List<Integer> getCheckedButtonIds() {
List<Integer> checkedIds = new ArrayList<>();
List<Integer> orderedCheckedIds = new ArrayList<>();
for (int i = 0; i < getChildCount(); i++) {
MaterialButton child = getChildButton(i);
if (child.isChecked()) {
checkedIds.add(child.getId());
int childId = getChildButton(i).getId();
if (checkedIds.contains(childId)) {
orderedCheckedIds.add(childId);
}
}

return checkedIds;
return orderedCheckedIds;
}

/**
Expand Down Expand Up @@ -505,12 +491,6 @@ private void setCheckedStateForView(@IdRes int viewId, boolean checked) {
}
}

private void setCheckedId(int checkedId) {
this.checkedId = checkedId;

dispatchOnButtonChecked(checkedId, true);
}

/**
* Sets a negative marginStart on all but the first child, if two adjacent children both have a
* stroke width greater than 0. This prevents a double-width stroke from being drawn for two
Expand Down Expand Up @@ -686,49 +666,45 @@ private static void updateBuilderWithCornerData(
.setBottomRightCornerSize(cornerData.bottomRight);
}

/**
* When a checked child is added, or a child is clicked, updates checked state and draw order of
* children to draw all checked children on top of all unchecked children.
*
* <p>If {@code singleSelection} is true, this will unselect any other children as well.
*
* <p>If {@code selectionRequired} is true, and the last child is unchecked it will undo the
* deselection.
*
* @param childId ID of child whose checked state may have changed
* @param childIsChecked Whether the child is checked
* @return Whether the checked state for childId has changed.
*/
private boolean updateCheckedStates(int childId, boolean childIsChecked) {
List<Integer> checkedButtonIds = getCheckedButtonIds();
if (selectionRequired && checkedButtonIds.isEmpty()) {
// undo deselection
setCheckedStateForView(childId, true);
checkedId = childId;
return false;
}

// un select previous selection
if (childIsChecked && singleSelection) {
checkedButtonIds.remove((Integer) childId);
for (int buttonId : checkedButtonIds) {
setCheckedStateForView(buttonId, false);
dispatchOnButtonChecked(buttonId, false);
private void checkInternal(@IdRes int buttonId, boolean checked) {
if (buttonId == View.NO_ID) {
Log.e(LOG_TAG, "Button ID is not valid: " + buttonId);
return;
}
Set<Integer> checkedIds = new HashSet<>(this.checkedIds);
if (checked && !checkedIds.contains(buttonId)) {
if (singleSelection && !checkedIds.isEmpty()) {
checkedIds.clear();
}
checkedIds.add(buttonId);
} else if (!checked && checkedIds.contains(buttonId)) {
// Do not uncheck button if no selection remains when selection is required.
if (!selectionRequired || checkedIds.size() > 1) {
checkedIds.remove(buttonId);
}
} else {
// No state change, do nothing
return;
}
return true;
updateCheckedIds(checkedIds);
}

private void dispatchOnButtonChecked(@IdRes int buttonId, boolean checked) {
for (OnButtonCheckedListener listener : onButtonCheckedListeners) {
listener.onButtonChecked(this, buttonId, checked);
private void updateCheckedIds(Set<Integer> checkedIds) {
Set<Integer> previousCheckedIds = this.checkedIds;
this.checkedIds = new HashSet<>(checkedIds);
for (int i = 0; i < getChildCount(); i++) {
int buttonId = getChildButton(i).getId();
setCheckedStateForView(buttonId, checkedIds.contains(buttonId));
if (previousCheckedIds.contains(buttonId) != checkedIds.contains(buttonId)) {
dispatchOnButtonChecked(buttonId, checkedIds.contains(buttonId));
}
}
invalidate();
}

private void checkForced(int checkedId, boolean checked) {
MaterialButton button = findViewById(checkedId);
if (button != null) {
button.setChecked(checked);
private void dispatchOnButtonChecked(@IdRes int buttonId, boolean checked) {
for (OnButtonCheckedListener listener : onButtonCheckedListeners) {
listener.onButtonChecked(this, buttonId, checked);
}
}

Expand All @@ -751,7 +727,6 @@ private void setupButtonChild(@NonNull MaterialButton buttonChild) {
buttonChild.setEllipsize(TruncateAt.END);
buttonChild.setCheckable(true);

buttonChild.addOnCheckedChangeListener(checkedStateTracker);
buttonChild.setOnPressedChangeListenerInternal(pressedStateTracker);

// Enables surface layer drawing for semi-opaque strokes
Expand Down Expand Up @@ -792,26 +767,12 @@ private void updateChildOrder() {
childOrder = viewToIndexMap.values().toArray(new Integer[0]);
}

private class CheckedStateTracker implements MaterialButton.OnCheckedChangeListener {
@Override
public void onCheckedChanged(@NonNull MaterialButton button, boolean isChecked) {
// Prevents infinite recursion
void onButtonCheckedStateChanged(@NonNull MaterialButton button, boolean isChecked) {
// Checked state change is triggered by the button group, do not update checked ids again.
if (skipCheckedStateTracker) {
return;
}

if (singleSelection) {
checkedId = isChecked ? button.getId() : View.NO_ID;
}

boolean buttonCheckedStateChanged = updateCheckedStates(button.getId(), isChecked);
if (buttonCheckedStateChanged) {
// Dispatch button.isChecked instead of isChecked in case its checked state was updated
// internally.
dispatchOnButtonChecked(button.getId(), button.isChecked());
}
invalidate();
}
checkInternal(button.getId(), isChecked);
}

private class PressedStateTracker implements OnPressedChangeListener {
Expand Down
54 changes: 54 additions & 0 deletions lib/javatests/com/google/android/material/button/MaterialButtonToggleGroupTest.java
View file
Expand Up @@ -36,6 +36,7 @@
import androidx.test.core.app.ApplicationProvider;
import com.google.android.material.button.MaterialButtonToggleGroup.OnButtonCheckedListener;
import com.google.android.material.shape.ShapeAppearanceModel;
import java.util.List;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -183,6 +184,59 @@ public void singleSelection_withoutSelectionRequired_unSelects() {
assertThat(((Checkable) button).isChecked()).isFalse();
}

@Test
public void singleSelection_doesNotMultiSelect() {
toggleGroup.setSingleSelection(true);

View button1 = toggleGroup.getChildAt(0);
button1.performClick();
View button2 = toggleGroup.getChildAt(1);
button2.performClick();

assertThat(((Checkable) button1).isChecked()).isFalse();
assertThat(((Checkable) button2).isChecked()).isTrue();
}

@Test
public void singleSelection_doesNotMultiSelect_programmatically() {
toggleGroup.setSingleSelection(true);

View button1 = toggleGroup.getChildAt(0);
int id1 = ViewCompat.generateViewId();
button1.setId(id1);

View button2 = toggleGroup.getChildAt(1);
int id2 = ViewCompat.generateViewId();
button2.setId(id2);

toggleGroup.check(id1);
toggleGroup.check(id2);

assertThat(((Checkable) button1).isChecked()).isFalse();
assertThat(((Checkable) button2).isChecked()).isTrue();
}

@Test
public void multiSelection_correctSelectedIds() {
toggleGroup.setSingleSelection(false);

View button1 = toggleGroup.getChildAt(0);
int id1 = ViewCompat.generateViewId();
button1.setId(id1);

View button2 = toggleGroup.getChildAt(1);
int id2 = ViewCompat.generateViewId();
button2.setId(id2);

toggleGroup.check(id1);
toggleGroup.check(id2);

List<Integer> checkedIds = toggleGroup.getCheckedButtonIds();
assertThat(checkedIds.contains(id1)).isTrue();
assertThat(checkedIds.contains(id2)).isTrue();
assertThat(checkedIds.size()).isEqualTo(2);
}

@Test
public void multiSelection_withSelectionRequired_unSelectsIfTwo() {
toggleGroup.setSingleSelection(false);
Expand Down

0 comments on commit 3db25be

Please sign in to comment.