Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
package com.philkes.notallyx.presentation.activity.main.fragment.settings

import android.annotation.SuppressLint
import android.content.Context
import android.hardware.biometrics.BiometricManager
import android.net.Uri
import android.os.Build
import android.os.Handler
import android.os.Looper
import android.text.method.PasswordTransformationMethod
import android.view.LayoutInflater
import android.view.MotionEvent
import android.view.View
import android.view.inputmethod.EditorInfo
import androidx.core.view.isVisible
import androidx.core.widget.doAfterTextChanged
import androidx.documentfile.provider.DocumentFile
import com.google.android.material.color.DynamicColors
import com.google.android.material.dialog.MaterialAlertDialogBuilder
Expand All @@ -23,11 +29,14 @@ import com.philkes.notallyx.databinding.DialogSelectionBoxBinding
import com.philkes.notallyx.databinding.DialogTextInputBinding
import com.philkes.notallyx.databinding.PreferenceBinding
import com.philkes.notallyx.databinding.PreferenceSeekbarBinding
import com.philkes.notallyx.databinding.PreferenceStepperBinding
import com.philkes.notallyx.presentation.checkedTag
import com.philkes.notallyx.presentation.hideKeyboard
import com.philkes.notallyx.presentation.select
import com.philkes.notallyx.presentation.setCancelButton
import com.philkes.notallyx.presentation.setTextSizeSp
import com.philkes.notallyx.presentation.showAndFocus
import com.philkes.notallyx.presentation.showKeyboard
import com.philkes.notallyx.presentation.showToast
import com.philkes.notallyx.presentation.view.misc.MenuDialog
import com.philkes.notallyx.presentation.viewmodel.BaseNoteModel
Expand Down Expand Up @@ -501,43 +510,157 @@ fun PreferenceSeekbarBinding.setupTextSizePreference(
}
}

