From f9b09f29a15fb3cac6364574ba3f8a0a666d5ff8 Mon Sep 17 00:00:00 2001 From: Phillip Hsu Date: Tue, 6 Sep 2016 02:42:26 -0700 Subject: [PATCH] Persist label change to Timer as it is made. Fixed bug where label wasn't binding correctly afterward because of wrong parameter ordering when creating Timer instance in TimerCursor. --- .../clock2/AsyncTimersTableUpdateHandler.java | 2 + .../java/com/philliphsu/clock2/Timer.java | 10 ++++ .../philliphsu/clock2/model/TimerCursor.java | 2 +- .../clock2/model/TimersTableManager.java | 1 + .../clock2/timers/TimerController.java | 16 +++++++ .../clock2/timers/TimerViewHolder.java | 47 +++++++++++++++---- 6 files changed, 67 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/philliphsu/clock2/AsyncTimersTableUpdateHandler.java b/app/src/main/java/com/philliphsu/clock2/AsyncTimersTableUpdateHandler.java index 0852a3c..dce39dd 100644 --- a/app/src/main/java/com/philliphsu/clock2/AsyncTimersTableUpdateHandler.java +++ b/app/src/main/java/com/philliphsu/clock2/AsyncTimersTableUpdateHandler.java @@ -4,6 +4,7 @@ import android.app.AlarmManager; import android.app.PendingIntent; import android.content.Context; import android.content.Intent; +import android.util.Log; import com.philliphsu.clock2.alarms.ScrollHandler; import com.philliphsu.clock2.model.TimersTableManager; @@ -40,6 +41,7 @@ public final class AsyncTimersTableUpdateHandler extends AsyncDatabaseTableUpdat @Override protected void onPostAsyncUpdate(Long result, Timer timer) { + Log.d(TAG, "onPostAsyncUpdate, timer = " + timer); if (timer.isRunning()) { // We don't need to cancel the previous alarm, because this one // will remove and replace it. diff --git a/app/src/main/java/com/philliphsu/clock2/Timer.java b/app/src/main/java/com/philliphsu/clock2/Timer.java index 45a3bc6..ec1d1e5 100644 --- a/app/src/main/java/com/philliphsu/clock2/Timer.java +++ b/app/src/main/java/com/philliphsu/clock2/Timer.java @@ -32,6 +32,9 @@ public abstract class Timer extends ObjectWithId implements Parcelable { public abstract int hour(); public abstract int minute(); public abstract int second(); + // 9/6/2016: Just found/fixed a very subtle bug involving mixing up the parameter orders + // of group and label when `create()`ing a Timer in TimerCursor. + // TODO: We have never used this at all, so consider deleting this! public abstract String group(); public abstract String label(); @@ -54,6 +57,13 @@ public abstract class Timer extends ObjectWithId implements Parcelable { return new AutoValue_Timer(hour, minute, second, group, label); } + public void copyMutableFieldsTo(Timer target) { + target.setId(this.getId()); + target.endTime = this.endTime; + target.pauseTime = this.pauseTime; + target.duration = this.duration; + } + public long endTime() { return endTime; } diff --git a/app/src/main/java/com/philliphsu/clock2/model/TimerCursor.java b/app/src/main/java/com/philliphsu/clock2/model/TimerCursor.java index 0a579a6..9c29fec 100644 --- a/app/src/main/java/com/philliphsu/clock2/model/TimerCursor.java +++ b/app/src/main/java/com/philliphsu/clock2/model/TimerCursor.java @@ -22,7 +22,7 @@ public class TimerCursor extends BaseItemCursor { int second = getInt(getColumnIndexOrThrow(TimersTable.COLUMN_SECOND)); String label = getString(getColumnIndexOrThrow(TimersTable.COLUMN_LABEL)); // String group = getString(getColumnIndexOrThrow(COLUMN_GROUP)); - Timer t = Timer.create(hour, minute, second, label, /*group*/""); + Timer t = Timer.create(hour, minute, second, ""/*group*/, label); t.setId(getLong(getColumnIndexOrThrow(TimersTable.COLUMN_ID))); t.setEndTime(getLong(getColumnIndexOrThrow(TimersTable.COLUMN_END_TIME))); t.setPauseTime(getLong(getColumnIndexOrThrow(TimersTable.COLUMN_PAUSE_TIME))); diff --git a/app/src/main/java/com/philliphsu/clock2/model/TimersTableManager.java b/app/src/main/java/com/philliphsu/clock2/model/TimersTableManager.java index 1ab756e..55a646b 100644 --- a/app/src/main/java/com/philliphsu/clock2/model/TimersTableManager.java +++ b/app/src/main/java/com/philliphsu/clock2/model/TimersTableManager.java @@ -48,6 +48,7 @@ public class TimersTableManager extends DatabaseTableManager { cv.put(TimersTable.COLUMN_HOUR, timer.hour()); cv.put(TimersTable.COLUMN_MINUTE, timer.minute()); cv.put(TimersTable.COLUMN_SECOND, timer.second()); + Log.d(TAG, "toContentValues() label = " + timer.label()); cv.put(TimersTable.COLUMN_LABEL, timer.label()); // cv.put(TimersTable.COLUMN_GROUP, timer.group()); Log.d(TAG, "endTime = " + timer.endTime() + ", pauseTime = " + timer.pauseTime()); diff --git a/app/src/main/java/com/philliphsu/clock2/timers/TimerController.java b/app/src/main/java/com/philliphsu/clock2/timers/TimerController.java index a10262e..12a24cc 100644 --- a/app/src/main/java/com/philliphsu/clock2/timers/TimerController.java +++ b/app/src/main/java/com/philliphsu/clock2/timers/TimerController.java @@ -1,5 +1,7 @@ package com.philliphsu.clock2.timers; +import android.util.Log; + import com.philliphsu.clock2.AsyncTimersTableUpdateHandler; import com.philliphsu.clock2.Timer; @@ -7,6 +9,7 @@ import com.philliphsu.clock2.Timer; * Created by Phillip Hsu on 7/27/2016. */ public class TimerController { + private static final String TAG = "TimerController"; private final Timer mTimer; private final AsyncTimersTableUpdateHandler mUpdateHandler; @@ -40,6 +43,19 @@ public class TimerController { mTimer.addOneMinute(); update(); } + + public void updateLabel(String label) { + Log.d(TAG, "Updating Timer with label = " + label); + Timer newTimer = Timer.create( + mTimer.hour(), + mTimer.minute(), + mTimer.second(), + mTimer.group(), + label); + mTimer.copyMutableFieldsTo(newTimer); + mUpdateHandler.asyncUpdate(mTimer.getId(), newTimer); + // Prompts a reload of the list data, so the list will reflect this modified timer + } private void update() { mUpdateHandler.asyncUpdate(mTimer.getId(), mTimer); diff --git a/app/src/main/java/com/philliphsu/clock2/timers/TimerViewHolder.java b/app/src/main/java/com/philliphsu/clock2/timers/TimerViewHolder.java index b6f865e..01ef566 100644 --- a/app/src/main/java/com/philliphsu/clock2/timers/TimerViewHolder.java +++ b/app/src/main/java/com/philliphsu/clock2/timers/TimerViewHolder.java @@ -2,6 +2,7 @@ package com.philliphsu.clock2.timers; import android.animation.ObjectAnimator; import android.graphics.drawable.Drawable; +import android.support.v4.app.FragmentManager; import android.support.v4.content.ContextCompat; import android.support.v7.app.AppCompatActivity; import android.util.Log; @@ -27,12 +28,14 @@ import butterknife.OnClick; */ public class TimerViewHolder extends BaseViewHolder { private static final String TAG = "TimerViewHolder"; + private static final String TAG_ADD_LABEL_DIALOG = "add_label_dialog"; private final AsyncTimersTableUpdateHandler mAsyncTimersTableUpdateHandler; private TimerController mController; private ObjectAnimator mProgressAnimator; private final Drawable mStartIcon; private final Drawable mPauseIcon; + private final FragmentManager mFragmentManager; @Bind(R.id.label) TextView mLabel; @Bind(R.id.duration) CountdownChronometer mChronometer; @@ -48,6 +51,21 @@ public class TimerViewHolder extends BaseViewHolder { mAsyncTimersTableUpdateHandler = asyncTimersTableUpdateHandler; mStartIcon = ContextCompat.getDrawable(getContext(), R.drawable.ic_start_24dp); mPauseIcon = ContextCompat.getDrawable(getContext(), R.drawable.ic_pause_24dp); + + // TODO: This is bad! Use a Controller/Presenter instead... + // or simply pass in an instance of FragmentManager to the ctor. + AppCompatActivity act = (AppCompatActivity) getContext(); + mFragmentManager = act.getSupportFragmentManager(); + + // Are we recreating this because of a rotation? + // If so, try finding any dialog that was last shown in our backstack, + // and restore the callback. + AddLabelDialog labelDialog = (AddLabelDialog) + mFragmentManager.findFragmentByTag(TAG_ADD_LABEL_DIALOG); + if (labelDialog != null) { + Log.i(TAG, "Restoring add label callback"); + labelDialog.setOnLabelSetListener(newOnLabelSetListener()); + } } @Override @@ -56,6 +74,7 @@ public class TimerViewHolder extends BaseViewHolder { Log.d(TAG, "Binding TimerViewHolder"); // TOneverDO: create before super mController = new TimerController(timer, mAsyncTimersTableUpdateHandler); + Log.d(TAG, "timer.label() = " + timer.label()); bindLabel(timer.label()); bindChronometer(timer); bindButtonControls(timer); @@ -79,16 +98,8 @@ public class TimerViewHolder extends BaseViewHolder { @OnClick(R.id.label) void openLabelEditor() { - AddLabelDialog dialog = AddLabelDialog.newInstance(new AddLabelDialog.OnLabelSetListener() { - @Override - public void onLabelSet(String label) { - mLabel.setText(label); - // TODO: persist change. Use TimerController and its update() - } - }, mLabel.getText()); - // TODO: This is bad! Use a Controller instead! - AppCompatActivity act = (AppCompatActivity) getContext(); - dialog.show(act.getSupportFragmentManager(), "TAG"); + AddLabelDialog dialog = AddLabelDialog.newInstance(newOnLabelSetListener(), mLabel.getText()); + dialog.show(mFragmentManager, TAG_ADD_LABEL_DIALOG); } private void bindLabel(String label) { @@ -163,4 +174,20 @@ public class TimerViewHolder extends BaseViewHolder { } mSeekBar.getThumb().mutate().setAlpha(timeRemaining <= 0 ? 0 : 255); } + + private AddLabelDialog.OnLabelSetListener newOnLabelSetListener() { + // Create a new listener per request. This is primarily used for + // setting the dialog callback again after a rotation. + // + // If we saved a reference to a listener, it would be tied to + // its ViewHolder instance. ViewHolders are reused, so we + // could accidentally leak this reference to other Timer items + // in the list. + return new AddLabelDialog.OnLabelSetListener() { + @Override + public void onLabelSet(String label) { + mController.updateLabel(label); + } + }; + } }