Skip to content

Conversation

@utkarshdalal
Copy link
Owner

@utkarshdalal utkarshdalal commented Jan 23, 2026


Summary by cubic

Adds dual-screen support with external display input modes and optional screen swap. You can now render the game on a connected display while using the device as a touchpad/keyboard controller.

  • New Features

    • External display input modes: off, touchpad, keyboard, hybrid (touchpad + toggleable on-screen keyboard).
    • Screen swap: move the game to the external display; device shows touchpad and/or keyboard overlay based on mode.
    • Settings: “External Display Input” and “Show Game on External Display” added to container config and persisted.
    • Integrated with XServer: safe Presentation usage, lifecycle start/stop, and touch routing that prioritizes the overlay.
    • New on-screen keyboard, overlay, and UI colors; added translations.
  • Refactors

    • Replaced the old external keyboard with a new ExternalOnScreenKeyboardView, removed unused deps, added proper key down/up and pressed states, and prevented presentations from stealing focus.

Written for commit 5574076. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • External display support with configurable input modes: Off, Touchpad, Keyboard, Hybrid.
    • Option to swap game content to an external display.
    • On-screen keyboard, touchpad and hybrid overlay controls with toggle and lifecycle handling.
    • Settings persisted and exposed in container/config so mode and swap persist across sessions.
  • Documentation

    • Added localized UI strings for external display settings (DA, DE, FR, PT-BR, RO, UK, ZH).

✏️ Tip: You can customize this high-level summary in your review settings.

SapphireRhodonite and others added 12 commits December 15, 2025 10:40
This commit refactors the external display input handling.

It removes the `ExternalKeyboardView` and replaces it with a new `ExternalOnScreenKeyboardView` for a cleaner implementation. The `winHandler` and `inputControlsViewProvider` dependencies have been removed from the `ExternalDisplayInputController` and related classes.

The "Hybrid" mode UI is updated to use a floating action button to toggle the on-screen keyboard visibility over the touchpad, instead of switching between two full-screen views.
# Conflicts:
#	app/src/main/res/values/strings.xml
externaldisplay: add screen swap support
# Conflicts:
#	app/src/main/res/values-uk/strings.xml
@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Adds external display support: input modes (OFF, TOUCHPAD, KEYBOARD, HYBRID), swap-to-external display, controllers and UI for keyboard/touchpad overlays, persistence in Container/PrefManager, view wiring in XServerScreen, resources and localized strings, and new color assets.

Changes

Cohort / File(s) Summary
External Display Controllers
app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt, app/src/main/java/app/gamenative/externaldisplay/ExternalDisplaySwapController.kt
New controllers: InputController manages input-mode presentations (OFF/TOUCHPAD/KEYBOARD/HYBRID) and display listeners; SwapController moves the game view between internal and external Presentation displays and tracks external state.
External Display UI Components
app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt, app/src/main/java/app/gamenative/externaldisplay/SwapInputOverlayView.kt
New on-screen keyboard view (key layout, shift/caps, XServer injection) and overlay composing keyboard/touchpad/toggle logic for HYBRID mode and visibility toggles.
XServer Screen Integration
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
Integrates controllers and SwapInputOverlayView, adds gameHost container, routes pointer events to overlay when visible, and manages controller lifecycle on attach/detach.
Preferences & Config UI
app/src/main/java/app/gamenative/PrefManager.kt, app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt
PrefManager gains externalDisplayInputMode and externalDisplaySwap keys. ContainerConfigDialog adds dropdown for mode and switch for swap, and binds them to config state.
Container Model & Utils
app/src/main/java/app/gamenative/utils/ContainerUtils.kt, app/src/main/java/com/winlator/container/Container.java, app/src/main/java/com/winlator/container/ContainerData.kt
Container and ContainerData extended with externalDisplayMode and externalDisplaySwap (+defaults, persistence, getters/setters). ContainerUtils propagates these fields through creation and persistence paths.
Resources & Colors
app/src/main/res/values*/strings.xml (multiple locales), app/src/main/res/values/colors.xml
Added localized strings for external display input/modes/swap across languages and five new color resources for external surface/keyboard/key states.

Sequence Diagram(s)

