Conversation
Ethran
changed the title
There was a problem hiding this comment.
Pull request overview
This PR refactors the Settings screen into separate composables/files, adds a setting to disable the three-finger QuickNav gesture, and extracts selection visual rendering into a dedicated composable to improve selection handling/render layering.
Changes:
- Added an
enableQuickNavsetting (UI toggle + runtime gate) for the three-finger QuickNav gesture. - Split Settings tab contents into separate component files and extracted shared Settings UI elements.
- Moved selection visual cue rendering into a standalone
SelectionVisualCuescomposable.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Adds new user-facing string for QuickNav enable setting. |
| app/src/main/res/values-pl/strings.xml | Adds Polish translation for the new QuickNav enable setting label. |
| app/src/main/java/com/ethran/notable/ui/views/Settings.kt | Wires Settings tabs to new separated composables. |
| app/src/main/java/com/ethran/notable/ui/components/SettingsUIElements.kt | Introduces shared UI rows/dividers for Settings pages. |
| app/src/main/java/com/ethran/notable/ui/components/GesturesSettings.kt | Implements gesture settings UI and adds QuickNav enable toggle. |
| app/src/main/java/com/ethran/notable/ui/components/GeneralSettings.kt | Moves general settings UI into its own file. |
| app/src/main/java/com/ethran/notable/ui/components/DebugSettings.kt | Moves debug settings UI into its own file. |
| app/src/main/java/com/ethran/notable/ui/Router.kt | Gates three-finger QuickNav gesture detection based on new setting. |
| app/src/main/java/com/ethran/notable/editor/ui/selectionVisualCues.kt | Adds a dedicated composable for selection overlay rendering. |
| app/src/main/java/com/ethran/notable/editor/ui/EditorGestureReceiver.kt | Uses SelectionVisualCues instead of inline selection drawing. |
| app/src/main/java/com/ethran/notable/data/datastore/AppSettings.kt | Adds enableQuickNav to AppSettings and reorganizes fields by section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| value = GlobalAppSettings.current.enableQuickNav, | ||
| onToggle = { isChecked -> | ||
| kv.setAppSettings( GlobalAppSettings.current.copy(enableQuickNav = isChecked)) | ||
| }) |
There was a problem hiding this comment.
There’s an extra space after setAppSettings( which is inconsistent with Kotlin formatting used elsewhere in the codebase; please run formatting / remove the stray space.
app/src/main/java/com/ethran/notable/ui/components/GeneralSettings.kt
Outdated
Show resolved
Hide resolved
| import androidx.compose.foundation.layout.fillMaxWidth | ||
| import androidx.compose.foundation.layout.padding | ||
| import androidx.compose.material.MaterialTheme | ||
| import androidx.compose.material.TabRowDefaults.Divider |
There was a problem hiding this comment.
SettingsDivider imports TabRowDefaults.Divider, which is intended for TabRow defaults and is easy to confuse with the standard Material Divider. Prefer importing/using androidx.compose.material.Divider here for clarity since this divider is not specific to a TabRow.
| import androidx.compose.material.TabRowDefaults.Divider | |
| import androidx.compose.material.Divider |
app/src/main/java/com/ethran/notable/editor/ui/selectionVisualCues.kt
Outdated
Show resolved
Hide resolved
| import androidx.compose.ui.unit.IntOffset | ||
| import androidx.compose.ui.unit.dp | ||
| import androidx.compose.ui.zIndex | ||
| import com.ethran.notable.data.datastore.AppSettings | ||
| import com.ethran.notable.data.datastore.GlobalAppSettings | ||
| import com.ethran.notable.editor.canvas.CanvasEventBus | ||
| import com.ethran.notable.editor.EditorControlTower | ||
| import com.ethran.notable.editor.canvas.CanvasEventBus |
There was a problem hiding this comment.
zIndex is imported but not used in this file after moving the selection rendering into SelectionVisualCues. Please remove the unused import (and any other now-unused imports) to avoid lint/CI failures in setups that treat unused imports as errors.
| ) { | ||
| SelectorRow( | ||
| label = title, options = listOf( | ||
| null to "None", |
There was a problem hiding this comment.
The selector options include a hardcoded "None" label. Since the rest of the gesture action labels are localized via stringResource, this should also be a string resource so it can be translated consistently.
| null to "None", | |
| null to stringResource(R.string.gesture_action_none), |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
No description provided.