feat: disable IME extract UI and centralize singleLine keyboard behavior#1014
Open
jeremybernstein wants to merge 1 commit intoutkarshdalal:masterfrom
Open
feat: disable IME extract UI and centralize singleLine keyboard behavior#1014jeremybernstein wants to merge 1 commit intoutkarshdalal:masterfrom
jeremybernstein wants to merge 1 commit intoutkarshdalal:masterfrom
Conversation
Contributor
📝 WalkthroughWalkthroughIntroduces Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant IME
participant Interceptor as InterceptPlatformTextInput
participant TextField as NoExtractOutlinedTextField
participant Focus as FocusManager
User->>IME: type text / press IME action
IME->>Interceptor: PlatformTextInputMethodRequest (EditorInfo)
Interceptor-->>IME: modify EditorInfo (OR IME_FLAG_NO_EXTRACT_UI) and forward
Interceptor->>TextField: deliver input events / committed text
TextField->>User: render updated text
alt IME action == Done (singleLine)
TextField->>Focus: clearFocus()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/ui/component/settings/SettingsTextField.kt (1)
42-49: Consider parameterizingsingleLinefor flexibility.The hardcoded
singleLine = trueworks for most use cases (button labels, typical settings). However, this component is also used for environment variable values (perSettingsEnvVars.kt), which could theoretically contain longer or multi-part values.Given the 76.dp width constraint already implies compact single-line usage, this is acceptable. If multi-line env var support is needed later, consider exposing
singleLineas a parameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/component/settings/SettingsTextField.kt` around lines 42 - 49, The NoExtractOutlinedTextField usage hardcodes singleLine = true; make this configurable by adding a singleLine: Boolean parameter to the SettingsTextField composable (default true) and forward it into NoExtractOutlinedTextField, then update call sites (e.g., SettingsEnvVars usage) to pass singleLine = false where multi-line values may be needed; ensure default preserves current behavior.app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt (1)
69-91: Consider addingImeAction.Nextto navigate between width and height fields.Both fields use
singleLine = truewithout explicitimeAction, so pressing "Done" on the width field will clear focus instead of moving to the height field. Users can still tap the height field manually, but addingImeAction.Nextwith focus navigation would improve the input flow.♻️ Optional: Add focus navigation between width and height
+val heightFocusRequester = remember { FocusRequester() } + NoExtractOutlinedTextField( - modifier = Modifier.width(128.dp), + modifier = Modifier.width(128.dp).focusRequester(widthFocusRequester), value = state.customScreenWidth.value, onValueChange = { state.customScreenWidth.value = it }, - keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), + keyboardOptions = KeyboardOptions( + keyboardType = KeyboardType.Number, + imeAction = ImeAction.Next, + ), + keyboardActions = KeyboardActions( + onNext = { heightFocusRequester.requestFocus() }, + ), label = { Text(text = stringResource(R.string.width)) }, singleLine = true, ) // ... separator ... NoExtractOutlinedTextField( - modifier = Modifier.width(128.dp), + modifier = Modifier.width(128.dp).focusRequester(heightFocusRequester), value = state.customScreenHeight.value, // ... rest remains the same with singleLine = true for Done behavior )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt` around lines 69 - 91, The width and height NoExtractOutlinedTextField inputs (NoExtractOutlinedTextField using state.customScreenWidth.value and state.customScreenHeight.value) lack an explicit IME action, so pressing "Done" on the width field clears focus instead of moving to the height field; update the first field's KeyboardOptions to include imeAction = ImeAction.Next and add KeyboardActions (or a FocusRequester/LocalFocusManager) to move focus to the height field on Next, and set the height field's imeAction appropriately (e.g., ImeAction.Done) so keyboard navigation flows from the width field to the height field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt`:
- Around line 69-91: The width and height NoExtractOutlinedTextField inputs
(NoExtractOutlinedTextField using state.customScreenWidth.value and
state.customScreenHeight.value) lack an explicit IME action, so pressing "Done"
on the width field clears focus instead of moving to the height field; update
the first field's KeyboardOptions to include imeAction = ImeAction.Next and add
KeyboardActions (or a FocusRequester/LocalFocusManager) to move focus to the
height field on Next, and set the height field's imeAction appropriately (e.g.,
ImeAction.Done) so keyboard navigation flows from the width field to the height
field.
In `@app/src/main/java/app/gamenative/ui/component/settings/SettingsTextField.kt`:
- Around line 42-49: The NoExtractOutlinedTextField usage hardcodes singleLine =
true; make this configurable by adding a singleLine: Boolean parameter to the
SettingsTextField composable (default true) and forward it into
NoExtractOutlinedTextField, then update call sites (e.g., SettingsEnvVars usage)
to pass singleLine = false where multi-line values may be needed; ensure default
preserves current behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9daec99-4575-41e4-ab4f-3fa8b6c81e0a
📒 Files selected for processing (11)
app/src/main/java/app/gamenative/ui/component/NoExtractOutlinedTextField.ktapp/src/main/java/app/gamenative/ui/component/dialog/Box64PresetsDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/ControllerBindingDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/EnvironmentTab.ktapp/src/main/java/app/gamenative/ui/component/dialog/FEXCorePresetsDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/GameFeedbackDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.ktapp/src/main/java/app/gamenative/ui/component/dialog/TouchGestureSettingsDialog.ktapp/src/main/java/app/gamenative/ui/component/settings/SettingsTextField.ktapp/src/main/java/app/gamenative/ui/screen/login/TwoFactorAuthScreen.ktapp/src/main/java/app/gamenative/ui/screen/login/UserLoginScreen.kt
jeremybernstein
force-pushed
the
jb/fix-textfield-ime
branch
from
a9471d6 to
b4d9061
Compare
Contributor
Author
|
Contributor
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
🧠 Learnings used |
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
1 participant
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.
Description
adds
NoExtractOutlinedTextFieldwrapper that setsIME_FLAG_NO_EXTRACT_UI, opting out of the IME's fullscreen extract editing mode. centralizesImeAction.Done+clearFocus()forsingleLinefields, removing per-callsite boilerplate. migrates allOutlinedTextFieldusages across the app (11 files).closes #1013.
Recording
before:
https://github.com/user-attachments/assets/c195692d-3120-44bb-8221-a6fc94844d3e
after:
https://github.com/user-attachments/assets/b96dcd62-6adc-4b44-a26c-66b3969b2695
Test plan
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.