DIY Action To Change Default Keyboard - #809
Draft
thekrdev wants to merge 10 commits into
Draft
Conversation
# Conflicts: # app/src/main/java/com/sameerasw/essentials/domain/diy/Action.kt # app/src/main/java/com/sameerasw/essentials/services/automation/executors/CombinedActionExecutor.kt # app/src/main/java/com/sameerasw/essentials/ui/activities/AutomationEditorActivity.kt
…igger and before setting trigger
thekrdev
force-pushed
the
feature/diy/set-keyboard
branch
from
August 19, 2026 09:42
7181462 to
f269521
Compare
sameerasw
self-requested a review
August 19, 2026 10:07
sameerasw
requested changes
Aug 19, 2026
Owner
There was a problem hiding this comment.
🔴 High Severity Issues (Must Fix Before Merge)
1. Design System & Component Styling Violations in KeyboardSelectionSheet.kt
- Location:
app/src/main/java/com/sameerasw/essentials/ui/features/apps/sheets/KeyboardSelectionSheet.kt(Lines 96–193) - Issues:
- Manual Shape Logic / Rule Violation: The code uses ad-hoc corner math with temporary comments (
// remove it later and replace with official method.):This violates our design guidelines. It should be wrapped inval currentIndex = if (imesList.size == 1) -2 else if (index == (imesList.size - 1)) -1 else index val shape = when (currentIndex) { ... }
RoundedCardContainer(spacing = 2.dp, cornerRadius = 24.dp)using the standard index/count shape segmentation orSegmentedListItem. - Unnecessary Fullscreen Height: The outer Column uses
Modifier.fillMaxSize(), which forces the bottom sheet to take up the full display height even if the user only has 1 or 2 keyboards installed. UseModifier.fillMaxWidth()with.weight(1f, fill = false)on the list container instead. - Bypassed Haptics on RadioButton:
RadioButton(selected = isSelected, onClick = { ... })defines its ownonClickthat bypassesHapticUtil.performUIHaptic(view), causing inconsistent feedback when tapping the radio button vs tapping the row. PassonClick = nullon theRadioButtonand let the row'sModifier.clickablehandle the haptic feedback and selection.
- Manual Shape Logic / Rule Violation: The code uses ad-hoc corner math with temporary comments (
| Settings.Secure.putString(context.contentResolver, Settings.Secure.DEFAULT_INPUT_METHOD, action.inputMethodId) | ||
| return@withContext | ||
| } | ||
| Toast.makeText(context, "give me Write Secure Settings permission to change your keyboard.", Toast.LENGTH_SHORT).show() |
Owner
There was a problem hiding this comment.
2. Hardcoded User-Facing String in Action Executor
- Location:
app/src/main/java/com/sameerasw/essentials/services/automation/executors/CombinedActionExecutor.kt(Line 664) - Issue:
Hardcoded user-facing string directly in Kotlin code. All strings must be declared in
Toast.makeText(context, "give me Write Secure Settings permission to change your keyboard.", Toast.LENGTH_SHORT).show()
strings.xmland localized with proper capitalization (e.g.R.string.msg_write_secure_settings_required).
Owner
There was a problem hiding this comment.
3. Missing isActionConfigured Validation for Action.Keyboard
- Location:
app/src/main/java/com/sameerasw/essentials/ui/activities/AutomationEditorActivity.kt - Issue:
isActionConfigureddoes not validateAction.Keyboard. If a user adds the "Set Keyboard" action without opening the settings sheet,action.inputMethodIdremainsnull. When triggered, it will execute withnull, which can clear the user's default keyboard in Android settings or fail. - Fix: In the editor's configuration check, ensure
action.inputMethodId != null && action.inputMethodId.isNotBlank().
Comment on lines
+1304
to
+1322
| if (showSetKeyboardSheet && configAction is Action.Keyboard) { | ||
| KeyboardSelectionSheet( | ||
| onDismissRequest = { newIme -> | ||
| showSetKeyboardSheet = false | ||
| when (automationType) { | ||
| Automation.Type.TRIGGER -> selectedAction = Action.Keyboard(newIme) | ||
| Automation.Type.ACTION_SHORTCUT, Automation.Type.PIXEL_SEARCHBAR -> selectedAction = | ||
| Action.Keyboard(newIme) | ||
|
|
||
| Automation.Type.STATE, Automation.Type.APP -> { | ||
| if (selectedActionTab == 0) selectedInAction = Action.Keyboard(newIme) | ||
| else selectedOutAction = Action.Keyboard(newIme) | ||
| } | ||
| } | ||
| configAction = null | ||
| }, | ||
| selectedIme | ||
| ) | ||
| } |
Owner
There was a problem hiding this comment.
4. Reactive State Handling for selectedIme in Editor
- Location:
app/src/main/java/com/sameerasw/essentials/ui/activities/AutomationEditorActivity.kt(Lines 1012–1016 & Lines 1304–1322) - Issue:
When invokingKeyboardSelectionSheet,selectedImeis passed as a positional parameter and is not cleanly synced or reset when switching tabs (In/Out actions for App/State automations). - Fix: Pass
selectedIme = (configAction as? Action.Keyboard)?.inputMethodIddirectly toKeyboardSelectionSheetusing named parameters.
Comment on lines
+151
to
+190
| Row( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .clip(shape) | ||
| .clickable { | ||
| HapticUtil.performUIHaptic(view) | ||
| if (isEnabled) { | ||
| defaultInputMethod = ime.id | ||
| return@clickable | ||
| } | ||
| Toast.makeText( | ||
| context, | ||
| R.string.diy_set_keyboard_input_method_disabled, | ||
| Toast.LENGTH_SHORT | ||
| ).show() | ||
| } | ||
| .background(MaterialTheme.colorScheme.surfaceBright) | ||
| .padding(16.dp), | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| horizontalArrangement = Arrangement.spacedBy(12.dp) | ||
| ) { | ||
| Image( | ||
| bitmap = ime.loadIcon(context.packageManager).toBitmap() | ||
| .asImageBitmap(), | ||
| contentDescription = ime.serviceInfo.name, | ||
| modifier = Modifier.size(24.dp), | ||
| contentScale = ContentScale.Fit | ||
| ) | ||
| Text( | ||
| text = ime.loadLabel(context.packageManager).toString(), | ||
| style = MaterialTheme.typography.bodyLarge, | ||
| modifier = Modifier.weight(1f), | ||
| color = MaterialTheme.colorScheme.onSurface | ||
| ) | ||
| RadioButton( | ||
| selected = isSelected, | ||
| onClick = { defaultInputMethod = ime.id }, | ||
| enabled = isEnabled | ||
| ) | ||
| } |
Owner
There was a problem hiding this comment.
There is a re-usable component already made for this row
thekrdev
force-pushed
the
feature/diy/set-keyboard
branch
from
August 19, 2026 12:41
5a0c94c to
bee40a4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added
Changes