From a17af6ee04c6092846b3060a7e871b55db71c77f Mon Sep 17 00:00:00 2001 From: conradchen Date: Thu, 23 Dec 2021 17:05:39 -0500 Subject: [PATCH] Automated g4 rollback of changelist 418054400 PiperOrigin-RevId: 418062580 --- .../button/MaterialButtonToggleGroup.java | 139 ++++++++++++------ .../button/MaterialButtonToggleGroupTest.java | 54 ------- 2 files changed, 90 insertions(+), 103 deletions(-) diff --git a/lib/java/com/google/android/material/button/MaterialButtonToggleGroup.java b/lib/java/com/google/android/material/button/MaterialButtonToggleGroup.java index 2763df0c93b..2f216bc3e5a 100644 --- a/lib/java/com/google/android/material/button/MaterialButtonToggleGroup.java +++ b/lib/java/com/google/android/material/button/MaterialButtonToggleGroup.java @@ -49,10 +49,8 @@ import com.google.android.material.shape.ShapeAppearanceModel; import java.util.ArrayList; 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; @@ -172,7 +170,7 @@ public int compare(MaterialButton v1, MaterialButton v2) { private boolean singleSelection; private boolean selectionRequired; - private Set checkedIds = new HashSet<>(); + @IdRes private int checkedId; public MaterialButtonToggleGroup(@NonNull Context context) { this(context, null); @@ -193,11 +191,8 @@ public MaterialButtonToggleGroup( setSingleSelection( attributes.getBoolean(R.styleable.MaterialButtonToggleGroup_singleSelection, false)); - if (attributes.hasValue(R.styleable.MaterialButtonToggleGroup_checkedButton)) { - checkedIds.add( - attributes.getResourceId( - R.styleable.MaterialButtonToggleGroup_checkedButton, View.NO_ID)); - } + checkedId = + attributes.getResourceId(R.styleable.MaterialButtonToggleGroup_checkedButton, View.NO_ID); selectionRequired = attributes.getBoolean(R.styleable.MaterialButtonToggleGroup_selectionRequired, false); setChildrenDrawingOrderEnabled(true); @@ -209,7 +204,11 @@ public MaterialButtonToggleGroup( @Override protected void onFinishInflate() { super.onFinishInflate(); - updateCheckedIds(checkedIds); + + // Checks the appropriate button as requested via XML + if (checkedId != View.NO_ID) { + checkForced(checkedId, true); + } } @Override @@ -235,8 +234,11 @@ 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); - // Update button group's checked states - checkInternal(buttonChild.getId(), buttonChild.isChecked()); + // Reorders children if a checked child was added to this layout + if (buttonChild.isChecked()) { + updateCheckedStates(buttonChild.getId(), true); + setCheckedId(buttonChild.getId()); + } // Saves original corner data ShapeAppearanceModel shapeAppearanceModel = buttonChild.getShapeAppearanceModel(); @@ -325,7 +327,11 @@ public void onInitializeAccessibilityNodeInfo(@NonNull AccessibilityNodeInfo inf * @see #getCheckedButtonId() */ public void check(@IdRes int id) { - checkInternal(id, true); + if (id == checkedId) { + return; + } + + checkForced(id, true); } /** @@ -338,7 +344,7 @@ public void check(@IdRes int id) { * @see #getCheckedButtonId() */ public void uncheck(@IdRes int id) { - checkInternal(id, false); + checkForced(id, false); } /** @@ -351,7 +357,16 @@ public void uncheck(@IdRes int id) { * @see #getCheckedButtonId() */ public void clearChecked() { - updateCheckedIds(new HashSet()); + 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); } /** @@ -370,7 +385,7 @@ public void clearChecked() { */ @IdRes public int getCheckedButtonId() { - return singleSelection && !checkedIds.isEmpty() ? checkedIds.iterator().next() : View.NO_ID; + return singleSelection ? checkedId : View.NO_ID; } /** @@ -387,15 +402,15 @@ public int getCheckedButtonId() { */ @NonNull public List getCheckedButtonIds() { - List orderedCheckedIds = new ArrayList<>(); + List checkedIds = new ArrayList<>(); for (int i = 0; i < getChildCount(); i++) { - int childId = getChildButton(i).getId(); - if (checkedIds.contains(childId)) { - orderedCheckedIds.add(childId); + MaterialButton child = getChildButton(i); + if (child.isChecked()) { + checkedIds.add(child.getId()); } } - return orderedCheckedIds; + return checkedIds; } /** @@ -490,6 +505,12 @@ 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 @@ -665,35 +686,37 @@ private static void updateBuilderWithCornerData( .setBottomRightCornerSize(cornerData.bottomRight); } - private void checkInternal(@IdRes int buttonId, boolean checked) { - Set 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; - } - updateCheckedIds(checkedIds); - } - - private void updateCheckedIds(Set checkedIds) { - for (int i = 0; i < getChildCount(); i++) { - int buttonId = getChildButton(i).getId(); - setCheckedStateForView(buttonId, checkedIds.contains(buttonId)); - if (this.checkedIds.contains(buttonId) != checkedIds.contains(buttonId)) { - dispatchOnButtonChecked(buttonId, checkedIds.contains(buttonId)); + /** + * 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. + * + *

If {@code singleSelection} is true, this will unselect any other children as well. + * + *

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 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); } } - this.checkedIds = new HashSet<>(checkedIds); - invalidate(); + return true; } private void dispatchOnButtonChecked(@IdRes int buttonId, boolean checked) { @@ -702,6 +725,13 @@ private void dispatchOnButtonChecked(@IdRes int buttonId, boolean checked) { } } + private void checkForced(int checkedId, boolean checked) { + MaterialButton button = findViewById(checkedId); + if (button != null) { + button.setChecked(checked); + } + } + private void setGeneratedIdIfNeeded(@NonNull MaterialButton materialButton) { // Generates an ID if none is set, for relative positioning purposes if (materialButton.getId() == View.NO_ID) { @@ -765,11 +795,22 @@ private void updateChildOrder() { private class CheckedStateTracker implements MaterialButton.OnCheckedChangeListener { @Override public void onCheckedChanged(@NonNull MaterialButton button, boolean isChecked) { - // Checked state change is triggered by the button group, do not update checked ids again. + // Prevents infinite recursion if (skipCheckedStateTracker) { return; } - checkInternal(button.getId(), isChecked); + + 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(); } } diff --git a/lib/javatests/com/google/android/material/button/MaterialButtonToggleGroupTest.java b/lib/javatests/com/google/android/material/button/MaterialButtonToggleGroupTest.java index e7598362858..005e3e2bf6e 100644 --- a/lib/javatests/com/google/android/material/button/MaterialButtonToggleGroupTest.java +++ b/lib/javatests/com/google/android/material/button/MaterialButtonToggleGroupTest.java @@ -36,7 +36,6 @@ 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; @@ -184,59 +183,6 @@ 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 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);