From 46291d6b3ef5ee9ed2154eb4dfd5becd54d176c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= <dominik@dominikschuermann.de>
Date: Sat, 18 Jan 2014 19:54:27 +0100
Subject: [PATCH] Much more usable sign activity

---
 .../select_secret_key_layout_fragment.xml     |  4 ++
 .../res/layout/sign_key_activity.xml          | 64 +++++++++++--------
 OpenPGP-Keychain/res/values/strings.xml       |  6 +-
 .../ui/SelectSecretKeyLayoutFragment.java     | 60 +++++++++--------
 .../keychain/ui/SignKeyActivity.java          | 61 ++++++++----------
 5 files changed, 105 insertions(+), 90 deletions(-)

diff --git a/OpenPGP-Keychain/res/layout/select_secret_key_layout_fragment.xml b/OpenPGP-Keychain/res/layout/select_secret_key_layout_fragment.xml
index 7557a2830..11429ea05 100644
--- a/OpenPGP-Keychain/res/layout/select_secret_key_layout_fragment.xml
+++ b/OpenPGP-Keychain/res/layout/select_secret_key_layout_fragment.xml
@@ -22,12 +22,16 @@
         android:orientation="vertical"
         android:paddingLeft="16dp" >
 
+        <!-- Has been made focusable to display error messages with setError -->
+
         <TextView
             android:id="@+id/select_secret_key_user_id"
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"
             android:layout_gravity="right"
             android:ellipsize="end"
+            android:focusable="true"
+            android:focusableInTouchMode="true"
             android:singleLine="true"
             android:text="@string/api_settings_no_key"
             android:textAppearance="?android:attr/textAppearanceMedium" />
diff --git a/OpenPGP-Keychain/res/layout/sign_key_activity.xml b/OpenPGP-Keychain/res/layout/sign_key_activity.xml
index ada4572ca..1bb2e6bab 100644
--- a/OpenPGP-Keychain/res/layout/sign_key_activity.xml
+++ b/OpenPGP-Keychain/res/layout/sign_key_activity.xml
@@ -1,51 +1,61 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:bootstrapbutton="http://schemas.android.com/apk/res-auto"
+    xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
     android:layout_height="match_parent"
     android:orientation="vertical"
-    android:padding="10dp" >
+    android:padding="16dp" >
 
-    <include layout="@layout/select_secret_key_layout_fragment" />
+    <TextView
+        style="@style/SectionHeader"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_marginBottom="4dp"
+        android:text="@string/section_signing_key" />
 
-    <LinearLayout
+    <fragment
+        android:id="@+id/sign_key_select_key_fragment"
+        android:name="org.sufficientlysecure.keychain.ui.SelectSecretKeyLayoutFragment"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
-        android:orientation="horizontal" >
+        android:layout_marginBottom="4dp"
+        android:layout_marginTop="4dp"
+        tools:layout="@layout/select_secret_key_layout_fragment" />
 
-        <TextView
-            android:layout_width="wrap_content"
-            android:layout_height="wrap_content"
-            android:text="@string/label_send_key"
-            android:textAppearance="?android:attr/textAppearanceMedium" />
+    <TextView
+        style="@style/SectionHeader"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_marginBottom="4dp"
+        android:layout_marginTop="14dp"
+        android:text="@string/section_upload_key" />
 
-        <CheckBox
-            android:id="@+id/sign_key_upload_checkbox"
-            android:layout_width="wrap_content"
-            android:layout_height="wrap_content"
-            android:checked="true"
-            android:text="" />
-    </LinearLayout>
+    <CheckBox
+        android:id="@+id/sign_key_upload_checkbox"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_marginBottom="4dp"
+        android:layout_marginTop="4dp"
+        android:checked="false"
+        android:text="@string/label_send_key" />
 
