Fixes the issue that the index of the view becomes inconsistent on the (#311)

thagikura · thagikura · commit e171891a78a4 · 2017-06-28T14:17:47.000+09:00
conditions that

- Adapter has multiple viewTypes (1, 2. Assuming 2 is the special viewtype)
- A new item is inserted to the adpater before the view which has the
  special viewType
- At the time item is inserted, the special view is not visible.

In that case, the index of the view which has the special viewType is
shifted by the number of items inserted before the position.
diff --git a/flexbox/src/androidTest/java/com/google/android/flexbox/test/FlexboxLayoutManagerTest.java b/flexbox/src/androidTest/java/com/google/android/flexbox/test/FlexboxLayoutManagerTest.java
@@ -27,6 +27,7 @@
 
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.lessThan;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.core.IsNot.not;
 import static org.junit.Assert.assertThat;
@@ -3194,6 +3195,79 @@ public void run() {
                 isEqualAllowingError(TestUtil.dpToPixel(activity, 120)));
     }
 
+    @Test
+    @FlakyTest
+    public void testDrawDirtyFlexLine_multi_viewTypes_direction_row() throws Throwable {
+        // This test verifies https://github.com/google/flexbox-layout/issues/280
+        // the position of the view type is shifted if a new item is inserted before the
+        // view which has the special viewType and that view isn't visible at the time the item
+        // was inserted.
+        final FlexboxTestActivity activity = mActivityRule.getActivity();
+        final FlexboxLayoutManager layoutManager = new FlexboxLayoutManager(activity);
+        final TestAdapterMultiViewTypes adapter = new TestAdapterMultiViewTypes();
+        mActivityRule.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                activity.setContentView(R.layout.recyclerview);
+                RecyclerView recyclerView = (RecyclerView) activity.findViewById(R.id.recyclerview);
+                layoutManager.setFlexDirection(FlexDirection.ROW);
+                recyclerView.setLayoutManager(layoutManager);
+                recyclerView.setAdapter(adapter);
+            }
+        });
+        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
+        RecyclerView parent = (RecyclerView) activity.findViewById(R.id.recyclerview);
+        TextView matchParentText = (TextView) layoutManager
+                .getChildAt(TestAdapterMultiViewTypes.POSITION_MATCH_PARENT);
+        assertThat(matchParentText.getWidth(), is(parent.getWidth()));
+        assertThat(matchParentText.getText().toString(),
+                is(String.valueOf(TestAdapterMultiViewTypes.POSITION_MATCH_PARENT + 1)));
+
+        onView(withId(R.id.recyclerview)).perform(swipe(GeneralLocation.BOTTOM_CENTER,
+                GeneralLocation.TOP_CENTER));
+        onView(withId(R.id.recyclerview)).perform(swipe(GeneralLocation.BOTTOM_CENTER,
+                GeneralLocation.TOP_CENTER));
+        onView(withId(R.id.recyclerview)).perform(swipe(GeneralLocation.BOTTOM_CENTER,
+                GeneralLocation.TOP_CENTER));
+        onView(withId(R.id.recyclerview)).perform(swipe(GeneralLocation.BOTTOM_CENTER,
+                GeneralLocation.TOP_CENTER));
+
+        final int insertedValue = 10;
+        mActivityRule.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                TestAdapterMultiViewTypes.Item item = new TestAdapterMultiViewTypes.Item();
+                item.value = insertedValue;
+                // Insert an item before the position that has a special viewType
+                adapter.addItemAt(TestAdapterMultiViewTypes.POSITION_MATCH_PARENT - 1, item);
+            }
+        });
+        onView(withId(R.id.recyclerview)).perform(swipe(GeneralLocation.TOP_CENTER,
+                GeneralLocation.BOTTOM_CENTER));
+        onView(withId(R.id.recyclerview)).perform(swipe(GeneralLocation.TOP_CENTER,
+                GeneralLocation.BOTTOM_CENTER));
+        onView(withId(R.id.recyclerview)).perform(swipe(GeneralLocation.TOP_CENTER,
+                GeneralLocation.BOTTOM_CENTER));
+        onView(withId(R.id.recyclerview)).perform(swipe(GeneralLocation.TOP_CENTER,
+                GeneralLocation.BOTTOM_CENTER));
+        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
+
+        // Since a new item is inserted before the position, the index at the view who has the
+        // special viewType should be shifted.
+        matchParentText = (TextView) layoutManager
+                .getChildAt(TestAdapterMultiViewTypes.POSITION_MATCH_PARENT + 1);
+        assertThat(matchParentText.getWidth(), is(parent.getWidth()));
+        assertThat(matchParentText.getText().toString(),
+                is(String.valueOf(TestAdapterMultiViewTypes.POSITION_MATCH_PARENT + 1)));
+
+        // The position of this view is the old position who had the special viewType, but
+        // now the viewType should be a normal one
+        TextView textView = (TextView) layoutManager
+                .getChildAt(TestAdapterMultiViewTypes.POSITION_MATCH_PARENT - 1);
+        assertThat(textView.getWidth(), lessThan(parent.getWidth()));
+        assertThat(textView.getText().toString(), is(String.valueOf(insertedValue)));
+    }
+
     @Test
     @FlakyTest
     public void testChildrenSizeWithMargin() throws Throwable {
diff --git a/flexbox/src/androidTest/java/com/google/android/flexbox/test/TestAdapterMultiViewTypes.java b/flexbox/src/androidTest/java/com/google/android/flexbox/test/TestAdapterMultiViewTypes.java
@@ -0,0 +1,99 @@
+/*
+ * Copyright 2017 Google Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.android.flexbox.test;
+
+import android.support.v7.widget.RecyclerView;
+import android.view.Gravity;
+import android.view.LayoutInflater;
+import android.view.View;
+import android.view.ViewGroup;
+
+import com.google.android.flexbox.FlexboxLayoutManager;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * {@link RecyclerView.Adapter} implementation for {@link TestViewHolder}, which has multiple
+ * view types.
+ */
+class TestAdapterMultiViewTypes extends RecyclerView.Adapter<TestViewHolder> {
+
+    static final int POSITION_MATCH_PARENT = 3;
+    private static final int ITEMS = 50;
+
+    private static final int VIEW_TYPE_NORMAL = 0;
+    private static final int VIEW_TYPE_MATCH_PARENT = 1;
+
+    private final List<Item> mItems;
+
+    TestAdapterMultiViewTypes() {
+        mItems = new ArrayList<>();
+        for (int i = 0; i < ITEMS; i++) {
+            Item item = new Item();
+            if (i == POSITION_MATCH_PARENT) {
+                item.viewType = VIEW_TYPE_MATCH_PARENT;
+            }
+            item.value = i + 1;
+            mItems.add(item);
+        }
+    }
+
+    @Override
+    public TestViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
+        View view = LayoutInflater.from(parent.getContext())
+                .inflate(R.layout.recyclerview_viewholder, parent, false);
+        if (viewType == VIEW_TYPE_MATCH_PARENT) {
+            FlexboxLayoutManager.LayoutParams flexboxLp = (FlexboxLayoutManager.LayoutParams)
+                    view.getLayoutParams();
+            flexboxLp.setFlexBasisPercent(90f);
+            flexboxLp.setFlexGrow(1f);
+        }
+        return new TestViewHolder(view);
+    }
+
+    @Override
+    public void onBindViewHolder(TestViewHolder holder, int position) {
+        holder.mTextView.setText(String.valueOf(mItems.get(position).value));
+        holder.mTextView.setBackgroundResource(R.drawable.flex_item_background);
+        holder.mTextView.setGravity(Gravity.CENTER);
+    }
+
+    @Override
+    public int getItemViewType(int position) {
+        return mItems.get(position).viewType;
+    }
+
+    void addItemAt(int position, Item item) {
+        mItems.add(position, item);
+        notifyItemInserted(position);
+    }
+
+    Item getItemAt(int position) {
+        return mItems.get(position);
+    }
+
+    @Override
+    public int getItemCount() {
+        return mItems.size();
+    }
+
+    static class Item {
+        int viewType = VIEW_TYPE_NORMAL;
+        int value;
+    }
+}
diff --git a/flexbox/src/main/java/com/google/android/flexbox/FlexLine.java b/flexbox/src/main/java/com/google/android/flexbox/FlexLine.java
@@ -94,21 +94,6 @@ public class FlexLine {
 
     int mLastIndex;
 
-    /**
-     * Indicates whether this flex line instance is in a dirty state (needs to be computed before
-     * rendering). This flag is set to true for example an item is added at the position not
-     * visible and the position is prior to the position than the first visible position.
-     * In that case, the newly added item and flex line needs to be re-computed when the
-     * RecyclerView is scrolled toward start and it's about to be drawn.
-     *
-     * Note that a re-computation is not needed for the case where the dirty flag is set to a
-     * flex line after the last visible position. Because the flex lines after the visible potion
-     * are going to be cleared in the normal layout pass in the onLayoutChildren -> updateFlexLine
-     * method. Thus at the time scrolling to the added position, new flex lines will be computed
-     * anyway.
-     */
-    boolean mDirty;
-
     /**
      * @return the distance in pixels from the top edge of this view's parent
      * to the top edge of this FlexLine.
diff --git a/flexbox/src/main/java/com/google/android/flexbox/FlexboxHelper.java b/flexbox/src/main/java/com/google/android/flexbox/FlexboxHelper.java
@@ -362,7 +362,7 @@ FlexLinesResult calculateVerticalFlexLinesToIndex(int widthMeasureSpec, int heig
      * @return an instance of {@link FlexLinesResult} that contains a list of flex lines and the
      * child state used by {@link View#setMeasuredDimension(int, int)}.
      */
-    private FlexLinesResult calculateFlexLines(int mainMeasureSpec, int crossMeasureSpec,
+    FlexLinesResult calculateFlexLines(int mainMeasureSpec, int crossMeasureSpec,
             int needsCalcAmount, int fromIndex, int toIndex,
             @Nullable List<FlexLine> existingLines) {
 
@@ -825,7 +825,6 @@ private void addFlexLine(List<FlexLine> flexLines, FlexLine flexLine, int viewIn
         flexLine.mSumCrossSizeBefore = usedCrossSizeSoFar;
         mFlexContainer.onNewFlexLineAdded(flexLine);
         flexLine.mLastIndex = viewIndex;
-        flexLine.mDirty = false;
         flexLines.add(flexLine);
     }
 
@@ -1556,6 +1555,9 @@ void stretchViews(int fromIndex) {
                 FlexLine flexLine = flexLines.get(i);
                 for (int j = 0, itemCount = flexLine.mItemCount; j < itemCount;
                         j++, viewIndex++) {
+                    if (j >= mFlexContainer.getFlexItemCount()) {
+                        continue;
+                    }
                     View view = mFlexContainer.getReorderedFlexItemAt(viewIndex);
                     if (view == null || view.getVisibility() == View.GONE) {
                         continue;
diff --git a/flexbox/src/main/java/com/google/android/flexbox/FlexboxLayoutManager.java b/flexbox/src/main/java/com/google/android/flexbox/FlexboxLayoutManager.java