sequenceDiagram
    participant XScreen as XServerScreen
    participant DIC as ExternalDisplayInputController
    participant DSC as ExternalDisplaySwapController
    participant DM as DisplayManager
    participant Pres as Presentation
    participant Input as InputViews

    XScreen->>DIC: create(context, xServer, touchpadProvider)
    XScreen->>DSC: create(context, xServerViewProvider, internalHostProvider)
    XScreen->>DIC: start()
    XScreen->>DSC: start()
    DM->>DIC: onDisplayAdded/Changed()
    DIC->>DIC: find presentation display
    DIC->>Pres: create/update ExternalInputPresentation(mode)
    Pres->>Input: render TOUCHPAD/KEYBOARD/HYBRID content
    DM->>DSC: display events (when swap enabled)
    DSC->>Pres: create GamePresentation on external display
    DSC->>DSC: moveGameToExternal() / moveGameToInternal()
    Input->>XScreen: user input (touch/keys)
    XScreen->>DIC: setMode(newMode)
    DIC->>Pres: update presentation content
Loading
sequenceDiagram
    participant User as User
    participant UI as ContainerConfigDialog
    participant Pref as PrefManager
    participant Container as Container
    participant CD as ContainerData

    User->>UI: select external display mode / toggle swap
    UI->>CD: update externalDisplayMode / externalDisplaySwap
    UI->>Pref: set externalDisplayInputMode / externalDisplaySwap
    Pref-->>Pref: persist to DataStore
    Note over Container: on startup
    Container->>Pref: read externalDisplayInputMode / externalDisplaySwap
    Pref-->>Container: return values
    Container->>CD: populate fields
    CD-->>XScreen: applied config
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I hopped onto a second screen tonight,

Keys and touchpads dancing in moonlight,
Hybrid toggles, swap and play,
External views lead the way—
Hooray for hops and extra sight!

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is vague and generic. 'Dual screen support utkarsh2' uses non-descriptive phrasing that doesn't clearly convey the specific technical changes or main features being added. Consider a more specific title that captures the core functionality, such as 'Add external display input modes and screen swap functionality' or 'Implement dual-display support with input routing and overlay.'
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

# Conflicts:
#	app/src/main/java/app/gamenative/utils/ContainerUtils.kt
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 20 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt">

<violation number="1" location="app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt:162">
P2: Hardcoded `Color.WHITE` should use a color resource for consistency with other colors in this file. Define `R.color.external_display_key_text` and use `ContextCompat.getColor(context, R.color.external_display_key_text)` to allow centralized theming.

