[ChipGroup] Fix ChipGroup.getCheckedChipIds() returns wrong state

In the Chip implementation, onCheckedChangeListener was called before onCheckedChangeListenerInternal. This causes an issue that in onCheckedChangeListener's callback, the checkable group's checked state is not updated yet, therefore ChipGroup.getCheckedChipIds() will return the outdated checked state. Fixes this by overriding Chip.setOnCheckedChangeListener to get full control of the execution order between onCheckedChangeListener and onCheckedChangeListenerInternal. Resolves #2691 PiperOrigin-RevId: 449100861 (cherry picked from commit 413a047)
material-components · May 25, 2022 · 0356d7c · 0356d7c
1 parent bef8ca1
commit 0356d7c
Show file tree

Hide file tree

Showing 55 changed files with 636 additions and 58 deletions.
diff --git a/build.gradle b/build.gradle
@@ -5,7 +5,7 @@ buildscript {
     mavenCentral()
   }
   dependencies {
-    classpath 'com.android.tools.build:gradle:4.0.0'
+    classpath 'com.android.tools.build:gradle:7.1.3'
     classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion"
   }
 }

diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties
@@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-6.1.1-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-7.2-all.zip
diff --git a/lib/java/com/google/android/material/chip/Chip.java b/lib/java/com/google/android/material/chip/Chip.java
@@ -16,6 +16,7 @@
 
 package com.google.android.material.chip;
 
+import android.widget.CompoundButton.OnCheckedChangeListener;
 import com.google.android.material.R;
 
 import static androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP;
@@ -54,6 +55,7 @@
 import android.view.ViewParent;
 import android.view.accessibility.AccessibilityEvent;
 import android.view.accessibility.AccessibilityNodeInfo;
+import android.widget.CompoundButton;
 import androidx.annotation.AnimatorRes;
 import androidx.annotation.BoolRes;
 import androidx.annotation.CallSuper;
@@ -151,6 +153,7 @@ public class Chip extends AppCompatCheckBox
   @Nullable private RippleDrawable ripple;
 
   @Nullable private OnClickListener onCloseIconClickListener;
+  @Nullable private CompoundButton.OnCheckedChangeListener onCheckedChangeListener;
   @Nullable private MaterialCheckable.OnCheckedChangeListener<Chip> onCheckedChangeListenerInternal;
   private boolean deferredCheckedValue;
   private boolean closeIconPressed;
@@ -251,6 +254,19 @@ public Chip(Context context, AttributeSet attrs, int defStyleAttr) {
       setMinHeight(minTouchTargetSize);
     }
     lastLayoutDirection = ViewCompat.getLayoutDirection(this);
+
+    super.setOnCheckedChangeListener(
+        new CompoundButton.OnCheckedChangeListener() {
+          @Override
+          public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
+            if (onCheckedChangeListenerInternal != null) {
+              onCheckedChangeListenerInternal.onCheckedChanged(Chip.this, isChecked);
+            }
+            if (onCheckedChangeListener != null) {
+              onCheckedChangeListener.onCheckedChanged(buttonView, isChecked);
+            }
+          }
+        });
   }
 
   @Override
@@ -707,17 +723,17 @@ public void setChecked(boolean checked) {
       // Defer the setChecked() call until after initialization.
       deferredCheckedValue = checked;
     } else if (chipDrawable.isCheckable()) {
-      boolean wasChecked = isChecked();
       super.setChecked(checked);
-
-      if (wasChecked != checked) {
-        if (onCheckedChangeListenerInternal != null) {
-          onCheckedChangeListenerInternal.onCheckedChanged(this, checked);
-        }
-      }
     }
   }
 
