Merge "Save the focused conversation via id instead of position" into ub-gmail-ur14-dev

6 files changed, 72 insertions, 53 deletions

diff --git a/src/com/android/mail/browse/ConversationItemView.java b/src/com/android/mail/browse/ConversationItemView.java
index 65cf5ce33..e2dcf472f 100644
--- a/src/com/android/mail/browse/ConversationItemView.java
+++ b/src/com/android/mail/browse/ConversationItemView.java
@@ -58,7 +58,6 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewParent;
 import android.view.animation.DecelerateInterpolator;
-import android.widget.ListView;
 import android.widget.TextView;
 
 import com.android.mail.R;
@@ -197,7 +196,6 @@ public class ConversationItemView extends View
     private final TextView mSubjectTextView;
     private final TextView mSnippetTextView;
     private int mGadgetMode;
-    private int mAdapterPosition = ListView.INVALID_POSITION;
 
     private static int sFoldersMaxCount;
     private static TextAppearanceSpan sSubjectTextUnreadSpan;
@@ -472,9 +470,8 @@ public class ConversationItemView extends View
             final ConversationCheckedSet set, final Folder folder,
             final int checkboxOrSenderImage,
             final boolean swipeEnabled, final boolean importanceMarkersEnabled,
-            final boolean showChevronsEnabled, final AnimatedAdapter adapter, int position) {
+            final boolean showChevronsEnabled, final AnimatedAdapter adapter) {
         Utils.traceBeginSection("CIVC.bind");
-        mAdapterPosition = position;
         bind(ConversationItemViewModel.forConversation(mAccount.getEmailAddress(), conversation),
                 activity, null /* conversationItemAreaClickListener */,
                 set, folder, checkboxOrSenderImage, swipeEnabled, importanceMarkersEnabled,
@@ -1277,7 +1274,7 @@ public class ConversationItemView extends View
 
         // The focused bar
         final SwipeableListView listView = getListView();
-        if (listView != null && listView.isPositionSelected(getViewPosition())) {
+        if (listView != null && listView.isConversationSelected(getConversation())) {
             final int w = FOCUSED_CONVERSATION_HIGHLIGHT.getIntrinsicWidth();
             final boolean isRtl = ViewUtils.isViewRtl(this);
             // This bar is on the right side of the conv list if it's RTL
@@ -1299,26 +1296,12 @@ public class ConversationItemView extends View
         if (selected) {
             final SwipeableListView listView = getListView();
             if (listView != null) {
-                listView.setSelectedPosition(getViewPosition());
+                listView.setSelectedConversation(getConversation());
             }
         }
         super.setSelected(selected);
     }
 
-    private int getViewPosition() {
-        if (mAdapterPosition != ListView.INVALID_POSITION) {
-            return mAdapterPosition;
-        } else {
-            final ListView listView = getListView();
-            final int position = listView != null ?
-                    listView.getPositionForView(this) : ListView.INVALID_POSITION;
-            LogUtils.e(LOG_TAG,
-                    "ConversationItemView didn't set position, using listview's position: %d",
-                    position);
-            return position;
-        }
-    }
-
     private void drawSendersImage(final Canvas canvas) {
         if (!mSendersImageView.isFlipping()) {
             final boolean showSenders = !mChecked;
@@ -1405,7 +1388,8 @@ public class ConversationItemView extends View
             final SwipeableListView listView = getListView();
 
             try {
-                conv.position = mChecked ? getViewPosition() : Conversation.NO_POSITION;
+                conv.position = mChecked && listView != null ? listView.getPositionForView(this)
+                        : Conversation.NO_POSITION;
             } catch (final NullPointerException e) {
                 // TODO(skennedy) Remove this if we find the root cause b/9527863
             }
diff --git a/src/com/android/mail/browse/SwipeableConversationItemView.java b/src/com/android/mail/browse/SwipeableConversationItemView.java
index 9db966906..10657f4f1 100644
--- a/src/com/android/mail/browse/SwipeableConversationItemView.java
+++ b/src/com/android/mail/browse/SwipeableConversationItemView.java
@@ -56,10 +56,9 @@ public class SwipeableConversationItemView extends FrameLayout implements Toggle
             final ConversationCheckedSet set, final Folder folder,
             final int checkboxOrSenderImage, boolean swipeEnabled,
             final boolean importanceMarkersEnabled, final boolean showChevronsEnabled,
-            final AnimatedAdapter animatedAdapter, final int position) {
+            final AnimatedAdapter animatedAdapter) {
         mConversationItemView.bind(conversation, activity, set, folder, checkboxOrSenderImage,
-                swipeEnabled, importanceMarkersEnabled, showChevronsEnabled, animatedAdapter,
-                position);
+                swipeEnabled, importanceMarkersEnabled, showChevronsEnabled, animatedAdapter);
     }
 
     public void startUndoAnimation(AnimatorListener listener, boolean swipe) {
diff --git a/src/com/android/mail/ui/AnimatedAdapter.java b/src/com/android/mail/ui/AnimatedAdapter.java
index 86a3658bb..2b38d27f8 100644
--- a/src/com/android/mail/ui/AnimatedAdapter.java
+++ b/src/com/android/mail/ui/AnimatedAdapter.java
@@ -380,12 +380,12 @@ public class AnimatedAdapter extends SimpleCursorAdapter {
     }
 
     public View createConversationItemView(SwipeableConversationItemView view, Context context,
-            Conversation conv, int position) {
+            Conversation conv) {
         if (view == null) {
             view = new SwipeableConversationItemView(context, mAccount);
         }
         view.bind(conv, mActivity, mBatchConversations, mFolder, getCheckboxSetting(),
-                mSwipeEnabled, mImportanceMarkersEnabled, mShowChevronsEnabled, this, position);
+                mSwipeEnabled, mImportanceMarkersEnabled, mShowChevronsEnabled, this);
         return view;
     }
 
@@ -556,7 +556,7 @@ public class AnimatedAdapter extends SimpleCursorAdapter {
             ((SwipeableConversationItemView) convertView).reset();
         }
         final View v = createConversationItemView((SwipeableConversationItemView) convertView,
-                mContext, conv, position);
+                mContext, conv);
         Utils.traceEndSection();
         return v;
     }
@@ -790,7 +790,7 @@ public class AnimatedAdapter extends SimpleCursorAdapter {
                 position, null, parent);
         view.reset();
         view.bind(conversation, mActivity, mBatchConversations, mFolder, getCheckboxSetting(),
-                mSwipeEnabled, mImportanceMarkersEnabled, mShowChevronsEnabled, this, position);
+                mSwipeEnabled, mImportanceMarkersEnabled, mShowChevronsEnabled, this);
         mAnimatingViews.put(conversation.id, view);
         return view;
     }