-    <LinearLayout
+    <Spinner
+        android:id="@+id/sign_key_keyserver"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
-        android:orientation="horizontal" >
-
-        <Spinner
-            android:id="@+id/sign_key_keyserver"
-            android:layout_width="0dip"
-            android:layout_height="wrap_content"
-            android:layout_weight="1" />
-    </LinearLayout>
+        android:layout_marginBottom="4dp"
+        android:layout_marginTop="4dp"
+        android:enabled="false" />
 
     <com.beardedhen.androidbootstrap.BootstrapButton
         android:id="@+id/sign_key_sign_button"
         android:layout_width="match_parent"
         android:layout_height="60dp"
-        android:padding="4dp"
+        android:layout_marginBottom="4dp"
+        android:layout_marginTop="14dp"
         android:text="@string/btn_sign"
-        bootstrapbutton:bb_icon_left="fa-lock"
+        bootstrapbutton:bb_icon_left="fa-pencil"
         bootstrapbutton:bb_type="info" />
 
 </LinearLayout>
\ No newline at end of file
diff --git a/OpenPGP-Keychain/res/values/strings.xml b/OpenPGP-Keychain/res/values/strings.xml
index 190dcfa53..21b834f47 100644
--- a/OpenPGP-Keychain/res/values/strings.xml
+++ b/OpenPGP-Keychain/res/values/strings.xml
@@ -57,6 +57,8 @@
     <string name="section_master_key">Master Key</string>
     <string name="section_master_user_id">Master User ID</string>
     <string name="section_actions">Actions</string>
+    <string name="section_signing_key">Your Key used for Signing</string>
+    <string name="section_upload_key">Upload Key</string>
 
     <!-- button -->
     <string name="btn_sign_to_clipboard">Sign (Clipboard)</string>
@@ -137,7 +139,7 @@
     <string name="label_name">Name</string>
     <string name="label_comment">Comment</string>
     <string name="label_email">Email</string>
-    <string name="label_send_key">Send Key to Server?</string>
+    <string name="label_send_key">Upload key to selected keyserver after signing</string>
     <string name="label_fingerprint">Fingerprint</string>
     <string name="no_keys_selected">Select</string>
     <string name="one_key_selected">1 Selected</string>
@@ -222,6 +224,8 @@
     <string name="list_empty">This list is empty!</string>
     <string name="nfc_successfull">Successfully sent key with NFC Beam!</string>
     <string name="key_copied_to_clipboard">Key has been copied to the clipboard!</string>
+    <string name="key_has_already_been_signed">Key has already been signed!</string>
+    <string name="select_key_to_sign">Please select a key to be used for signing!</string>
 
     <!--
          errors
diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/ui/SelectSecretKeyLayoutFragment.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/ui/SelectSecretKeyLayoutFragment.java
index 996071a31..221325613 100644
--- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/ui/SelectSecretKeyLayoutFragment.java
+++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/ui/SelectSecretKeyLayoutFragment.java
@@ -54,32 +54,6 @@ public class SelectSecretKeyLayoutFragment extends Fragment {
         mCallback = callback;
     }
 