(Based on your team's feedback about not hardcoding UI colors.) [FEEDBACK_USED]</violation>
</file>

<file name="app/src/main/java/app/gamenative/externaldisplay/ExternalDisplaySwapController.kt">

<violation number="1" location="app/src/main/java/app/gamenative/externaldisplay/ExternalDisplaySwapController.kt:51">
P2: Empty catch block silently swallows exceptions. At minimum, log the exception to aid debugging when issues arise with display listener cleanup.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

keys.forEach { spec ->
val button = Button(context).apply {
isAllCaps = false
setTextColor(Color.WHITE)
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 23, 2026

Choose a reason for hiding this comment

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

P2: Hardcoded Color.WHITE should use a color resource for consistency with other colors in this file. Define R.color.external_display_key_text and use ContextCompat.getColor(context, R.color.external_display_key_text) to allow centralized theming.

(Based on your team's feedback about not hardcoding UI colors.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt, line 162:

<comment>Hardcoded `Color.WHITE` should use a color resource for consistency with other colors in this file. Define `R.color.external_display_key_text` and use `ContextCompat.getColor(context, R.color.external_display_key_text)` to allow centralized theming.

(Based on your team's feedback about not hardcoding UI colors.) </comment>

<file context>
@@ -0,0 +1,330 @@
+        keys.forEach { spec ->
+            val button = Button(context).apply {
+                isAllCaps = false
+                setTextColor(Color.WHITE)
+                setTextSize(16f)
+                typeface = Typeface.DEFAULT_BOLD
</file context>
Fix with Cubic

dismissPresentation()
try {
displayManager?.unregisterDisplayListener(displayListener)
} catch (_: Exception) {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 23, 2026

Choose a reason for hiding this comment

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

P2: Empty catch block silently swallows exceptions. At minimum, log the exception to aid debugging when issues arise with display listener cleanup.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/externaldisplay/ExternalDisplaySwapController.kt, line 51:

<comment>Empty catch block silently swallows exceptions. At minimum, log the exception to aid debugging when issues arise with display listener cleanup.</comment>

<file context>
@@ -0,0 +1,150 @@
+        dismissPresentation()
+        try {
+            displayManager?.unregisterDisplayListener(displayListener)
+        } catch (_: Exception) {
+        }
+    }
</file context>
Fix with Cubic

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In
`@app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt`:
- Around line 62-142: The keyboard view currently hardcodes user-visible labels
in buildLayout() (e.g., "Shift", "Caps", "Space", "Enter", "Tab", "Esc") via
KeySpec calls; extract these into string resources and replace the literal
strings with calls that fetch localized strings (e.g.,
context.getString(R.string.key_shift)) when constructing KeySpec in
buildLayout() and the other affected block (lines ~243-252), ensuring KeySpec
usage (key label parameters) still matches the existing constructors and keeping
weights/actions unchanged.

In `@app/src/main/java/app/gamenative/externaldisplay/SwapInputOverlayView.kt`:
- Around line 46-63: The keyboard toggle ImageButton (keyboardToggleButton in
SwapInputOverlayView) lacks a contentDescription, making it invisible to
accessibility services; add a descriptive contentDescription for screen readers
(e.g., via setContentDescription or contentDescription property) when
initializing keyboardToggleButton, using a string resource like
R.string.external_display_keyboard_toggle (create this string if missing) and
ensure it remains in sync with toggleKeyboard() behavior.

In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 898-904: Remove the empty block for the onViewAttachedToWindow
override to satisfy detekt by replacing it with an expression body; in the
addOnAttachStateChangeListener object (the View.OnAttachStateChangeListener
implementation), change "override fun onViewAttachedToWindow(v: View) {}" to
"override fun onViewAttachedToWindow(v: View) = Unit" so only the
onViewDetachedFromWindow contains the stop calls
(externalDisplayController?.stop(), swapController?.stop()) and detekt no longer
flags an empty block.
🧹 Nitpick comments (6)
app/src/main/res/values-pt-rBR/strings.xml (1)

358-365: Minor terminology inconsistency in translations.

The translations are accurate, but there's a slight inconsistency: line 358 uses "exibição externa" while line 359 uses "tela de apresentação" (presentation screen). For consistency, consider using the same term throughout—either "exibição externa" or "tela externa" would work well for both.

💡 Suggested fix for consistency
 <string name="external_display_input">Entrada de exibição externa</string>
-<string name="external_display_input_subtitle">Escolha como uma tela de apresentação conectada deve se comportar</string>
+<string name="external_display_input_subtitle">Escolha como uma tela externa conectada deve se comportar</string>
app/src/main/res/values-uk/strings.xml (1)

527-534: Minor terminology inconsistency in subtitle.

Line 528 uses "презентаційний дисплей" (presentation display) while all other strings in this section use "зовнішній дисплей" (external display). Consider using consistent terminology for better user experience.

Suggested fix
 <string name="external_display_input">Ввід зовнішнього дисплея</string>
-<string name="external_display_input_subtitle">Виберіть, як має працювати підключений презентаційний дисплей</string>
+<string name="external_display_input_subtitle">Виберіть, як має працювати підключений зовнішній дисплей</string>
app/src/main/java/com/winlator/container/Container.java (1)

970-972: Consider validating the external display mode value.

The setter accepts any string value. Unlike setSteamType() which validates and normalizes the input, setExternalDisplayMode() stores any value directly. Consider adding validation to ensure only valid modes are accepted.

♻️ Suggested validation pattern
 public void setExternalDisplayMode(String externalDisplayMode) {
-    this.externalDisplayMode = externalDisplayMode != null ? externalDisplayMode : DEFAULT_EXTERNAL_DISPLAY_MODE;
+    if (externalDisplayMode == null) {
+        this.externalDisplayMode = DEFAULT_EXTERNAL_DISPLAY_MODE;
+    } else {
+        switch (externalDisplayMode) {
+            case EXTERNAL_DISPLAY_MODE_OFF:
+            case EXTERNAL_DISPLAY_MODE_TOUCHPAD:
+            case EXTERNAL_DISPLAY_MODE_KEYBOARD:
+            case EXTERNAL_DISPLAY_MODE_HYBRID:
+                this.externalDisplayMode = externalDisplayMode;
+                break;
+            default:
+                this.externalDisplayMode = DEFAULT_EXTERNAL_DISPLAY_MODE;
+                break;
+        }
+    }
 }
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (1)

163-168: Avoid duplicated mode/index mapping.
The mode→index and index→mode mapping appears in two places; a single list of (mode, label) pairs would prevent drift when modes change.

♻️ Suggested refactor
-        val externalDisplayModes = listOf(
-            stringResource(R.string.external_display_mode_off),
-            stringResource(R.string.external_display_mode_touchpad),
-            stringResource(R.string.external_display_mode_keyboard),
-            stringResource(R.string.external_display_mode_hybrid),
-        )
+        val externalDisplayModeItems = listOf(
+            Container.EXTERNAL_DISPLAY_MODE_OFF to stringResource(R.string.external_display_mode_off),
+            Container.EXTERNAL_DISPLAY_MODE_TOUCHPAD to stringResource(R.string.external_display_mode_touchpad),
+            Container.EXTERNAL_DISPLAY_MODE_KEYBOARD to stringResource(R.string.external_display_mode_keyboard),
+            Container.EXTERNAL_DISPLAY_MODE_HYBRID to stringResource(R.string.external_display_mode_hybrid),
+        )
+        val externalDisplayModes = externalDisplayModeItems.map { it.second }
-        var externalDisplayModeIndex by rememberSaveable {
-            val index = when (config.externalDisplayMode.lowercase()) {
-                Container.EXTERNAL_DISPLAY_MODE_TOUCHPAD -> 1
-                Container.EXTERNAL_DISPLAY_MODE_KEYBOARD -> 2
-                Container.EXTERNAL_DISPLAY_MODE_HYBRID -> 3
-                else -> 0
-            }
-            mutableIntStateOf(index)
-        }
+        var externalDisplayModeIndex by rememberSaveable {
+            val index = externalDisplayModeItems.indexOfFirst { it.first == config.externalDisplayMode }
+                .coerceAtLeast(0)
+            mutableIntStateOf(index)
+        }
-                                    onItemSelected = { index ->
-                                        externalDisplayModeIndex = index
-                                        config = config.copy(
-                                            externalDisplayMode = when (index) {
-                                                1 -> Container.EXTERNAL_DISPLAY_MODE_TOUCHPAD
-                                                2 -> Container.EXTERNAL_DISPLAY_MODE_KEYBOARD
-                                                3 -> Container.EXTERNAL_DISPLAY_MODE_HYBRID
-                                                else -> Container.EXTERNAL_DISPLAY_MODE_OFF
-                                            },
-                                        )
-                                    },
+                                    onItemSelected = { index ->
+                                        externalDisplayModeIndex = index
+                                        config = config.copy(
+                                            externalDisplayMode = externalDisplayModeItems[index].first
+                                        )
+                                    },

Also applies to: 687-695, 1729-1737

app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt (2)

70-76: Consider logging suppressed exceptions for debugging.

The empty catch block silently swallows exceptions during cleanup. While this is acceptable for teardown code, logging the exception (even at debug level) would help with troubleshooting potential issues.

♻️ Suggested improvement
 fun stop() {
     dismissPresentation()
     try {
         displayManager?.unregisterDisplayListener(displayListener)
-    } catch (_: Exception) {
+    } catch (e: Exception) {
+        android.util.Log.d("ExternalDisplayInput", "Failed to unregister listener", e)
     }
 }

115-123: Consider extracting duplicate display detection logic.

This findPresentationDisplay() implementation is identical to the one in ExternalDisplaySwapController.kt. Extracting this to a shared utility would reduce duplication and ensure consistent behavior across both controllers.

♻️ Suggested approach

Create a shared utility function:

// In a new file like ExternalDisplayUtils.kt
object ExternalDisplayUtils {
    fun findPresentationDisplay(context: Context): Display? {
        val displayManager = context.getSystemService(DisplayManager::class.java)
        val currentDisplay = context.display ?: return null
        return displayManager
            ?.getDisplays(DisplayManager.DISPLAY_CATEGORY_PRESENTATION)
            ?.firstOrNull { display ->
                display.displayId != currentDisplay.displayId && display.name != "HiddenDisplay"
            }
    }
}

Then use ExternalDisplayUtils.findPresentationDisplay(context) in both controllers.

Comment on lines +62 to +142
private fun buildLayout() {
addRow(
listOf(
KeySpec("Esc", keycode = XKeycode.KEY_ESC, weight = 1.25f, action = Action.ESC),
KeySpec("1", "!", XKeycode.KEY_1),
KeySpec("2", "@", XKeycode.KEY_2),
KeySpec("3", "#", XKeycode.KEY_3),
KeySpec("4", "$", XKeycode.KEY_4),
KeySpec("5", "%", XKeycode.KEY_5),
KeySpec("6", "^", XKeycode.KEY_6),
KeySpec("7", "&", XKeycode.KEY_7),
KeySpec("8", "*", XKeycode.KEY_8),
KeySpec("9", "(", XKeycode.KEY_9),
KeySpec("0", ")", XKeycode.KEY_0),
KeySpec("-", "_", XKeycode.KEY_MINUS),
KeySpec("=", "+", XKeycode.KEY_EQUAL),
KeySpec("", keycode = XKeycode.KEY_BKSP, weight = 1.75f, action = Action.BACKSPACE),
),
)

addRow(
listOf(
KeySpec("Tab", keycode = XKeycode.KEY_TAB, weight = 1.5f, action = Action.TAB),
KeySpec("q", "Q", XKeycode.KEY_Q, isLetter = true),
KeySpec("w", "W", XKeycode.KEY_W, isLetter = true),
KeySpec("e", "E", XKeycode.KEY_E, isLetter = true),
KeySpec("r", "R", XKeycode.KEY_R, isLetter = true),
KeySpec("t", "T", XKeycode.KEY_T, isLetter = true),
KeySpec("y", "Y", XKeycode.KEY_Y, isLetter = true),
KeySpec("u", "U", XKeycode.KEY_U, isLetter = true),
KeySpec("i", "I", XKeycode.KEY_I, isLetter = true),
KeySpec("o", "O", XKeycode.KEY_O, isLetter = true),
KeySpec("p", "P", XKeycode.KEY_P, isLetter = true),
KeySpec("[", "{", XKeycode.KEY_BRACKET_LEFT),
KeySpec("]", "}", XKeycode.KEY_BRACKET_RIGHT),
KeySpec("\\", "|", XKeycode.KEY_BACKSLASH, weight = 1.25f),
),
)

addRow(
listOf(
KeySpec("Shift", weight = 1.75f, action = Action.SHIFT),
KeySpec("a", "A", XKeycode.KEY_A, isLetter = true),
KeySpec("s", "S", XKeycode.KEY_S, isLetter = true),
KeySpec("d", "D", XKeycode.KEY_D, isLetter = true),
KeySpec("f", "F", XKeycode.KEY_F, isLetter = true),
KeySpec("g", "G", XKeycode.KEY_G, isLetter = true),
KeySpec("h", "H", XKeycode.KEY_H, isLetter = true),
KeySpec("j", "J", XKeycode.KEY_J, isLetter = true),
KeySpec("k", "K", XKeycode.KEY_K, isLetter = true),
KeySpec("l", "L", XKeycode.KEY_L, isLetter = true),
KeySpec(";", ":", XKeycode.KEY_SEMICOLON),
KeySpec("'", "\"", XKeycode.KEY_APOSTROPHE),
KeySpec("Enter", keycode = XKeycode.KEY_ENTER, weight = 2.0f, action = Action.ENTER),
),
)

addRow(
listOf(
KeySpec("`", "~", XKeycode.KEY_GRAVE, weight = 1.25f),
KeySpec("z", "Z", XKeycode.KEY_Z, isLetter = true),
KeySpec("x", "X", XKeycode.KEY_X, isLetter = true),
KeySpec("c", "C", XKeycode.KEY_C, isLetter = true),
KeySpec("v", "V", XKeycode.KEY_V, isLetter = true),
KeySpec("b", "B", XKeycode.KEY_B, isLetter = true),
KeySpec("n", "N", XKeycode.KEY_N, isLetter = true),
KeySpec("m", "M", XKeycode.KEY_M, isLetter = true),
KeySpec(",", "<", XKeycode.KEY_COMMA),
KeySpec(".", ">", XKeycode.KEY_PERIOD),
KeySpec("/", "?", XKeycode.KEY_SLASH),
KeySpec("", keycode = XKeycode.KEY_UP, weight = 1.25f, action = Action.ARROW_UP),
),
)

addRow(
listOf(
KeySpec("Space", keycode = XKeycode.KEY_SPACE, weight = 6f, action = Action.SPACE),
KeySpec("", keycode = XKeycode.KEY_LEFT, weight = 1.25f, action = Action.ARROW_LEFT),
KeySpec("", keycode = XKeycode.KEY_DOWN, weight = 1.25f, action = Action.ARROW_DOWN),
KeySpec("", keycode = XKeycode.KEY_RIGHT, weight = 1.25f, action = Action.ARROW_RIGHT),
),
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Localize special key labels.
Strings like “Shift”, “Caps”, “Space”, “Enter”, “Tab”, and “Esc” are user-visible but hardcoded, so they won’t translate with locale changes. Consider moving them to string resources.

Also applies to: 243-252

🤖 Prompt for AI Agents
In
`@app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt`
around lines 62 - 142, The keyboard view currently hardcodes user-visible labels
in buildLayout() (e.g., "Shift", "Caps", "Space", "Enter", "Tab", "Esc") via
KeySpec calls; extract these into string resources and replace the literal
strings with calls that fetch localized strings (e.g.,
context.getString(R.string.key_shift)) when constructing KeySpec in
buildLayout() and the other affected block (lines ~243-252), ensuring KeySpec
usage (key label parameters) still matches the existing constructors and keeping
weights/actions unchanged.

Comment on lines +46 to +63
private val keyboardToggleButton: ImageButton = ImageButton(context).apply {
val density = resources.displayMetrics.density
val sizePx = (56 * density).toInt()
val marginPx = (16 * density).toInt()
layoutParams = LayoutParams(sizePx, sizePx).apply {
gravity = Gravity.BOTTOM or Gravity.END
setMargins(marginPx, marginPx, marginPx, marginPx)
}
background = GradientDrawable().apply {
shape = GradientDrawable.OVAL
setColor(ContextCompat.getColor(context, R.color.external_display_key_background))
}
setImageResource(R.drawable.icon_keyboard)
scaleType = ImageView.ScaleType.CENTER_INSIDE
setPadding(marginPx / 2, marginPx / 2, marginPx / 2, marginPx / 2)
visibility = View.GONE
setOnClickListener { toggleKeyboard() }
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a contentDescription for the keyboard toggle button.
Clickable ImageButtons should be readable by accessibility services.

🔧 Suggested fix
         setImageResource(R.drawable.icon_keyboard)
+        contentDescription = context.getString(R.string.keyboard)
🤖 Prompt for AI Agents
In `@app/src/main/java/app/gamenative/externaldisplay/SwapInputOverlayView.kt`
around lines 46 - 63, The keyboard toggle ImageButton (keyboardToggleButton in
SwapInputOverlayView) lacks a contentDescription, making it invisible to
accessibility services; add a descriptive contentDescription for screen readers
(e.g., via setContentDescription or contentDescription property) when
initializing keyboardToggleButton, using a string resource like
R.string.external_display_keyboard_toggle (create this string if missing) and
ensure it remains in sync with toggleKeyboard() behavior.

Comment on lines +898 to +904
frameLayout.addOnAttachStateChangeListener(object : View.OnAttachStateChangeListener {
override fun onViewAttachedToWindow(v: View) {}

override fun onViewDetachedFromWindow(v: View) {
externalDisplayController?.stop()
swapController?.stop()
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove the empty onViewAttachedToWindow block to satisfy detekt.
The empty block is flagged by static analysis; use an expression body or a suppression.

🧹 Suggested fix
             frameLayout.addOnAttachStateChangeListener(object : View.OnAttachStateChangeListener {
-                override fun onViewAttachedToWindow(v: View) {}
+                override fun onViewAttachedToWindow(v: View) = Unit
🧰 Tools
🪛 detekt (1.23.8)

[warning] 899-899: This empty block of code can be removed.

(detekt.empty-blocks.EmptyFunctionBlock)

🤖 Prompt for AI Agents
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around
lines 898 - 904, Remove the empty block for the onViewAttachedToWindow override
to satisfy detekt by replacing it with an expression body; in the
addOnAttachStateChangeListener object (the View.OnAttachStateChangeListener
implementation), change "override fun onViewAttachedToWindow(v: View) {}" to
"override fun onViewAttachedToWindow(v: View) = Unit" so only the
onViewDetachedFromWindow contains the stop calls
(externalDisplayController?.stop(), swapController?.stop()) and detekt no longer
flags an empty block.

@utkarshdalal utkarshdalal merged commit 17b1e06 into master Jan 23, 2026
2 checks passed
@utkarshdalal utkarshdalal deleted the Dual-Screen-Support-utkarsh2 branch January 23, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants