am c90cc15b: Make saving a contact less confusing

* commit 'c90cc15b804fb00339a3b98e5c951549f9b03599':
  Make saving a contact less confusing

7 files changed, 71 insertions, 46 deletions

diff --git a/res/menu/edit_contact.xml b/res/menu/edit_contact.xml
index fe6d2b9c4..2ff5f8149 100644
--- a/res/menu/edit_contact.xml
+++ b/res/menu/edit_contact.xml
@@ -16,9 +16,10 @@
 
 <menu xmlns:android="http://schemas.android.com/apk/res/android">
     <item
-        android:id="@+id/menu_done"
-        android:alphabeticShortcut="\n"
-        android:showAsAction="always" />
+        android:id="@+id/menu_save"
+        android:showAsAction="always"
+        android:icon="@drawable/ic_done_wht_24dp"
+        android:title="@string/menu_save" />
 
     <item
         android:id="@+id/menu_split"
diff --git a/res/values/strings.xml b/res/values/strings.xml
index 7dbe6c10b..9e76bd383 100644
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -150,6 +150,9 @@
     <!-- Menu item that joins an aggregate with another aggregate -->
     <string name="menu_joinAggregate">Merge</string>
 
+    <!-- Menu item (in the action bar) to indicate that changes should be saved [CHAR LIMIT=20] -->
+    <string name="menu_save">Save</string>
+
     <!-- Heading of the Join Contact screen -->
     <string name="titleJoinContactDataWith">Join contacts</string>
 
diff --git a/src/com/android/contacts/activities/CompactContactEditorActivity.java b/src/com/android/contacts/activities/CompactContactEditorActivity.java
index c45c261b3..082bb74f3 100644
--- a/src/com/android/contacts/activities/CompactContactEditorActivity.java
+++ b/src/com/android/contacts/activities/CompactContactEditorActivity.java
@@ -60,7 +60,7 @@ public class CompactContactEditorActivity extends ContactEditorBaseActivity {
     @Override
     public void onBackPressed() {
         if (mFragment != null) {
-            mFragment.save(ContactEditor.SaveMode.CLOSE, /* backPressed =*/ true);
+            mFragment.revert();
         }
     }
 }
diff --git a/src/com/android/contacts/activities/ContactEditorBaseActivity.java b/src/com/android/contacts/activities/ContactEditorBaseActivity.java
index b9edabcda..12f1e961b 100644
--- a/src/com/android/contacts/activities/ContactEditorBaseActivity.java
+++ b/src/com/android/contacts/activities/ContactEditorBaseActivity.java
@@ -162,6 +162,12 @@ abstract public class ContactEditorBaseActivity extends ContactsActivity
         boolean save(int saveMode, boolean backPressed);
 
         /**
+         * If there are no unsaved changes, just close the editor, otherwise the user is prompted
+         * before discarding unsaved changes.
+         */
+        boolean revert();
+
+        /**
          * Invoked after the contact is saved.
          */
         void onSaveCompleted(boolean hadChanges, int saveMode, boolean saveSucceeded,
diff --git a/src/com/android/contacts/editor/CompactContactEditorFragment.java b/src/com/android/contacts/editor/CompactContactEditorFragment.java
index 23d5bb349..12b3b623e 100644
--- a/src/com/android/contacts/editor/CompactContactEditorFragment.java
+++ b/src/com/android/contacts/editor/CompactContactEditorFragment.java
@@ -36,6 +36,7 @@ import android.net.Uri;
 import android.os.Bundle;
 import android.util.Log;
 import android.view.LayoutInflater;
+import android.view.MenuItem;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.LinearLayout;
@@ -191,6 +192,14 @@ public class CompactContactEditorFragment extends ContactEditorBaseFragment impl
     }
 
     @Override