-    /**
-     * Inflate the layout for this fragment
-     */
-    @Override
-    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
-        View view = inflater.inflate(R.layout.select_secret_key_layout_fragment, container, false);
-
-        mKeyUserId = (TextView) view.findViewById(R.id.select_secret_key_user_id);
-        mKeyUserIdRest = (TextView) view.findViewById(R.id.select_secret_key_user_id_rest);
-        mSelectKeyButton = (BootstrapButton) view
-                .findViewById(R.id.select_secret_key_select_key_button);
-        mSelectKeyButton.setOnClickListener(new OnClickListener() {
-            @Override
-            public void onClick(View v) {
-                selectSecretKey();
-            }
-        });
-
-        return view;
-    }
-
-    private void selectSecretKey() {
-        Intent intent = new Intent(getActivity(), SelectSecretKeyActivity.class);
-        startActivityForResult(intent, REQUEST_CODE_SELECT_KEY);
-    }
-
     public void selectKey(long secretKeyId) {
         if (secretKeyId == Id.key.none) {
             mKeyUserId.setText(R.string.api_settings_no_key);
@@ -105,6 +79,37 @@ public class SelectSecretKeyLayoutFragment extends Fragment {
         }
     }
 
+    public void setError(String error) {
+        mKeyUserId.requestFocus();
+        mKeyUserId.setError(error);
+    }
+
+    /**
+     * Inflate the layout for this fragment
+     */
+    @Override
+    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
+        View view = inflater.inflate(R.layout.select_secret_key_layout_fragment, container, false);
+
+        mKeyUserId = (TextView) view.findViewById(R.id.select_secret_key_user_id);
+        mKeyUserIdRest = (TextView) view.findViewById(R.id.select_secret_key_user_id_rest);
+        mSelectKeyButton = (BootstrapButton) view
+                .findViewById(R.id.select_secret_key_select_key_button);
+        mSelectKeyButton.setOnClickListener(new OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                startSelectKeyActivity();
+            }
+        });
+
+        return view;
+    }
+
+    private void startSelectKeyActivity() {
+        Intent intent = new Intent(getActivity(), SelectSecretKeyActivity.class);
+        startActivityForResult(intent, REQUEST_CODE_SELECT_KEY);
+    }
+
     @Override
     public void onActivityResult(int requestCode, int resultCode, Intent data) {
         switch (requestCode & 0xFFFF) {
@@ -116,6 +121,9 @@ public class SelectSecretKeyLayoutFragment extends Fragment {
 
                 selectKey(secretKeyId);
 
+                // remove displayed errors
+                mKeyUserId.setError(null);
+
                 // give value back to callback
                 mCallback.onKeySelected(secretKeyId);
             }
diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/ui/SignKeyActivity.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/ui/SignKeyActivity.java
index a6390f9c9..3370a0564 100644
--- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/ui/SignKeyActivity.java
+++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/ui/SignKeyActivity.java
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2014 Dominik Schürmann <dominik@dominikschuermann.de>
  * Copyright (C) 2011 Senecaso
  * 
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -21,7 +22,6 @@ import java.util.Iterator;
 import org.spongycastle.openpgp.PGPPublicKeyRing;
 import org.spongycastle.openpgp.PGPSignature;
 import org.sufficientlysecure.keychain.Constants;
-import org.sufficientlysecure.keychain.Id;
 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.helper.Preferences;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
@@ -41,7 +41,6 @@ import android.os.Messenger;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.widget.ArrayAdapter;
-import android.widget.Button;
 import android.widget.CheckBox;
 import android.widget.CompoundButton;
 import android.widget.CompoundButton.OnCheckedChangeListener;
@@ -57,9 +56,10 @@ import com.beardedhen.androidbootstrap.BootstrapButton;
  * 
  * signs the specified public key with the specified secret master key
  */
-public class SignKeyActivity extends SherlockFragmentActivity {
+public class SignKeyActivity extends SherlockFragmentActivity implements
+        SelectSecretKeyLayoutFragment.SelectSecretKeyCallback {
 
-    public static final String EXTRA_KEY_ID = "keyId";
+    public static final String EXTRA_KEY_ID = "key_id";
 
     private long mPubKeyId = 0;
     private long mMasterKeyId = 0;
@@ -67,7 +67,8 @@ public class SignKeyActivity extends SherlockFragmentActivity {
     private BootstrapButton mSignButton;
     private CheckBox mUploadKeyCheckbox;
     private Spinner mSelectKeyserverSpinner;
-    private BootstrapButton mSelectKeyButton;
+
+    private SelectSecretKeyLayoutFragment mSelectKeyFragment;
 
     @Override
     protected void onCreate(Bundle savedInstanceState) {
@@ -80,6 +81,10 @@ public class SignKeyActivity extends SherlockFragmentActivity {
         actionBar.setDisplayHomeAsUpEnabled(false);
         actionBar.setHomeButtonEnabled(false);
 
+        mSelectKeyFragment = (SelectSecretKeyLayoutFragment) getSupportFragmentManager()
+                .findFragmentById(R.id.sign_key_select_key_fragment);
+        mSelectKeyFragment.setCallback(this);
+
         mSelectKeyserverSpinner = (Spinner) findViewById(R.id.sign_key_keyserver);
         ArrayAdapter<String> adapter = new ArrayAdapter<String>(this,
                 android.R.layout.simple_spinner_item, Preferences.getPreferences(this)
@@ -107,26 +112,24 @@ public class SignKeyActivity extends SherlockFragmentActivity {
         });
 
         mSignButton = (BootstrapButton) findViewById(R.id.sign_key_sign_button);
-        mSignButton.setEnabled(false); // disabled until the user selects a key to sign with
         mSignButton.setOnClickListener(new OnClickListener() {
 
             @Override
             public void onClick(View v) {
                 if (mPubKeyId != 0) {
-                    initiateSigning();
+                    if (mMasterKeyId == 0) {
+                        mSelectKeyFragment.setError(getString(R.string.select_key_to_sign));
+                    } else {
+                        initiateSigning();
+                    }
                 }
             }
         });
 
         mPubKeyId = getIntent().getLongExtra(EXTRA_KEY_ID, 0);
         if (mPubKeyId == 0) {
-            finish(); // nothing to do if we dont know what key to sign
-        } else {
-            // kick off the SecretKey selection activity so the user chooses which key to sign with
-            // first
-            Intent intent = new Intent(this, SelectSecretKeyActivity.class);
-            intent.putExtra(SelectSecretKeyActivity.EXTRA_FILTER_CERTIFY, true);
-            startActivityForResult(intent, Id.request.secret_keys);
+            Log.e(Constants.TAG, "No pub key id given!");
+            finish();
         }
     }
 
@@ -188,7 +191,8 @@ public class SignKeyActivity extends SherlockFragmentActivity {
                     startSigning();
                 }
             } else {
-                Toast.makeText(this, "Key has already been signed", Toast.LENGTH_SHORT).show();
+                Toast.makeText(this, R.string.key_has_already_been_signed, Toast.LENGTH_SHORT)
+                        .show();
 
                 setResult(RESULT_CANCELED);
                 finish();
@@ -226,8 +230,7 @@ public class SignKeyActivity extends SherlockFragmentActivity {
                             Toast.LENGTH_SHORT).show();
 
                     // check if we need to send the key to the server or not
-                    CheckBox sendKey = (CheckBox) findViewById(R.id.sign_key_upload_checkbox);
-                    if (sendKey.isChecked()) {
+                    if (mUploadKeyCheckbox.isChecked()) {
                         /*
                          * upload the newly signed key to the key server
                          */
@@ -295,25 +298,11 @@ public class SignKeyActivity extends SherlockFragmentActivity {
         startService(intent);
     }
 
+    /**
+     * callback from select key fragment
+     */
     @Override
-    protected void onActivityResult(int requestCode, int resultCode, Intent data) {
-        switch (requestCode) {
-        case Id.request.secret_keys: {
-            if (resultCode == RESULT_OK) {
-                Bundle bundle = data.getExtras();
-                mMasterKeyId = bundle.getLong(SelectSecretKeyActivity.RESULT_EXTRA_MASTER_KEY_ID);
-
-                // re-enable the sign button so the user can initiate the sign process
-                Button sign = (Button) findViewById(R.id.sign_key_sign_button);
-                sign.setEnabled(true);
-            }
-
-            break;
-        }
-
-        default: {
-            super.onActivityResult(requestCode, resultCode, data);
-        }
-        }
+    public void onKeySelected(long secretKeyId) {
+        mMasterKeyId = secretKeyId;
     }
 }