+  @Override
+  public void setOnCheckedChangeListener(
+      @Nullable CompoundButton.OnCheckedChangeListener listener) {
+    // Do not call super here - the wrapped listener set in the constructor will call the listener.
+    onCheckedChangeListener = listener;
+  }
+
   /** Register a callback to be invoked when the close icon is clicked. */
   public void setOnCloseIconClickListener(OnClickListener listener) {
     this.onCloseIconClickListener = listener;

diff --git a/lib/javatests/com/google/android/material/appbar/MaterialToolbarTest.java b/lib/javatests/com/google/android/material/appbar/MaterialToolbarTest.java
@@ -15,7 +15,7 @@
  */
 package com.google.android.material.appbar;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

diff --git a/lib/javatests/com/google/android/material/badge/BadgeDrawableTest.java b/lib/javatests/com/google/android/material/badge/BadgeDrawableTest.java
@@ -15,7 +15,7 @@
  */
 package com.google.android.material.badge;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

diff --git a/lib/javatests/com/google/android/material/badge/BadgeUtilsTest.java b/lib/javatests/com/google/android/material/badge/BadgeUtilsTest.java
@@ -15,7 +15,7 @@
  */
 package com.google.android.material.badge;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

diff --git a/lib/javatests/com/google/android/material/bottomnavigation/BottomNavigationViewTest.java b/lib/javatests/com/google/android/material/bottomnavigation/BottomNavigationViewTest.java
@@ -15,7 +15,7 @@
  */
 package com.google.android.material.bottomnavigation;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

diff --git a/lib/javatests/com/google/android/material/bottomsheet/BottomSheetBehaviorTest.java b/lib/javatests/com/google/android/material/bottomsheet/BottomSheetBehaviorTest.java
@@ -15,7 +15,7 @@
  */
 package com.google.android.material.bottomsheet;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

diff --git a/lib/javatests/com/google/android/material/button/MaterialButtonTest.java b/lib/javatests/com/google/android/material/button/MaterialButtonTest.java
@@ -15,7 +15,7 @@
  */
 package com.google.android.material.button;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

diff --git a/lib/javatests/com/google/android/material/button/MaterialButtonToggleGroupTest.java b/lib/javatests/com/google/android/material/button/MaterialButtonToggleGroupTest.java
@@ -16,7 +16,7 @@
 
 package com.google.android.material.button;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
 import static android.view.View.GONE;
 import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;

diff --git a/lib/javatests/com/google/android/material/checkbox/MaterialCheckBoxTest.java b/lib/javatests/com/google/android/material/checkbox/MaterialCheckBoxTest.java
@@ -16,7 +16,7 @@
 
 package com.google.android.material.checkbox;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

diff --git a/lib/javatests/com/google/android/material/chip/ChipGroupTest.java b/lib/javatests/com/google/android/material/chip/ChipGroupTest.java
@@ -15,15 +15,17 @@
  */
 package com.google.android.material.chip;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
+import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import android.content.Context;
 import androidx.appcompat.app.AppCompatActivity;
 import android.view.View;
+import android.widget.CompoundButton;
 import androidx.core.view.ViewCompat;
 import androidx.core.view.accessibility.AccessibilityNodeInfoCompat;
 import androidx.core.view.accessibility.AccessibilityNodeInfoCompat.CollectionInfoCompat;
@@ -196,6 +198,29 @@ public void onCheckedChanged(ChipGroup group, List<Integer> checkedIds) {
     assertThat(checkedIds).contains(second.getId());
   }
 
+  @Test
+  public void multipleSelection_chipListener() {
+    chipgroup.setSingleSelection(false);
+
+    Chip first = (Chip) chipgroup.getChildAt(0);
+    first.setOnCheckedChangeListener(this::onChipCheckedStateChanged);
+
+    Chip second = (Chip) chipgroup.getChildAt(1);
+    second.setOnCheckedChangeListener(this::onChipCheckedStateChanged);
+
+    first.performClick();
+    getInstrumentation().waitForIdleSync();
+
+    assertThat(checkedChangeCallCount).isEqualTo(1);
+    assertThat(checkedIds).containsExactly(first.getId());
+
+    second.performClick();
+    getInstrumentation().waitForIdleSync();
+
+    assertThat(checkedChangeCallCount).isEqualTo(2);
+    assertThat(checkedIds).containsExactly(first.getId(), second.getId());
+  }
+
   @Test
   public void multiSelection_withSelectionRequired_unSelectsIfTwo() {
     chipgroup.setSingleSelection(false);
@@ -260,4 +285,22 @@ public void isNotSingleLine_initializesAccessibilityNodeInfo() {
     assertEquals(1, itemInfo.getRowIndex());
     assertTrue(itemInfo.isSelected());
   }
+
+  @Test
+  public void getChipAccessibilityClassName_multipleChecked_buttonName() {
+    Chip chip = (Chip) chipgroup.getChildAt(0);
+    assertEquals("android.widget.Button", chip.getAccessibilityClassName().toString());
+  }
+
+  @Test
+  public void getChipAccessibilityClassName_singleChecked_radioButtonName() {
+    chipgroup.setSingleSelection(true);
+    Chip chip = (Chip) chipgroup.getChildAt(0);
+    assertEquals("android.widget.RadioButton", chip.getAccessibilityClassName().toString());
+  }
+
+  private void onChipCheckedStateChanged(CompoundButton chip, boolean checked) {
+    checkedChangeCallCount++;
+    checkedIds = chipgroup.getCheckedChipIds();
+  }
 }
diff --git a/lib/javatests/com/google/android/material/chip/ChipTest.java b/lib/javatests/com/google/android/material/chip/ChipTest.java
@@ -15,7 +15,7 @@
  */
 package com.google.android.material.chip;
 
-import com.google.android.material.R;
+import com.google.android.material.test.R;
 
 import static com.google.android.material.internal.ViewUtils.dpToPx;
 import static com.google.common.truth.Truth.assertThat;