+    public boolean onOptionsItemSelected(MenuItem item) {
+        if (item.getItemId() == android.R.id.home) {
+            return revert();
+        }
+        return super.onOptionsItemSelected(item);
+    }
+
+    @Override
     protected void bindEditors() {
         if (!isReadyToBindEditors()) {
             return;
@@ -347,7 +356,7 @@ public class CompactContactEditorFragment extends ContactEditorBaseFragment impl
         if (isInsert) {
             // For inserts, prevent any changes from being saved when the base fragment is destroyed
             mStatus = Status.CLOSING;
-        } else if (hasPendingChanges()) {
+        } else if (hasPendingRawContactChanges()) {
             // Save whatever is in the form
             save(SaveMode.CLOSE, /* backPressed =*/ false);
         }
diff --git a/src/com/android/contacts/editor/ContactEditorBaseFragment.java b/src/com/android/contacts/editor/ContactEditorBaseFragment.java
index e262cee20..434ed56b1 100644
--- a/src/com/android/contacts/editor/ContactEditorBaseFragment.java
+++ b/src/com/android/contacts/editor/ContactEditorBaseFragment.java
@@ -743,7 +743,7 @@ abstract public class ContactEditorBaseFragment extends Fragment implements
         // This supports the keyboard shortcut to save changes to a contact but shouldn't be visible
         // because the custom action bar contains the "save" button now (not the overflow menu).
         // TODO: Find a better way to handle shortcuts, i.e. onKeyDown()?
-        final MenuItem doneMenu = menu.findItem(R.id.menu_done);
+        final MenuItem saveMenu = menu.findItem(R.id.menu_save);
         final MenuItem splitMenu = menu.findItem(R.id.menu_split);
         final MenuItem joinMenu = menu.findItem(R.id.menu_join);
         final MenuItem helpMenu = menu.findItem(R.id.menu_help);
@@ -753,21 +753,18 @@ abstract public class ContactEditorBaseFragment extends Fragment implements
         final MenuItem deleteMenu = menu.findItem(R.id.menu_delete);
 
         // Set visibility of menus
-        doneMenu.setVisible(false);
-
         // Discard menu is only available if at least one raw contact is editable
         discardMenu.setVisible(mState != null &&
                 mState.getFirstWritableRawContact(mContext) != null);
 
         // help menu depending on whether this is inserting or editing
-        if (Intent.ACTION_INSERT.equals(mAction) ||
-                ContactEditorBaseActivity.ACTION_INSERT.equals(mAction)) {
+        if (isInsert(mAction)) {
             HelpUtils.prepareHelpMenuItem(mContext, helpMenu, R.string.help_url_people_add);
+            discardMenu.setVisible(false);
             splitMenu.setVisible(false);
             joinMenu.setVisible(false);
             deleteMenu.setVisible(false);
-        } else if (Intent.ACTION_EDIT.equals(mAction) ||
-                ContactEditorBaseActivity.ACTION_EDIT.equals(mAction)) {
+        } else if (isEdit(mAction)) {
             HelpUtils.prepareHelpMenuItem(mContext, helpMenu, R.string.help_url_people_edit);
             // Split only if there is more than one raw (non-user profile) contact and doing so
             // won't result in an empty contact
@@ -795,8 +792,7 @@ abstract public class ContactEditorBaseFragment extends Fragment implements
     @Override
     public boolean onOptionsItemSelected(MenuItem item) {
         switch (item.getItemId()) {
-            case android.R.id.home:
-            case R.id.menu_done:
+            case R.id.menu_save:
                 return save(SaveMode.CLOSE, /* backPressed =*/ true);
             case R.id.menu_discard:
                 return revert();
@@ -823,7 +819,8 @@ abstract public class ContactEditorBaseFragment extends Fragment implements
         return false;
     }
 
-    private boolean revert() {
+    @Override
+    public boolean revert() {
         if (mState.isEmpty() || !hasPendingChanges()) {
             onCancelEditConfirmed();
         } else {
@@ -870,7 +867,8 @@ abstract public class ContactEditorBaseFragment extends Fragment implements
 
         // If we just started creating a new contact and haven't added any data, it's too
         // early to do a join
-        if (mState.size() == 1 && mState.get(0).isContactInsert() && !hasPendingChanges()) {
+        if (mState.size() == 1 && mState.get(0).isContactInsert()
+                && !hasPendingRawContactChanges()) {
             Toast.makeText(mContext, R.string.toast_join_with_empty_contact,
                     Toast.LENGTH_LONG).show();
             return true;
@@ -923,33 +921,7 @@ abstract public class ContactEditorBaseFragment extends Fragment implements
 
         // Determine if changes were made in the editor that need to be saved
         // See go/editing-read-only-contacts
-        boolean hasPendingChanges;
-        if (mReadOnlyNameEditorView == null || mReadOnlyDisplayName == null) {
-            hasPendingChanges = hasPendingChanges();
-        } else {
-            // We created a new raw contact delta with a default display name. We must test for
-            // pending changes while ignoring the default display name.
-            final String displayName = mReadOnlyNameEditorView.getDisplayName();
-            if (mReadOnlyDisplayName.equals(displayName)) {
-                // The user did not modify the default display name, erase it and
-                // check if the user made any other changes
-                mReadOnlyNameEditorView.setDisplayName(null);
-                if (hasPendingChanges()) {
-                    // Other changes were made to the aggregate contact, restore
-                    // the display name and proceed.
-                    mReadOnlyNameEditorView.setDisplayName(displayName);
-                    hasPendingChanges = true;
-                } else {
-                    // No other changes were made to the aggregate contact. Don't add back
-                    // the displayName so that a "bogus" contact is not created.
-                    hasPendingChanges = false;
-                }
-            } else {
-                hasPendingChanges = true;
-            }
-        }
-
-        if (!hasPendingChanges) {
+        if (!hasPendingChanges()) {
             if (mLookupUri == null && saveMode == SaveMode.RELOAD) {
                 // We don't have anything to save and there isn't even an existing contact yet.
                 // Nothing to do, simply go back to editing mode
@@ -1006,12 +978,42 @@ abstract public class ContactEditorBaseFragment extends Fragment implements
      * Return true if there are any edits to the current contact which need to
      * be saved.
      */
-    protected boolean hasPendingChanges() {
+    protected boolean hasPendingRawContactChanges() {
         final AccountTypeManager accountTypes = AccountTypeManager.getInstance(mContext);
         return RawContactModifier.hasChanges(mState, accountTypes);
     }
 
     /**
+     * Determines if changes were made in the editor that need to be saved, while taking into
+     * account that name changes are not realfor read-only contacts.
+     * See go/editing-read-only-contacts
+     */
+    protected boolean hasPendingChanges() {
+        if (mReadOnlyNameEditorView == null || mReadOnlyDisplayName == null) {
+            return hasPendingRawContactChanges();
+        }
+        // We created a new raw contact delta with a default display name. We must test for
+        // pending changes while ignoring the default display name.
+        final String displayName = mReadOnlyNameEditorView.getDisplayName();
+        if (mReadOnlyDisplayName.equals(displayName)) {
+            // The user did not modify the default display name, erase it and
+            // check if the user made any other changes
+            mReadOnlyNameEditorView.setDisplayName(null);
+            if (hasPendingRawContactChanges()) {
+                // Other changes were made to the aggregate contact, restore
+                // the display name and proceed.
+                mReadOnlyNameEditorView.setDisplayName(displayName);
+                return true;
+            } else {
+                // No other changes were made to the aggregate contact. Don't add back
+                // the displayName so that a "bogus" contact is not created.
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
      * Whether editor inputs and the options menu should be enabled.
      */
     protected boolean isEnabled() {
@@ -1679,4 +1681,9 @@ abstract public class ContactEditorBaseFragment extends Fragment implements
         return Intent.ACTION_INSERT.equals(action)
                 || ContactEditorBaseActivity.ACTION_INSERT.equals(action);
     }
+
+    protected static boolean isEdit(String action) {
+        return Intent.ACTION_EDIT.equals(action)
+                || ContactEditorBaseActivity.ACTION_EDIT.equals(action);
+    }
 }
diff --git a/src/com/android/contacts/editor/ContactEditorFragment.java b/src/com/android/contacts/editor/ContactEditorFragment.java
index 7b17d638d..1e06306e4 100644
--- a/src/com/android/contacts/editor/ContactEditorFragment.java
+++ b/src/com/android/contacts/editor/ContactEditorFragment.java
@@ -131,8 +131,7 @@ public class ContactEditorFragment extends ContactEditorBaseFragment implements
 
     @Override
     public boolean onOptionsItemSelected(MenuItem item) {
-        // Override the home/done options to return to the compact editor
-        if (item.getItemId() == android.R.id.home || item.getItemId() == R.id.menu_done) {
+        if (item.getItemId() == android.R.id.home) {
             return save(SaveMode.COMPACT, /* backPressed =*/ true);
         }
         return super.onOptionsItemSelected(item);