Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #55 +/- ##
==========================================
- Coverage 43.17% 41.47% -1.71%
==========================================
Files 38 39 +1
Lines 945 1020 +75
Branches 262 283 +21
==========================================
+ Hits 408 423 +15
- Misses 437 488 +51
- Partials 100 109 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a smart delete feature for a cube notation keyboard app, allowing users to delete entire cube notations (R, U, F, x, M, etc.) instead of single characters when using the backspace button. The feature can be toggled on/off in the settings.
- Adds smart delete functionality that deletes text up to and including the previous main notation character
- Creates a new settings toggle to control smart delete behavior
- Updates the UI to show different backspace icons based on smart delete state
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
InputConnection.kt |
Implements core smart delete logic and adds logging |
SmartDeleteSwitchItem.kt |
Creates new UI component for smart delete toggle |
SettingsViewModel.kt |
Adds smart delete state management and reorders data class fields |
SettingsScreen.kt |
Integrates smart delete toggle into settings UI |
Notation.kt |
Adds companion object with helper method for notation characters |
ControlKeyButtonRow.kt |
Updates delete button to show different icons based on smart delete state |
KeyboardViewModel.kt |
Adds smart delete to keyboard state and reorders fields |
HushKeyboardView.kt |
Integrates smart delete functionality and adds extensive logging |
SettingsRepository.kt |
Adds repository method for updating smart delete setting |
AppSettings.kt |
Adds smart delete field to data model and reorders fields |
ic_backspace_*.xml |
Adds new drawable resources for backspace icons |
SettingsScreenUiTest.kt |
Updates test to include smart delete callback |
| @Composable | ||
| fun SmartDeleteSwitchItemPreview() { | ||
| HushKeyboardTheme { | ||
| AddSpaceBetweenNotationSwitchItem( |
There was a problem hiding this comment.
The preview function is calling AddSpaceBetweenNotationSwitchItem instead of SmartDeleteSwitchItem. This will cause the preview to display the wrong component.
| AddSpaceBetweenNotationSwitchItem( | |
| SmartDeleteSwitchItem( |
| } | ||
| } else { | ||
| { | ||
| Log.d(TAG, "Smart delete button tapped") |
There was a problem hiding this comment.
The log message says "Smart delete button tapped" when this is actually the regular delete functionality (when smart delete is disabled). This should be "Delete button tapped" to match the actual functionality.
| Log.d(TAG, "Smart delete button tapped") | |
| Log.d(TAG, "Delete button tapped") |
| val notationCharList = Notation.getCharList() + listOf('\n') | ||
|
|
There was a problem hiding this comment.
[nitpick] The inclusion of newline character ('\n') in the notation list seems inconsistent with the purpose of cube notations. Consider creating a separate constant or adding a comment explaining why newline is treated as a notation character for smart delete purposes.
| val notationCharList = Notation.getCharList() + listOf('\n') | |
| // Newline character is included to handle smart delete functionality, allowing deletion of line breaks. | |
| private const val NEWLINE_CHAR = '\n' | |
| val notationCharList = Notation.getCharList() + listOf(NEWLINE_CHAR) |
|
|
||
| beginBatchEdit() | ||
| try { | ||
| val scanWindow = 50 |
There was a problem hiding this comment.
The magic number 50 for scan window size should be extracted to a named constant to improve code maintainability and make it easier to adjust if needed.
| val scanWindow = 50 | |
| val scanWindow = SCAN_WINDOW_SIZE |
Description
How to verify
Screenshots / Videos