fun PreferenceSeekbarBinding.setupAutoSaveIdleTime(
preference: IntPreference,
@SuppressLint("ClickableViewAccessibility")
fun PreferenceStepperBinding.setup(
value: Int,
titleResId: Int,
min: Int,
max: Int,
context: Context,
value: Int = preference.value,
enabled: Boolean = true,
labelFormatter: ((Int) -> String)? = null,
onChange: (newValue: Int) -> Unit,
) {
Slider.apply {
setLabelFormatter { sliderValue ->
if (sliderValue == -1f) {
context.getString(R.string.disabled)
} else "${sliderValue.toInt()}s"
val initialValue = value.coerceIn(min, max)
Title.setText(titleResId)
ValueInput.setText(labelFormatter?.invoke(initialValue) ?: initialValue.toString())
ValueInput.isEnabled = enabled

fun updateButtons(value: Int) {
MinusButton.isEnabled = enabled && value > min
PlusButton.isEnabled = enabled && value < max
}

updateButtons(initialValue)

val handler = Handler(Looper.getMainLooper())
fun updateValue(increment: Int, commit: Boolean): Boolean {
val newValue = (ValueInput.tag as? Int ?: initialValue) + increment
val valid = newValue in min..max
if (valid) {
ValueInput.tag = newValue
ValueInput.setText(labelFormatter?.invoke(newValue) ?: newValue.toString())
Comment thread
coderabbitai[bot] marked this conversation as resolved.
updateButtons(newValue)
if (commit) {
onChange(newValue)
}
}
ValueInput.clearFocus()
return valid
Comment thread
Crustack marked this conversation as resolved.
}

fun stepperRunnable(isIncrement: Boolean) =
object : Runnable {
override fun run() {
if (updateValue(if (isIncrement) 1 else -1, false)) {
handler.postDelayed(this, 100)
}
}
}
addOnChangeListener { _, value, _ ->
if (value == -1f) {
setAlpha(0.6f) // Reduce opacity to make it look disabled
var runnable: Runnable? = null
val startAutoIncrement = { isIncrement: Boolean ->
runnable?.let { handler.removeCallbacks(it) }
runnable =
if (isIncrement) stepperRunnable(isIncrement = true)
else stepperRunnable(isIncrement = false)
handler.postDelayed(runnable!!, 100)
}

MinusButton.apply {
setOnClickListener { updateValue(-1, true) }
setOnLongClickListener {
startAutoIncrement(false)
true
}
setOnTouchListener { _, event ->
if (
runnable != null &&
(event.action == MotionEvent.ACTION_UP ||
event.action == MotionEvent.ACTION_CANCEL)
) {
handler.removeCallbacks(runnable!!)
onChange(ValueInput.tag as? Int ?: value)
}
false
}
}

PlusButton.apply {
setOnClickListener { updateValue(1, true) }
setOnLongClickListener {
startAutoIncrement(true)
true
}
setOnTouchListener { _, event ->
if (
runnable != null &&
(event.action == MotionEvent.ACTION_UP ||
event.action == MotionEvent.ACTION_CANCEL)
) {
handler.removeCallbacks(runnable!!)
onChange(ValueInput.tag as? Int ?: value)
}
false
}
}
Comment on lines +560 to +605
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't commit twice on a normal tap.

The touch listener saves on every button release, and the click listener saves again immediately after. For settings with side effects, that means duplicated work on every tap; AutoEmptyBin now cancels/schedules WorkManager twice.

Suggested fix
-    fun stepperRunnable(isIncrement: Boolean) =
+    var didAutoRepeat = false
+    fun stepperRunnable(isIncrement: Boolean) =
         object : Runnable {
             override fun run() {
                 if (updateValue(if (isIncrement) 1 else -1, false)) {
+                    didAutoRepeat = true
                     handler.postDelayed(this, 100)
                 }
             }
         }
     var runnable: Runnable? = null
     val startAutoIncrement = { isIncrement: Boolean ->
+        didAutoRepeat = false
         runnable?.let { handler.removeCallbacks(it) }
         runnable =
             if (isIncrement) stepperRunnable(isIncrement = true)
             else stepperRunnable(isIncrement = false)
         handler.postDelayed(runnable!!, 100)
@@
     MinusButton.setOnTouchListener { _, event ->
         if (event.action == MotionEvent.ACTION_UP || event.action == MotionEvent.ACTION_CANCEL) {
             runnable?.let { handler.removeCallbacks(it) }
-            onChange(ValueInput.tag as? Int ?: value)
+            if (didAutoRepeat) {
+                didAutoRepeat = false
+                onChange(ValueInput.tag as? Int ?: value)
+            }
         }
         false
     }
@@
     PlusButton.setOnTouchListener { _, event ->
         if (event.action == MotionEvent.ACTION_UP || event.action == MotionEvent.ACTION_CANCEL) {
             runnable?.let { handler.removeCallbacks(it) }
-            onChange(ValueInput.tag as? Int ?: value)
+            if (didAutoRepeat) {
+                didAutoRepeat = false
+                onChange(ValueInput.tag as? Int ?: value)
+            }
         }
         false
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/settings/PreferenceBindingExtensions.kt`
around lines 559 - 596, The touch listeners call onChange on ACTION_UP which
duplicates the click listener's save; to fix, add a small boolean flag (e.g.,
handledClick or isAutoIncrementing) and update it from the click/long-click
handlers so the touch listener only calls onChange if the click wasn't already
handled. Specifically, in the PlusButton/MinusButton click lambdas (where
updateValue(…) is called) set handledClick = true, in the touch listener check
if handledClick is false before calling onChange(ValueInput.tag as? Int ?:
value), and always reset handledClick = false at the end of the touch handling
(or when starting/stopping auto-increment in startAutoIncrement/runnable) so
subsequent interactions behave normally.


ValueInput.apply {
tag = value
doAfterTextChanged { text ->
if (ValueInput.hasFocus()) {
val newValue = text.toString().toIntOrNull()
if (newValue != null) {
val clampedValue = newValue.coerceIn(min, max)
if (clampedValue != ValueInput.tag) {
ValueInput.tag = clampedValue
updateButtons(clampedValue)
}
}
}
}
setOnFocusChangeListener { _, hasFocus ->
if (hasFocus) {
val currentValue = ValueInput.tag as? Int ?: initialValue
val text = currentValue.toString()
ValueInput.setText(text)
ValueInput.setSelection(text.length)
context.showKeyboard(ValueInput)
} else {
setAlpha(1f) // Restore normal appearance
val currentValue = ValueInput.tag as? Int ?: initialValue
ValueInput.setText(labelFormatter?.invoke(currentValue) ?: currentValue.toString())
updateButtons(currentValue)
onChange(currentValue)
}
}
setOnEditorActionListener { v, actionId, _ ->
if (actionId == EditorInfo.IME_ACTION_DONE || actionId == EditorInfo.IME_ACTION_NEXT) {
context.hideKeyboard(ValueInput)
v.clearFocus() // This triggers the OnFocusChangeListener logic
true
} else {
false
}
}
}
setup(preference, context, value, onChange = onChange)
}

fun PreferenceSeekbarBinding.setupAutoEmptyBin(
fun PreferenceStepperBinding.setup(
preference: IntPreference,
context: Context,
value: Int = preference.value,
labelFormatter: ((Int) -> String)? = null,
onChange: (newValue: Int) -> Unit,
) {
Slider.apply {
setLabelFormatter { sliderValue ->
if (sliderValue == 0f) {
context.getString(R.string.disabled)
} else "${sliderValue.toInt()} ${context.getString(R.string.days)}"
}
setup(
value,
preference.titleResId!!,
preference.min,
preference.max,
context,
labelFormatter = labelFormatter,
) { newValue ->
onChange(newValue)
}
setup(preference, context, value, R.string.auto_remove_deleted_notes_hint, onChange)
}

fun PreferenceBinding.setupStartView(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import android.os.Bundle
import android.provider.DocumentsContract
import android.provider.Settings
import android.text.method.PasswordTransformationMethod
import android.util.Log
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
Expand Down Expand Up @@ -376,10 +377,15 @@ class SettingsFragment : Fragment() {
}

autoRemoveDeletedNotesAfterDays.observe(viewLifecycleOwner) { value ->
binding.AutoEmptyBin.setupAutoEmptyBin(
binding.AutoEmptyBin.setup(
autoRemoveDeletedNotesAfterDays,
requireContext(),
labelFormatter = { v ->
if (v == 0) requireContext().getString(R.string.off)
else "$v ${requireContext().getString(R.string.days)}"
},
Comment thread
Crustack marked this conversation as resolved.
) { newValue ->
Log.d("Stepper", "save auto remove")
model.savePreference(autoRemoveDeletedNotesAfterDays, newValue)
val workManager = WorkManager.getInstance(requireContext())
if (newValue > 0) {
Expand Down Expand Up @@ -768,8 +774,13 @@ class SettingsFragment : Fragment() {
}
}
}
AutoSaveAfterIdle.setupAutoSaveIdleTime(autoSaveAfterIdleTime, requireContext()) {
newValue ->
AutoSaveAfterIdle.setup(
autoSaveAfterIdleTime,
requireContext(),
labelFormatter = { v ->
if (v == -1) requireContext().getString(R.string.off) else "${v}s"
},
) { newValue ->
model.savePreference(autoSaveAfterIdleTime, newValue)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class NotallyXPreferences private constructor(private val context: Context) {
preferences,
5,
0,
20,
200,
R.string.max_labels_to_display,
)

Expand All @@ -163,7 +163,7 @@ class NotallyXPreferences private constructor(private val context: Context) {
preferences,
0,
0,
365,
3650,
R.string.auto_remove_deleted_notes,
)

Expand All @@ -182,7 +182,7 @@ class NotallyXPreferences private constructor(private val context: Context) {
preferences,
5,
-1,
20,
60 * 60 * 5,
R.string.auto_save_after_idle_time,
)

Expand Down
10 changes: 10 additions & 0 deletions app/src/main/res/drawable/remove.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="960"
android:viewportHeight="960"
android:tint="?attr/colorControlNormal">
<path
android:fillColor="@android:color/white"
android:pathData="M200,520L200,440L760,440L760,520L200,520Z"/>
</vector>
10 changes: 5 additions & 5 deletions app/src/main/res/layout/fragment_settings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@

<include
android:id="@+id/AutoSaveAfterIdle"
layout="@layout/preference_seekbar" />
layout="@layout/preference_stepper" />

<include
android:id="@+id/AutoEmptyBin"
layout="@layout/preference_seekbar" />
layout="@layout/preference_stepper" />

<View
android:layout_width="match_parent"
Expand All @@ -102,7 +102,7 @@

<include
android:id="@+id/MaxLabels"
layout="@layout/preference_seekbar" />
layout="@layout/preference_stepper" />

<include
android:id="@+id/LabelsHiddenInOverview"
Expand Down Expand Up @@ -172,11 +172,11 @@

<include
android:id="@+id/PeriodicBackupsPeriodInDays"
layout="@layout/preference_seekbar" />
layout="@layout/preference_stepper" />

<include
android:id="@+id/PeriodicBackupsMax"
layout="@layout/preference_seekbar" />
layout="@layout/preference_stepper" />

<View
android:layout_width="match_parent"
Expand Down
Loading