diff --git a/src/com/android/mail/ui/ConversationListFragment.java b/src/com/android/mail/ui/ConversationListFragment.java
index d80cdf9ad..439f6b597 100644
--- a/src/com/android/mail/ui/ConversationListFragment.java
+++ b/src/com/android/mail/ui/ConversationListFragment.java
@@ -493,7 +493,7 @@ public final class ConversationListFragment extends Fragment implements
         sb.append(mViewContext.folder);
         if (mListView != null) {
             sb.append(" selectedPos=");
-            sb.append(mListView.getSelectedPosition());
+            sb.append(mListView.getSelectedConversationPosDebug());
             sb.append(" listSelectedPos=");
             sb.append(mListView.getSelectedItemPosition());
             sb.append(" isListInTouchMode=");
@@ -858,7 +858,7 @@ public final class ConversationListFragment extends Fragment implements
          * special views in the list.
          */
             conv.position = cursor.getPosition();
-            setActivated(conv.position, true);
+            setActivated(conv, true);
             mCallbacks.onConversationSelected(conv, false /* inLoaderCallbacks */);
         } else {
             LogUtils.e(LOG_TAG,
@@ -869,41 +869,41 @@ public final class ConversationListFragment extends Fragment implements
 
     /**
      * Sets the checked conversation to the position given here.
-     * @param cursorPosition The position of the conversation in the cursor (as opposed to
-     * in the list)
+     * @param conversation the activated conversation.
      * @param different if the currently checked conversation is different from the one provided
      * here.  This is a difference in conversations, not a difference in positions. For example, a
      * conversation at position 2 can move to position 4 as a result of new mail.
      */
-    public void setActivated(final int cursorPosition, boolean different) {
-        if (mListView.getChoiceMode() == ListView.CHOICE_MODE_NONE) {
+    public void setActivated(final Conversation conversation, boolean different) {
+        if (mListView.getChoiceMode() == ListView.CHOICE_MODE_NONE || conversation == null) {
             return;
         }
 
+        final int cursorPosition = conversation.position;
         final int position = cursorPosition + mListAdapter.getPositionOffset(cursorPosition);
         setRawActivated(position, different);
-        setRawSelected(position);
+        setRawSelected(conversation, position);
     }
 
     /**
      * Set the selected conversation (used by the framework to indicate current focus in the list).
-     * @param cursorPosition The position of the conversation in the cursor (as opposed to
-     * in the list)
+     * @param conversation the selected conversation.
      */
-    public void setSelected(final int cursorPosition) {
-        if (mListView.getChoiceMode() == ListView.CHOICE_MODE_NONE) {
+    public void setSelected(final Conversation conversation) {
+        if (mListView.getChoiceMode() == ListView.CHOICE_MODE_NONE || conversation == null) {
             return;
         }
 
+        final int cursorPosition = conversation.position;
         final int position = cursorPosition + mListAdapter.getPositionOffset(cursorPosition);
-        setRawSelected(position);
+        setRawSelected(conversation, position);
     }
 
     /**
      * Set the selected conversation (used by the framework to indicate current focus in the list).
      * @param position The position of the item in the list
      */
-    private void setRawSelected(final int position) {
+    private void setRawSelected(Conversation conversation, final int position) {
         final View selectedView = mListView.getChildAt(
                 position - mListView.getFirstVisiblePosition());
         // Don't do anything if the view is already selected.
@@ -919,7 +919,7 @@ public final class ConversationListFragment extends Fragment implements
                 // setSelection calls setSelectionFromTop with y = 0.
                 mListView.setSelectionFromTop(position, selectedView.getTop());
             }
-            mListView.setSelectedPosition(position);
+            mListView.setSelectedConversation(conversation);
         }
     }
 
@@ -1154,7 +1154,7 @@ public final class ConversationListFragment extends Fragment implements
         if (conv != null && !currentConvIsPeeking) {
             if (mListView.getChoiceMode() != ListView.CHOICE_MODE_NONE
                     && mListView.getCheckedItemPosition() == -1) {
-                setActivated(conv.position, true);
+                setActivated(conv, true);
             }
         }
     }
diff --git a/src/com/android/mail/ui/SwipeableListView.java b/src/com/android/mail/ui/SwipeableListView.java
index b8406b3d8..39429d03b 100644
--- a/src/com/android/mail/ui/SwipeableListView.java
+++ b/src/com/android/mail/ui/SwipeableListView.java
@@ -49,6 +49,8 @@ import java.util.Collection;
 import java.util.HashMap;
 
 public class SwipeableListView extends ListView implements Callback, OnScrollListener {
+    private static final long INVALID_CONVERSATION_ID = -1;
+
     private final SwipeHelper mSwipeHelper;
     /**
      * Are swipes enabled on all items? (Each individual item can still prevent swiping.)<br>
@@ -72,7 +74,7 @@ public class SwipeableListView extends ListView implements Callback, OnScrollLis
 
     private SwipeListener mSwipeListener;
 
-    private int mSelectedPosition = ListView.INVALID_POSITION;
+    private long mSelectedConversationId = INVALID_CONVERSATION_ID;
 
     // Instantiated through view inflation
     @SuppressWarnings("unused")
@@ -411,20 +413,54 @@ public class SwipeableListView extends ListView implements Callback, OnScrollLis
     /**
      * Set the currently selected (focused by the list view) position.
      */
-    public void setSelectedPosition(int position) {
-        if (position == ListView.INVALID_POSITION) {
+    public void setSelectedConversation(Conversation conv) {
+        if (conv == null) {
             return;
         }
 
-        mSelectedPosition = position;
+        mSelectedConversationId = conv.id;
+    }
+
+    public boolean isConversationSelected(Conversation conv) {
+        return mSelectedConversationId != INVALID_CONVERSATION_ID && conv != null
+                && mSelectedConversationId == conv.id;
     }
 
-    public boolean isPositionSelected(int position) {
-        return mSelectedPosition != ListView.INVALID_POSITION && mSelectedPosition == position;
+    /**
+     * This is only used for debugging/logging purposes. DO NOT call this function to try to get
+     * the currently selected position. Use {@link #mSelectedConversationId} instead.
+     */
+    public int getSelectedConversationPosDebug() {
+        for (int i = getFirstVisiblePosition(); i < getLastVisiblePosition(); i++) {
+            final Object item = getItemAtPosition(i);
+            if (item instanceof ConversationCursor) {
+                final Conversation c = ((ConversationCursor) item).getConversation();
+                if (c.id == mSelectedConversationId) {
+                    return i;
+                }
+            }
+        }
+        return ListView.INVALID_POSITION;
     }
 
-    public int getSelectedPosition() {
-        return mSelectedPosition;
+    @Override
+    public void onTouchModeChanged(boolean isInTouchMode) {
+        super.onTouchModeChanged(isInTouchMode);
+        if (!isInTouchMode) {
+            // We need to invalidate going from touch mode -> keyboard mode because the currently
+            // selected item might have changed in touch mode. However, since from the framework's
+            // perspective the selected position doesn't matter in touch mode, when we enter
+            // keyboard mode via up/down arrow, the list view will ONLY invalidate the newly
+            // selected item and not the currently selected item. As a result, we might get an
+            // inconsistent UI where it looks like both the old and new selected items are focused.
+            final int index = getSelectedItemPosition();
+            if (index != ListView.INVALID_POSITION) {
+                final View child = getChildAt(index - getFirstVisiblePosition());
+                if (child != null) {
+                    child.invalidate();
+                }
+            }
+        }
     }
 
     @Override
diff --git a/src/com/android/mail/ui/TwoPaneController.java b/src/com/android/mail/ui/TwoPaneController.java
index 7a9605f3b..44600d3ad 100644
--- a/src/com/android/mail/ui/TwoPaneController.java
+++ b/src/com/android/mail/ui/TwoPaneController.java
@@ -653,9 +653,9 @@ public final class TwoPaneController extends AbstractActivityController implemen
         if (different && convList != null && conversation != null) {
             if (mCurrentConversationJustPeeking) {
                 convList.clearChoicesAndActivated();
-                convList.setSelected(conversation.position);
+                convList.setSelected(conversation);
             } else {
-                convList.setActivated(conversation.position, different);
+                convList.setActivated(conversation, different);
             }
         }
     }