Consolidate OnePane, TwoPane, and Conversation Destructive actions.
Change-Id: Ieccc3fc1962572fec78dfb1ea91557e56f931a48
diff --git a/src/com/android/mail/providers/Folder.java b/src/com/android/mail/providers/Folder.java
index 6dae925..dcc41c2 100644
--- a/src/com/android/mail/providers/Folder.java
+++ b/src/com/android/mail/providers/Folder.java
@@ -512,8 +512,8 @@
}
/**
- * Returns true if the URI of the folder specified as the needle was found in the collection of
- * folders specified as the haystack. False otherwise. This method is safe to call with null
+ * Returns true if a conversation assigned to the needle will be assigned to the collection of
+ * folders in the haystack. False otherwise. This method is safe to call with null
* arguments.
* This method returns true under two circumstances
* <ul><li> If the URI of the needle was found in the collection of URIs that comprise the
@@ -525,11 +525,11 @@
* continue to appear in the Priority Inbox. However, the URI of Priority Inbox and Inbox will
* be different. So a direct equality check is insufficient.
* </li></ul>
- * @param haystack
- * @param needle
- * @return
+ * @param haystack a collection of folders, possibly overlapping
+ * @param needle a folder
+ * @return true if a conversation inside the needle will be in the folders in the haystack.
*/
- public final static boolean contains(Collection<Folder> haystack, Folder needle) {
+ public final static boolean containerIncludes(Collection<Folder> haystack, Folder needle) {
// If the haystack is empty, it cannot contain anything.
if (haystack == null || haystack.size() <= 0) {
return false;
diff --git a/src/com/android/mail/ui/AbstractActivityController.java b/src/com/android/mail/ui/AbstractActivityController.java
index c543f48..febaad6 100644
--- a/src/com/android/mail/ui/AbstractActivityController.java
+++ b/src/com/android/mail/ui/AbstractActivityController.java
@@ -689,9 +689,10 @@
/**
* Confirm (based on user's settings) and delete a conversation from the conversation list and
* from the database.
- * @param showDialog
- * @param confirmResource
- * @param action
+ * @param target the conversations to act upon
+ * @param showDialog true if a confirmation dialog is to be shown, false otherwise.
+ * @param confirmResource the resource ID of the string that is shown in the confirmation dialog
+ * @param action the action to perform after animating the deletion of the conversations.
*/
protected void confirmAndDelete(final Collection<Conversation> target, boolean showDialog,
int confirmResource, final DestructiveAction action) {
@@ -702,7 +703,8 @@
requestDelete(target, action);
}
};
- final CharSequence message = Utils.formatPlural(mContext, confirmResource, 1);
+ final CharSequence message = Utils.formatPlural(mContext, confirmResource,
+ target.size());
new AlertDialog.Builder(mActivity.getActivityContext()).setMessage(message)
.setPositiveButton(R.string.ok, onClick)
.setNegativeButton(R.string.cancel, null)
@@ -729,10 +731,8 @@
!Conversation.contains(target, mCurrentConversation)) {
final Conversation next = mTracker.getNextConversation(
Settings.getAutoAdvanceSetting(mAccount.settings));
- if (next != null) {
- showConversation(next);
- // TODO(viki): Change showConversation to allow for null inputs.
- }
+ LogUtils.d(LOG_TAG, "requestDelete: showing %s next.", next);
+ showConversation(next);
}
// No visible UI element handled it on our behalf. Perform the action ourself.
action.performAction();
@@ -1408,10 +1408,6 @@
* Destructive actions on Conversations. This class should only be created by controllers, and
* clients should only require {@link DestructiveAction}s, not specific implementations of the.
* Only the controllers should know what kind of destructive actions are being created.
- *
- * These Destructive Actions should not be used by themselves. These must be used by
- * classes that update the UI state like {@link OnePaneController.OnePaneDestructiveAction},
- * {@link TwoPaneController.TwoPaneDestructiveAction} and others.
*/
protected class ConversationAction implements DestructiveAction {
/**
@@ -1431,15 +1427,6 @@
* R.id.delete , R.id.mute, and R.id.report_spam.
* @param action
* @param target Conversation that we want to apply the action to.
- */
- public ConversationAction(int action, Collection<Conversation> target) {
- this(action, target, false);
- }
- /**
- * Create a listener object. action is one of four constants: R.id.y_button (archive),
- * R.id.delete , R.id.mute, and R.id.report_spam.
- * @param action
- * @param target Conversation that we want to apply the action to.
* @param isBatch whether the conversations are in the currently selected batch set.
*/
public ConversationAction(int action, Collection<Conversation> target, boolean isBatch) {
@@ -1457,6 +1444,10 @@
if (isPerformed()) {
return;
}
+ // Certain actions force a return to list.
+ boolean forceReturnToList = false;
+ // Enable undo for batch operations. Some actions disable the undo ability.
+ boolean undoEnabled = mIsSelectedSet;
LogUtils.d(LOG_TAG, "Target is: %s", mTarget);
switch (mAction) {
case R.id.archive:
@@ -1493,10 +1484,25 @@
mConversationListCursor.updateInt(mContext, mTarget,
ConversationColumns.PRIORITY, UIProvider.ConversationPriority.LOW);
break;
+ case R.id.inside_conversation_unread:
+ LogUtils.d(LOG_TAG, "Marking conversation unread: %s", mTarget);
+ mConversationListCursor.updateBoolean(mContext, mTarget,
+ ConversationColumns.READ, false);
+ forceReturnToList = true;
+ undoEnabled = false;
+ }
+ if (undoEnabled) {
+ onUndoAvailable(new UndoOperation(mTarget.size(), mAction));
+ }
+ // If the currently shown conversation is destroyed, show the next one.
+ final ConversationViewFragment convView = getConversationViewFragment();
+ if (convView != null && Conversation.contains(mTarget, mCurrentConversation)) {
+ final Conversation next = forceReturnToList ? null :
+ mTracker.getNextConversation(Settings.getAutoAdvanceSetting(mAccount.settings));
+ showConversation(next);
}
refreshConversationList();
if (mIsSelectedSet) {
- onUndoAvailable(new UndoOperation(mTarget.size(), mAction));
mSelectedSet.clear();
}
}
@@ -1514,12 +1520,26 @@
}
}
+ /**
+ * Get a destructive action for a menu action.
+ * This is a temporary method, to control the profusion of {@link DestructiveAction} classes
+ * that are created. Please do not copy this paradigm.
+ * @param action the resource ID of the menu action: R.id.delete, for example
+ * @param target the conversations to act upon.
+ * @return a {@link DestructiveAction} that performs the specified action.
+ */
+ protected final DestructiveAction getAction(int action, Collection<Conversation> target) {
+ final DestructiveAction da = new ConversationAction(action, target, false);
+ registerDestructiveAction(da);
+ return da;
+ }
+
// Called from the FolderSelectionDialog after a user is done selecting folders to assign the
// conversations to.
@Override
public final void onFolderChangesCommit(
Collection<Folder> folders, Collection<Conversation> target) {
- final boolean isDestructive = !Folder.contains(folders, mFolder);
+ final boolean isDestructive = !Folder.containerIncludes(folders, mFolder);
LogUtils.d(LOG_TAG, "onFolderChangesCommit: isDestructive = %b", isDestructive);
if (isDestructive) {
for (final Conversation c : target) {
@@ -1528,7 +1548,7 @@
}
final DestructiveAction folderChange = getFolderChange(target, folders, isDestructive);
// Update the UI elements depending no their visibility and availability
- // TODO(viki): Consolidate this into requestDelete.
+ // TODO(viki): Consolidate this into a single method requestDelete.
if (isDestructive) {
requestDelete(target, folderChange);
} else {
@@ -1537,7 +1557,7 @@
}
@Override
- public void onRefreshRequired() {
+ public final void onRefreshRequired() {
if (mIsConversationListScrolling) {
LogUtils.d(LOG_TAG, "onRefreshRequired: delay until scrolling done");
return;
@@ -1558,7 +1578,7 @@
* {@inheritDoc}
*/
@Override
- public void onRefreshReady() {
+ public final void onRefreshReady() {
if (!mIsConversationListScrolling) {
// Swap cursors
mConversationListCursor.sync();
@@ -1567,12 +1587,15 @@
}
@Override
- public void onDataSetChanged() {
+ public final void onDataSetChanged() {
updateConversationListFragment();
mConversationListObservable.notifyChanged();
}
- private void updateConversationListFragment() {
+ /**
+ * If the Conversation List Fragment is visible, updates the fragment.
+ */
+ private final void updateConversationListFragment() {
final ConversationListFragment convList = getConversationListFragment();
if (convList != null) {
refreshConversationList();
@@ -1858,11 +1881,10 @@
/**
* Register a destructive action with the controller. This performs the previous destructive
* action as a side effect. This method is final because we don't want the child classes to
- * embellish this method any more. This is a temporary workaround to reduce the number of
- * {@link DestructiveAction} classes. Please do not copy this paradigm.
+ * embellish this method any more.
* @param action
*/
- protected final void registerDestructiveAction(DestructiveAction action) {
+ private final void registerDestructiveAction(DestructiveAction action) {
// TODO(viki): This is not a good idea. The best solution is for clients to request a
// destructive action from the controller and for the controller to own the action. This is
// a half-way solution while refactoring DestructiveAction.
diff --git a/src/com/android/mail/ui/ActivityController.java b/src/com/android/mail/ui/ActivityController.java
index ac42dfe..d35e189 100644
--- a/src/com/android/mail/ui/ActivityController.java
+++ b/src/com/android/mail/ui/ActivityController.java
@@ -219,23 +219,25 @@
void showConversationList(ConversationListContext listContext);
/**
- * Show the conversation provided here.
- * @param conversation conversation to display.
+ * Show the conversation provided here. If the conversation is null, this is a request to pop
+ * <em>out</em> of conversation view mode and head back to conversation list mode, or whatever
+ * should best show in its place.
+ * @param conversation conversation to display, possibly null.
*/
void showConversation(Conversation conversation);
/**
- * Show the wait for account initilization mode.
+ * Show the wait for account initialization mode.
*/
public void showWaitForInitialization();
/**
- * Dismiss the wait for account initization mode.
+ * Dismiss the wait for account initialization mode.
*/
public void hideWaitForInitialization();
/**
- * Update the wait for account intization mode.
+ * Update the wait for account initialization mode.
*/
public void updateWaitMode();
@@ -294,13 +296,6 @@
void onConversationSeen(Conversation conv);
/**
- * Returns the destructive action that can change the folders for a specific conversation.
- * @param target the conversations to act upon.
- * @return
- */
- public abstract DestructiveAction getFolderDestructiveAction(Collection<Conversation> target);
-
- /**
* Load the default inbox associated with the current account.
*/
public abstract void loadAccountInbox();
diff --git a/src/com/android/mail/ui/ConversationListFragment.java b/src/com/android/mail/ui/ConversationListFragment.java
index 9ddac3b..a7b02f7 100644
--- a/src/com/android/mail/ui/ConversationListFragment.java
+++ b/src/com/android/mail/ui/ConversationListFragment.java
@@ -404,12 +404,21 @@
*/
protected void viewConversation(int position) {
LogUtils.d(LOG_TAG, "ConversationListFragment.viewConversation(%d)", position);
+ setSelected(position);
+ final ConversationCursor cursor = getConversationListCursor();
+ if (cursor != null && cursor.moveToPosition(position)) {
+ final Conversation conv = new Conversation(cursor);
+ conv.position = position;
+ mCallbacks.onConversationSelected(conv);
+ }
+ }
+
+ /**
+ * Sets the selected position (the highlighted conversation) to the position provided here.
+ * @param position
+ */
+ protected final void setSelected(int position) {
getListView().setItemChecked(position, true);
- ConversationCursor conversationListCursor = getConversationListCursor();
- conversationListCursor.moveToPosition(position);
- Conversation conv = new Conversation(conversationListCursor);
- conv.position = position;
- mCallbacks.onConversationSelected(conv);
}
private ConversationCursor getConversationListCursor() {
diff --git a/src/com/android/mail/ui/OnePaneController.java b/src/com/android/mail/ui/OnePaneController.java
index 84dc6ab..2ca5dc8 100644
--- a/src/com/android/mail/ui/OnePaneController.java
+++ b/src/com/android/mail/ui/OnePaneController.java
@@ -190,6 +190,13 @@
@Override
public void showConversation(Conversation conversation) {
super.showConversation(conversation);
+ if (conversation == null) {
+ // This is a request to remove the conversation view, and pop back the view stack.
+ // If we are in conversation list view already, this should be a safe thing to do, so
+ // we don't check viewmode.
+ transitionBackToConversationListMode();
+ return;
+ }
disableCabMode();
if (mConvListContext != null && mConvListContext.isSearchResult()) {
mViewMode.enterSearchResultsConversationMode();
@@ -422,7 +429,7 @@
}
case R.id.inside_conversation_unread:
// Mark as unread and advance.
- performInsideConversationUnread(target);
+ requestUpdate(target, getAction(R.id.inside_conversation_unread, target));
break;
case R.id.mark_important:
updateCurrentConversation(ConversationColumns.PRIORITY,
@@ -445,109 +452,6 @@
return handled || super.onOptionsItemSelected(item);
}
- // TODO: If when the conversation was opened, some of the messages were unread,
- // this is supposed to restore that state. Otherwise, this should mark all
- // messages as unread
- private void performInsideConversationUnread(Collection<Conversation> target) {
- updateCurrentConversation(ConversationColumns.READ, false);
- if (returnToList()) {
- onBackPressed();
- } else {
- final DestructiveAction action = getAction(R.id.inside_conversation_unread, target);
- action.performAction();
- }
- }
-
- /**
- * Destroy conversations and update the UI state for a one pane activity.
- */
- private class OnePaneDestructiveAction implements DestructiveAction {
- /** Whether this destructive action has already been performed */
- private boolean mCompleted;
- /** Menu Id that created this action */
- private final int mId;
- /** Action that updates the underlying database to modify the conversation. */
- private final DestructiveAction mAction;
-
- public OnePaneDestructiveAction(int action, Collection<Conversation> target) {
- mAction = new ConversationAction(action, target);
- mId = action;
- }
-
- @Override
- public void performAction() {
- if (isPerformed()) {
- return;
- }
- mAction.performAction();
- switch (mViewMode.getMode()) {
- case ViewMode.CONVERSATION:
- final Conversation next = mTracker.getNextConversation(
- Settings.getAutoAdvanceSetting(mAccount.settings));
- if (next != null) {
- showConversation(next);
- onUndoAvailable(new UndoOperation(1, mId));
- } else {
- // No next conversation, we should got back to conversation list.
- transitionBackToConversationListMode();
- }
- break;
- case ViewMode.CONVERSATION_LIST:
- if (mId != R.id.inside_conversation_unread) {
- onUndoAvailable(new UndoOperation(1, mId));
- }
- refreshConversationList();
- break;
- }
- }
- /**
- * Returns true if this action has been performed, false otherwise.
- * @return
- */
- private synchronized boolean isPerformed() {
- if (mCompleted) {
- return true;
- }
- mCompleted = true;
- return false;
- }
- }
-
- /**
- * Get a destructive action specific to the {@link OnePaneController}.
- * This is a temporary method, to control the profusion of {@link DestructiveAction} classes
- * that are created. Please do not copy this paradigm.
- * TODO(viki): Resolve the various actions and clean up their calling sequence.
- * @param action
- * @return
- */
- private final DestructiveAction getAction(int action, Collection<Conversation> target) {
- final DestructiveAction da = new OnePaneDestructiveAction(action, target);
- registerDestructiveAction(da);
- return da;
- }
-
- /**
- * Returns true if we need to return back to conversation list based on the current
- * AutoAdvance setting and the number of messages in the list.
- * @return true if we need to return back to conversation list, false otherwise.
- */
- private boolean returnToList() {
- final int pref = Settings.getAutoAdvanceSetting(mAccount.settings);
- final int position = mCurrentConversation.position;
- final boolean moveToNewer = (pref == AutoAdvance.NEWER && (position - 1 >= 0));
- final boolean moveToOlder = (pref == AutoAdvance.OLDER && mConversationListCursor != null
- && (position + 1 < mConversationListCursor.getCount()));
- final boolean canMove = moveToNewer || moveToOlder;
- // Return true if we cannot move forward or back, or if the user wants to go back to list.
- return pref == AutoAdvance.LIST || !canMove;
- }
-
- @Override
- public DestructiveAction getFolderDestructiveAction(Collection<Conversation> target) {
- return getAction(R.id.change_folder, target);
- }
-
@Override
public void onUndoAvailable(UndoOperation op) {
if (op != null && mAccount.supportsCapability(UIProvider.AccountCapabilities.UNDO)) {
diff --git a/src/com/android/mail/ui/TwoPaneController.java b/src/com/android/mail/ui/TwoPaneController.java
index d41a029..94da197 100644
--- a/src/com/android/mail/ui/TwoPaneController.java
+++ b/src/com/android/mail/ui/TwoPaneController.java
@@ -210,18 +210,28 @@
@Override
public void showConversation(Conversation conversation) {
+ super.showConversation(conversation);
if (mActivity == null) {
return;
}
- super.showConversation(conversation);
- int mode = mViewMode.getMode();
+ if (conversation == null) {
+ // This is a request to remove the conversation view and show the conversation list
+ // fragment instead.
+ onBackPressed();
+ return;
+ }
+ final int mode = mViewMode.getMode();
if (mode == ViewMode.SEARCH_RESULTS_LIST || mode == ViewMode.SEARCH_RESULTS_CONVERSATION) {
mViewMode.enterSearchResultsConversationMode();
} else {
mViewMode.enterConversationMode();
}
-
mPagerController.show(mAccount, mFolder, conversation);
+ final ConversationListFragment convList = getConversationListFragment();
+ if (convList != null) {
+ LogUtils.d(LOG_TAG, "showConversation: Selecting position %d.", conversation.position);
+ convList.setSelected(conversation.position);
+ }
}
@Override
@@ -381,78 +391,6 @@
return handled || super.onOptionsItemSelected(item);
}
- /**
- * An object that performs an action on the conversation database. This is a
- * {@link DestructiveAction}: this is called <b>after</b> the conversation list has animated
- * the conversation away. Once the animation is completed, the {@link #performAction()}
- * method is called which performs the correct data operation.
- */
- private class TwoPaneDestructiveAction implements DestructiveAction {
- /** Whether this destructive action has already been performed */
- private boolean mCompleted;
- private final int mId;
- /** Action that updates the underlying database to modify the conversation. */
- private final DestructiveAction mAction;
-
- public TwoPaneDestructiveAction(int action, Collection<Conversation> target) {
- mAction = new ConversationAction(action, target);
- mId = action;
- }
-
- @Override
- public void performAction() {
- if (isPerformed()) {
- return;
- }
- final Conversation nextConversation = mTracker.getNextConversation(
- Settings.getAutoAdvanceSetting(mAccount.settings));
- if (nextConversation != null) {
- // We have a conversation to auto advance to.
- final ConversationListFragment convList = getConversationListFragment();
- if (convList != null) {
- convList.viewConversation(nextConversation.position);
- }
- onUndoAvailable(new UndoOperation(1, mId));
- } else {
- // We don't have a conversation to show: show conversation list instead.
- onBackPressed();
- onUndoAvailable(new UndoOperation(1, mId));
- }
- mAction.performAction();
- refreshConversationList();
- }
- /**
- * Returns true if this action has been performed, false otherwise.
- * @return
- */
- private synchronized boolean isPerformed() {
- if (mCompleted) {
- return true;
- }
- mCompleted = true;
- return false;
- }
- }
-
- /**
- * Get a destructive action specific to the {@link TwoPaneController}.
- * This is a temporary method, to control the profusion of {@link DestructiveAction} classes
- * that are created. Please do not copy this paradigm.
- * TODO(viki): Resolve the various actions and clean up their calling sequence.
- * @param action
- * @return
- */
- private final DestructiveAction getAction(int action, Collection<Conversation> target) {
- final DestructiveAction da = new TwoPaneDestructiveAction(action, target);
- registerDestructiveAction(da);
- return da;
- }
-
- @Override
- public DestructiveAction getFolderDestructiveAction(Collection<Conversation> target) {
- return getAction(R.id.change_folder, target);
- }
-
@Override
public void onUndoAvailable(UndoOperation op) {
final int mode = mViewMode.getMode();