From c0b914500d39f680a71566924f35f7ff9df47fad Mon Sep 17 00:00:00 2001
From: Phillip Hsu <philliphsu@outlook.com>
Date: Sat, 16 Jul 2016 00:39:39 -0700
Subject: [PATCH] Fixed bug where clearing numpad did not disable the FAB

---
 .../clock2/editalarm/NumpadTimePicker.java    | 12 +++++--
 .../editalarm/NumpadTimePickerDialog.java     | 17 ++++++----
 .../res/layout/dialog_time_picker_numpad.xml  | 33 ++++++++-----------
 3 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/app/src/main/java/com/philliphsu/clock2/editalarm/NumpadTimePicker.java b/app/src/main/java/com/philliphsu/clock2/editalarm/NumpadTimePicker.java
index 24192d2..5f78bdb 100644
--- a/app/src/main/java/com/philliphsu/clock2/editalarm/NumpadTimePicker.java
+++ b/app/src/main/java/com/philliphsu/clock2/editalarm/NumpadTimePicker.java
@@ -92,8 +92,8 @@ public class NumpadTimePicker extends GridLayoutNumpad implements TimePicker {
     @Override
     protected void onDigitsCleared() {
         mFormattedInput.delete(0, mFormattedInput.length());
-        updateNumpadStates();
         mAmPmState = UNSPECIFIED;
+        updateNumpadStates(); // TOneverDO: before resetting mAmPmState to UNSPECIFIED
         super.onDigitsCleared();
     }
 
@@ -155,8 +155,14 @@ public class NumpadTimePicker extends GridLayoutNumpad implements TimePicker {
      * state must be set.
      */
     public boolean checkTimeValid() {
-        if (mAmPmState == UNSPECIFIED || mAmPmState == HRS_24 && count() < 3)
+        // While the test looks bare, it is actually comprehensive.
+        // mAmPmState will remain UNSPECIFIED until a legal
+        // sequence of digits is inputted, no matter the clock system in use.
+        // TODO: So if that's the case, do we actually need 'count() < 3' here? Or better yet,
+        // can we simplify the code to just 'return mAmPmState != UNSPECIFIED'?
+        if (mAmPmState == UNSPECIFIED || mAmPmState == HRS_24 && count() < 3) {
             return false;
+        }
         // AM or PM can only be set if the time was already valid previously, so we don't need
         // to check for them.
         return true;
@@ -306,7 +312,7 @@ public class NumpadTimePicker extends GridLayoutNumpad implements TimePicker {
             }
         } else if (count() == MAX_DIGITS) {
             int colonAt = mFormattedInput.indexOf(":");
-            // Since we now batch updating the formatted input whenever
+            // Since we now batch update the formatted input whenever
             // digits are inserted, the colon may legitimately not be
             // present in the formatted input when this is initialized.
             if (colonAt != -1) {
diff --git a/app/src/main/java/com/philliphsu/clock2/editalarm/NumpadTimePickerDialog.java b/app/src/main/java/com/philliphsu/clock2/editalarm/NumpadTimePickerDialog.java
index 1878d00..d53e27b 100644
--- a/app/src/main/java/com/philliphsu/clock2/editalarm/NumpadTimePickerDialog.java
+++ b/app/src/main/java/com/philliphsu/clock2/editalarm/NumpadTimePickerDialog.java
@@ -2,6 +2,7 @@ package com.philliphsu.clock2.editalarm;
 
 import android.os.Bundle;
 import android.support.v4.app.DialogFragment;
+import android.support.v7.widget.Toolbar;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -12,7 +13,6 @@ import com.philliphsu.clock2.R;
 
 import butterknife.Bind;
 import butterknife.ButterKnife;
-import butterknife.OnClick;
 import butterknife.OnTouch;
 
 /**
@@ -94,6 +94,16 @@ public class NumpadTimePickerDialog extends DialogFragment implements NumpadTime
         View view = inflater.inflate(R.layout.dialog_time_picker_numpad, container, false);
         ButterKnife.bind(this, view);
 
+        Toolbar toolbar = ButterKnife.findById(view, R.id.toolbar);
+        toolbar.setNavigationIcon(android.R.drawable.ic_menu_close_clear_cancel);
+        // Can't do a method bind on the navigation icon because we don't own it
+        toolbar.setNavigationOnClickListener(new View.OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                dismiss();
+            }
+        });
+
         // Can't do a method bind because the FAB is not part of this dialog's layout
         // Also can't do the bind in the Numpad's class, because it doesn't have access to
         // the OnTimeSetListener callback contained here or the dialog's dismiss()
@@ -146,11 +156,6 @@ public class NumpadTimePickerDialog extends DialogFragment implements NumpadTime
         return true;
     }
 
-    @OnClick(R.id.cancel_icon)
-    void myCancel() {
-        dismiss();
-    }
-
     private void updateInputText(String inputText) {
         mInputField.setText(inputText);
         // Move the cursor
diff --git a/app/src/main/res/layout/dialog_time_picker_numpad.xml b/app/src/main/res/layout/dialog_time_picker_numpad.xml
index 5f28480..76f6515 100644
--- a/app/src/main/res/layout/dialog_time_picker_numpad.xml
+++ b/app/src/main/res/layout/dialog_time_picker_numpad.xml
@@ -1,21 +1,15 @@
 <?xml version="1.0" encoding="utf-8"?>
-<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
               android:layout_width="match_parent"
-              android:layout_height="match_parent"
-              android:orientation="vertical">
+              android:layout_height="match_parent">
 
-    <RelativeLayout
+    <android.support.v7.widget.Toolbar
+        android:id="@+id/toolbar"
         android:layout_width="match_parent"
         android:layout_height="@dimen/header_height"
-        android:background="@color/colorPrimary">
-
-        <ImageButton
-            android:id="@+id/cancel_icon"
-            android:layout_width="@dimen/cancel_icon_size"
-            android:layout_height="match_parent"
-            android:src="@android:drawable/ic_menu_close_clear_cancel"
-            android:background="?selectableItemBackground"
-            android:layout_alignParentStart="true"/>
+        android:minHeight="@dimen/header_height"
+        android:background="@color/colorPrimary"
+        android:layout_alignParentTop="true">
 
         <!-- We want this to be an EditText because the cursor indicates
         where the user's input will go. Otherwise, when the dialog first
@@ -26,18 +20,17 @@
             android:id="@+id/input_time"
             android:layout_width="match_parent"
             android:layout_height="wrap_content"
-            android:layout_toEndOf="@id/cancel_icon"
-            android:layout_centerInParent="true"
+            android:textAlignment="center"
             android:textSize="@dimen/time_input_text_size"
-            android:text="12:00 AM"
-            android:background="@null"
+            android:background="@android:color/transparent"
             style="@style/TextAppearance.AppCompat.Inverse"/>
 
-    </RelativeLayout>
+    </android.support.v7.widget.Toolbar>
 
     <com.philliphsu.clock2.editalarm.NumpadTimePicker
         android:id="@+id/number_grid"
         android:layout_width="match_parent"
-        android:layout_height="@dimen/numpad_height"/>
+        android:layout_height="@dimen/numpad_height"
+        android:layout_below="@id/toolbar"/>
 
-</LinearLayout>
\ No newline at end of file
+</RelativeLayout>
\ No newline at end of file