Address review comments

uhabits-android/src/androidTest/java/org/isoron/uhabits/activities/habits/list/views/NumberButtonViewTest.kt

@@ -116,15 +116,7 @@ class NumberButtonViewTest : BaseViewTest() {
     }
 
     @Test
-    fun testClick_shortToggleDisabled() {
+    fun testClick() {
-        prefs.isShortToggleEnabled = false
-        view.performClick()
-        assertTrue(edited)
-    }
-
-    @Test
-    fun testClick_shortToggleEnabled() {
-        prefs.isShortToggleEnabled = true
         view.performClick()
         assertTrue(edited)
     }

uhabits-android/src/main/java/org/isoron/uhabits/activities/common/dialogs/HistoryEditorDialog.kt

@@ -66,7 +66,7 @@ class HistoryEditorDialog : AppCompatDialogFragment(), CommandRunner.Listener {
             hasNotes = emptyList(),
             theme = themeSwitcher.currentTheme,
             today = DateUtils.getTodayWithOffset().toLocalDate(),
-            onDateClickedListener = onDateClickedListener ?: OnDateClickedListener { _, _ -> },
+            onDateClickedListener = onDateClickedListener ?: object : OnDateClickedListener {},
             padding = 10.0,
         )
         dataView = AndroidDataView(context!!, null)

uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/CheckmarkButtonView.kt

@@ -38,6 +38,7 @@ import org.isoron.uhabits.core.models.Entry.Companion.YES_MANUAL
 import org.isoron.uhabits.core.preferences.Preferences
 import org.isoron.uhabits.inject.ActivityContext
 import org.isoron.uhabits.utils.dim
+import org.isoron.uhabits.utils.drawNotesIndicator
 import org.isoron.uhabits.utils.getFontAwesome
 import org.isoron.uhabits.utils.sres
 import org.isoron.uhabits.utils.toMeasureSpec
@@ -104,7 +105,8 @@ class CheckmarkButtonView(
     }
 
     override fun onLongClick(v: View): Boolean {
-        performToggle()
+        if (preferences.isShortToggleEnabled) onEdit()
+        else performToggle()
         return true
     }
 
@@ -134,8 +136,6 @@ class CheckmarkButtonView(
             textAlign = Paint.Align.CENTER
         }
 
-        private val pNotesIndicator: Paint = Paint()
-
         fun draw(canvas: Canvas) {
             paint.color = when (value) {
                 YES_MANUAL, YES_AUTO, SKIP -> color
@@ -145,7 +145,6 @@ class CheckmarkButtonView(
                 }
                 else -> lowContrastColor
             }
-            pNotesIndicator.color = color
             val id = when (value) {
                 SKIP -> R.string.fa_skipped
                 NO -> R.string.fa_times
@@ -181,10 +180,7 @@ class CheckmarkButtonView(
                 canvas.drawText(label, rect.centerX(), rect.centerY(), paint)
             }
 
-            if (hasNotes) {
+            drawNotesIndicator(canvas, color, em, hasNotes)
-                val cy = 0.8f * em
-                canvas.drawCircle(width.toFloat() - cy, cy, 8f, pNotesIndicator)
-            }
         }
     }
 }

uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/CheckmarkPanelView.kt

@@ -54,7 +54,7 @@ class CheckmarkPanelView(
             setupButtons()
         }
 
-    var notes = BooleanArray(0)
+    var notesIndicators = BooleanArray(0)
         set(values) {
             field = values
             setupButtons()
@@ -85,7 +85,7 @@ class CheckmarkPanelView(
                 else -> UNKNOWN
             }
             button.hasNotes = when {
-                index + dataOffset < notes.size -> notes[index + dataOffset]
+                index + dataOffset < notesIndicators.size -> notesIndicators[index + dataOffset]
                 else -> false
             }
             button.color = color

uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardListView.kt

@@ -99,7 +99,7 @@ class HabitCardListView(
         cardView.score = score
         cardView.unit = habit.unit
         cardView.threshold = habit.targetValue / habit.frequency.denominator
-        cardView.notes = notesIndicators
+        cardView.notesIndicators = notesIndicators
 
         val detector = GestureDetector(context, CardViewGestureDetector(holder))
         cardView.setOnTouchListener { _, ev ->

uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardView.kt

@@ -115,11 +115,11 @@ class HabitCardView(
             numberPanel.threshold = value
         }
 
-    var notes
+    var notesIndicators
-        get() = checkmarkPanel.notes
+        get() = checkmarkPanel.notesIndicators
         set(values) {
-            checkmarkPanel.notes = values
+            checkmarkPanel.notesIndicators = values
-            numberPanel.notes = values
+            numberPanel.notesIndicators = values
         }
 
     var checkmarkPanel: CheckmarkPanelView

uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/NumberButtonView.kt

@@ -34,6 +34,7 @@ import org.isoron.uhabits.core.preferences.Preferences
 import org.isoron.uhabits.inject.ActivityContext
 import org.isoron.uhabits.utils.InterfaceUtils.getDimension
 import org.isoron.uhabits.utils.dim
+import org.isoron.uhabits.utils.drawNotesIndicator
 import org.isoron.uhabits.utils.getFontAwesome
 import org.isoron.uhabits.utils.sres
 import java.lang.Double.max
@@ -156,8 +157,6 @@ class NumberButtonView(
             textAlign = Paint.Align.CENTER
         }
 
-        private val pNotesIndicator: Paint = Paint()
-
         init {
             em = pNumber.measureText("m")
             lowContrast = sres.getColor(R.attr.contrast40)
@@ -205,7 +204,6 @@ class NumberButtonView(
             pNumber.color = activeColor
             pNumber.typeface = typeface
             pUnit.color = activeColor
-            pNotesIndicator.color = color
 
             if (units.isBlank()) {
                 rect.set(0f, 0f, width.toFloat(), height.toFloat())
@@ -218,10 +216,7 @@ class NumberButtonView(
                 canvas.drawText(units, rect.centerX(), rect.centerY(), pUnit)
             }
 
-            if (hasNotes) {
+            drawNotesIndicator(canvas, color, em, hasNotes)
-                val cy = 0.8f * em
-                canvas.drawCircle(width.toFloat() - cy, cy, 8f, pNotesIndicator)
-            }
         }
     }
 }

uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/NumberPanelView.kt

@@ -72,7 +72,7 @@ class NumberPanelView(
             setupButtons()
         }
 
-    var notes = BooleanArray(0)
+    var notesIndicators = BooleanArray(0)
         set(values) {
             field = values
             setupButtons()
@@ -97,7 +97,7 @@ class NumberPanelView(
                 else -> 0.0
             }
             button.hasNotes = when {
-                index + dataOffset < notes.size -> notes[index + dataOffset]
+                index + dataOffset < notesIndicators.size -> notesIndicators[index + dataOffset]
                 else -> false
             }
             button.color = color

uhabits-android/src/main/java/org/isoron/uhabits/utils/ViewExtensions.kt

@@ -22,7 +22,9 @@ package org.isoron.uhabits.utils
 import android.app.Activity
 import android.content.ActivityNotFoundException
 import android.content.Intent
+import android.graphics.Canvas
 import android.graphics.Color
+import android.graphics.Paint
 import android.graphics.drawable.ColorDrawable
 import android.os.Handler
 import android.view.LayoutInflater
@@ -199,5 +201,15 @@ fun View.dim(id: Int) = InterfaceUtils.getDimension(context, id)
 fun View.sp(value: Float) = InterfaceUtils.spToPixels(context, value)
 fun View.dp(value: Float) = InterfaceUtils.dpToPixels(context, value)
 fun View.str(id: Int) = resources.getString(id)
+
+fun View.drawNotesIndicator(canvas: Canvas, color: Int, size: Float, hasNotes: Boolean) {
+    val pNotesIndicator = Paint()
+    pNotesIndicator.color = color
+    if (hasNotes) {
+        val cy = 0.8f * size
+        canvas.drawCircle(width.toFloat() - cy, cy, 8f, pNotesIndicator)
+    }
+}
+
 val View.sres: StyledResources
     get() = StyledResources(context)

uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/EntryList.kt

@@ -100,7 +100,7 @@ open class EntryList {
             val intervals = buildIntervals(frequency, original)
             snapIntervalsTogether(intervals)
             val computed = buildEntriesFromInterval(original, intervals)
-            computed.filter { it.value != UNKNOWN }.forEach { add(it) }
+            computed.filter { it.value != UNKNOWN || it.notes.isNotEmpty() }.forEach { add(it) }
         }
     }
 

uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/list/HabitCardListCache.kt

@@ -298,14 +298,14 @@ class HabitCardListCache @Inject constructor(
                 if (targetId != null && targetId != habit.id) continue
                 newData.scores[habit.id] = habit.scores[today].value
                 val list: MutableList<Int> = ArrayList()
-                val notesList: MutableList<Boolean> = ArrayList()
+                val notesIndicators: MutableList<Boolean> = ArrayList()
                 for ((_, value, note) in habit.computedEntries.getByInterval(dateFrom, today)) {
                     list.add(value)
-                    if (note.isNotEmpty()) notesList.add(true) else notesList.add(false)
+                    notesIndicators.add(note.isNotEmpty())
                 }
                 val entries = list.toTypedArray()
                 newData.checkmarks[habit.id] = ArrayUtils.toPrimitive(entries)
-                newData.notesIndicators[habit.id] = notesList.toBooleanArray()
+                newData.notesIndicators[habit.id] = notesIndicators.toBooleanArray()
                 runner!!.publishProgress(this, position)
             }
         }

uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/list/ListHabitsBehavior.kt

@@ -48,24 +48,22 @@ open class ListHabitsBehavior @Inject constructor(
     }
 
     fun onEdit(habit: Habit, timestamp: Timestamp?) {
-        val entries = habit.computedEntries.get(timestamp!!)
+        val entry = habit.computedEntries.get(timestamp!!)
-        val notes = entries.notes
         if (habit.type == HabitType.NUMERICAL) {
-            val oldValue = entries.value.toDouble()
+            val oldValue = entry.value.toDouble()
             screen.showNumberPicker(
                 oldValue / 1000,
                 habit.unit,
-                notes
+                entry.notes
             ) { newValue: Double, newNotes: String, ->
                 val value = (newValue * 1000).roundToInt()
                 commandRunner.run(CreateRepetitionCommand(habitList, habit, timestamp, value, newNotes))
             }
         } else {
-            val value = entries.value
             screen.showCheckmarkDialog(
-                notes
+                entry.notes
             ) { newNotes ->
-                commandRunner.run(CreateRepetitionCommand(habitList, habit, timestamp, value, newNotes))
+                commandRunner.run(CreateRepetitionCommand(habitList, habit, timestamp, entry.value, newNotes))
             }
         }
     }

uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/HistoryCard.kt

@@ -59,14 +59,59 @@ class HistoryCardPresenter(
     val screen: Screen,
 ) : OnDateClickedListener {
 
-    override fun onDateClicked(date: LocalDate, isLongClick: Boolean) {
+    override fun onDateShortPress(date: LocalDate) {
         val timestamp = Timestamp.fromLocalDate(date)
         screen.showFeedback()
-        val entries = habit.computedEntries
-        val oldValue = entries.get(timestamp).value
-        val notes = entries.get(timestamp).notes
         if (habit.isNumerical) {
-            screen.showNumberPicker(oldValue / 1000.0, habit.unit, notes) { newValue: Double, newNotes: String ->
+            showNumberPicker(timestamp)
+        } else {
+            val entry = habit.computedEntries.get(timestamp)
+            val nextValue = Entry.nextToggleValue(
+                value = entry.value,
+                isSkipEnabled = preferences.isSkipEnabled,
+                areQuestionMarksEnabled = preferences.areQuestionMarksEnabled
+            )
+            commandRunner.run(
+                CreateRepetitionCommand(
+                    habitList,
+                    habit,
+                    timestamp,
+                    nextValue,
+                    entry.notes,
+                ),
+            )
+        }
+    }
+
+    override fun onDateLongPress(date: LocalDate) {
+        val timestamp = Timestamp.fromLocalDate(date)
+        screen.showFeedback()
+        if (habit.isNumerical) {
+            showNumberPicker(timestamp)
+        } else {
+            val entry = habit.computedEntries.get(timestamp)
+            screen.showCheckmarkDialog(entry.notes) { newNotes ->
+                commandRunner.run(
+                    CreateRepetitionCommand(
+                        habitList,
+                        habit,
+                        timestamp,
+                        entry.value,
+                        newNotes,
+                    ),
+                )
+            }
+        }
+    }
+
+    private fun showNumberPicker(timestamp: Timestamp) {
+        val entry = habit.computedEntries.get(timestamp)
+        val oldValue = entry.value
+        screen.showNumberPicker(
+            oldValue / 1000.0,
+            habit.unit,
+            entry.notes
+        ) { newValue: Double, newNotes: String ->
             val thousands = (newValue * 1000).roundToInt()
             commandRunner.run(
                 CreateRepetitionCommand(
@@ -78,36 +123,6 @@ class HistoryCardPresenter(
                 ),
             )
         }
-        } else {
-            if (!isLongClick) {
-                val nextValue = Entry.nextToggleValue(
-                    value = oldValue,
-                    isSkipEnabled = preferences.isSkipEnabled,
-                    areQuestionMarksEnabled = preferences.areQuestionMarksEnabled
-                )
-                commandRunner.run(
-                    CreateRepetitionCommand(
-                        habitList,
-                        habit,
-                        timestamp,
-                        nextValue,
-                        notes,
-                    ),
-                )
-                return
-            }
-            screen.showCheckmarkDialog(notes) { newNotes ->
-                commandRunner.run(
-                    CreateRepetitionCommand(
-                        habitList,
-                        habit,
-                        timestamp,
-                        oldValue,
-                        newNotes,
-                    ),
-                )
-            }
-        }
     }
 
     fun onClickEditButton() {
@@ -167,7 +182,7 @@ class HistoryCardPresenter(
                 today = today.toLocalDate(),
                 theme = theme,
                 series = series,
-                defaultSquare = defaultSquare
+                defaultSquare = defaultSquare,
                 hasNotes = hasNotes,
             )
         }

uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/views/HistoryChart.kt

@@ -32,8 +32,9 @@ import kotlin.math.max
 import kotlin.math.min
 import kotlin.math.round
 
-fun interface OnDateClickedListener {
+interface OnDateClickedListener {
-    fun onDateClicked(date: LocalDate, isLongClick: Boolean)
+    fun onDateShortPress(date: LocalDate) {}
+    fun onDateLongPress(date: LocalDate) {}
 }
 
 class HistoryChart(
@@ -45,7 +46,7 @@ class HistoryChart(
     var hasNotes: List<Boolean>,
     var theme: Theme,
     var today: LocalDate,
-    var onDateClickedListener: OnDateClickedListener = OnDateClickedListener { _, _ -> },
+    var onDateClickedListener: OnDateClickedListener = object : OnDateClickedListener {},
     var padding: Double = 0.0,
 ) : DataView {
 
@@ -88,7 +89,11 @@ class HistoryChart(
         if (row == 0 || col == nColumns) return
         val clickedDate = topLeftDate.plus(offset)
         if (clickedDate.isNewerThan(today)) return
-        onDateClickedListener.onDateClicked(clickedDate, isLongClick)
+        if (isLongClick) {
+            onDateClickedListener.onDateLongPress(clickedDate)
+        } else {
+            onDateClickedListener.onDateShortPress(clickedDate)
+        }
     }
 
     override fun draw(canvas: Canvas) {

uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/widgets/WidgetBehavior.kt

@@ -38,43 +38,38 @@ class WidgetBehavior @Inject constructor(
     fun onAddRepetition(habit: Habit, timestamp: Timestamp?) {
         notificationTray.cancel(habit)
         val entry = habit.originalEntries.get(timestamp!!)
-        val notes = entry.notes
+        setValue(habit, timestamp, Entry.YES_MANUAL, entry.notes)
-        setValue(habit, timestamp, Entry.YES_MANUAL, notes)
     }
 
     fun onRemoveRepetition(habit: Habit, timestamp: Timestamp?) {
         notificationTray.cancel(habit)
         val entry = habit.originalEntries.get(timestamp!!)
-        val notes = entry.notes
+        setValue(habit, timestamp, Entry.NO, entry.notes)
-        setValue(habit, timestamp, Entry.NO, notes)
     }
 
     fun onToggleRepetition(habit: Habit, timestamp: Timestamp) {
         val entry = habit.originalEntries.get(timestamp)
         val currentValue = entry.value
-        val notes = entry.notes
         val newValue = nextToggleValue(
             value = currentValue,
             isSkipEnabled = preferences.isSkipEnabled,
             areQuestionMarksEnabled = preferences.areQuestionMarksEnabled
         )
-        setValue(habit, timestamp, newValue, notes)
+        setValue(habit, timestamp, newValue, entry.notes)
         notificationTray.cancel(habit)
     }
 
     fun onIncrement(habit: Habit, timestamp: Timestamp, amount: Int) {
         val entry = habit.computedEntries.get(timestamp)
         val currentValue = entry.value
-        val notes = entry.notes
+        setValue(habit, timestamp, currentValue + amount, entry.notes)
-        setValue(habit, timestamp, currentValue + amount, notes)
         notificationTray.cancel(habit)
     }
 
     fun onDecrement(habit: Habit, timestamp: Timestamp, amount: Int) {
         val entry = habit.computedEntries.get(timestamp)
         val currentValue = entry.value
-        val notes = entry.notes
+        setValue(habit, timestamp, currentValue - amount, entry.notes)
-        setValue(habit, timestamp, currentValue - amount, notes)
         notificationTray.cancel(habit)
     }
 

uhabits-core/src/jvmTest/java/org/isoron/uhabits/core/ui/views/HistoryChartTest.kt

@@ -90,20 +90,20 @@ class HistoryChartTest {
 
         // Click top left date
         view.onClick(20.0, 46.0)
-        verify(dateClickedListener).onDateClicked(LocalDate(2014, 10, 26), false)
+        verify(dateClickedListener).onDateShortPress(LocalDate(2014, 10, 26))
         reset(dateClickedListener)
         view.onClick(2.0, 28.0)
-        verify(dateClickedListener).onDateClicked(LocalDate(2014, 10, 26), false)
+        verify(dateClickedListener).onDateShortPress(LocalDate(2014, 10, 26))
         reset(dateClickedListener)
 
         // Click date in the middle
         view.onClick(163.0, 113.0)
-        verify(dateClickedListener).onDateClicked(LocalDate(2014, 12, 10), false)
+        verify(dateClickedListener).onDateShortPress(LocalDate(2014, 12, 10))
         reset(dateClickedListener)
 
         // Click today
         view.onClick(336.0, 37.0)
-        verify(dateClickedListener).onDateClicked(LocalDate(2015, 1, 25), false)
+        verify(dateClickedListener).onDateShortPress(LocalDate(2015, 1, 25))
         reset(dateClickedListener)
 
         // Click header
@@ -115,6 +115,37 @@ class HistoryChartTest {
         verifyNoMoreInteractions(dateClickedListener)
     }
 
+    @Test
+    fun testLongClick() = runBlocking {
+        assertRenders(400, 200, "$base/base.png", view)
+
+        // Click top left date
+        view.onLongClick(20.0, 46.0)
+        verify(dateClickedListener).onDateLongPress(LocalDate(2014, 10, 26))
+        reset(dateClickedListener)
+        view.onLongClick(2.0, 28.0)
+        verify(dateClickedListener).onDateLongPress(LocalDate(2014, 10, 26))
+        reset(dateClickedListener)
+
+        // Click date in the middle
+        view.onLongClick(163.0, 113.0)
+        verify(dateClickedListener).onDateLongPress(LocalDate(2014, 12, 10))
+        reset(dateClickedListener)
+
+        // Click today
+        view.onLongClick(336.0, 37.0)
+        verify(dateClickedListener).onDateLongPress(LocalDate(2015, 1, 25))
+        reset(dateClickedListener)
+
+        // Click header
+        view.onLongClick(160.0, 15.0)
+        verifyNoMoreInteractions(dateClickedListener)
+
+        // Click right axis
+        view.onLongClick(360.0, 60.0)
+        verifyNoMoreInteractions(dateClickedListener)
+    }
+
     @Test
     fun testDrawWeekDay() = runBlocking {
         view.firstWeekday = DayOfWeek.MONDAY