feat: Added theme support for External Display Input#497
feat: Added theme support for External Display Input#497MayRedBeWithYou wants to merge 3 commits intoutkarshdalal:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds a theming system for external display input: new Theme enum with color properties, persisted preference key in PrefManager, settings UI to choose theme, controller API to set theme, and views/presentation updated to render using theme colors and propagate updates. Changes
Sequence DiagramsequenceDiagram
actor User
participant Settings as Settings UI
participant PrefMgr as PrefManager
participant XScreen as XServerScreen
participant DisplayCtrl as ExternalDisplayInputController
participant Presentation as ExternalInputPresentation
participant Views as External Views
User->>Settings: select theme option
Settings->>PrefMgr: save externalDisplayTheme
PrefMgr-->>Settings: persisted
XScreen->>PrefMgr: read externalDisplayTheme
PrefMgr-->>XScreen: return theme
XScreen->>DisplayCtrl: start / setTheme(theme)
DisplayCtrl->>Presentation: updateTheme(theme)
Presentation->>Views: re-render with theme
Views-->>User: display themed UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
There was a problem hiding this comment.
1 issue found across 8 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/ui/screen/settings/SettingsGroupInterface.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt:419">
P2: User-visible dropdown labels are hardcoded instead of using string resources, preventing localization and inconsistent with the rest of the settings UI.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/app/gamenative/externaldisplay/SwapInputOverlayView.kt (1)
55-60:⚠️ Potential issue | 🟠 MajorBug:
keyboardToggleButtonbackground color is not themed.Line 57 uses hardcoded
R.color.external_display_key_backgroundwhile every other color in this file and the equivalent button inHybridInputLayout(line 291 ofExternalDisplayInputController.kt) usestheme.keyBg. This means the AMOLED theme won't apply to this button's background inSwapInputOverlayView.🐛 Proposed fix
background = GradientDrawable().apply { shape = GradientDrawable.OVAL - setColor(ContextCompat.getColor(context, R.color.external_display_key_background)) + setColor(ContextCompat.getColor(context, theme.keyBg)) }app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt (1)
213-222:⚠️ Potential issue | 🟡 MinorInconsistent icon styling:
alpha = 0.35fhere vs. no alpha inSwapInputOverlayView.In the
ExternalInputPresentationKEYBOARD mode, the hint icon uses bothsetColorFilter(theme.iconColor)(line 220) andalpha = 0.35f(line 221). InSwapInputOverlayView(line 30), onlysetColorFilter(theme.iconColor)is applied—no alpha. This means the hint icon will look different depending on which code path renders it.If the intent is for the theme's
iconColorto fully control opacity, remove the hardcoded alpha here. If the alpha is intentional for the external display, consider documenting why.
🤖 Fix all issues with AI agents
In
`@app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt`:
- Around line 415-429: The dropdown is comparing localized labels ("Default",
"AMOLED") to a stored enum name (PrefManager.externalDisplayTheme, e.g.,
"DEFAULT"), causing indexOf to return -1 and wrong persistence; change the
dropdown to be enum-backed: build a list of enum values (e.g., Theme.values())
or enum-name strings to use as the internal items for value/indexing and
persistence, and separately map those enum entries to localized display labels
for title/visible text; initialize selectedExternalDisplayTheme from
PrefManager.externalDisplayTheme (the enum name), use the enum-name list for
SettingsListDropdown.value/indexOf, and onItemSelected persist the selected
enum.name back to PrefManager.externalDisplayTheme while showing the
corresponding localized label.
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt (1)
160-165: HardcodedColor.WHITEfor key text.Key text color is
Color.WHITEregardless of theme. This works for both current dark themes, but if a light theme is added later, this won't adapt. Consider adding atextColorproperty to theThemeenum for forward-compatibility.app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt (1)
47-52:fromConfigrelies on raw string matching—consider using enum name.
fromConfigmatches"amoled"as a magic string. If a new theme is added, the developer must remember to update this function. UsingTheme.valueOf()(with a fallback) orentries.find { it.name.equals(value, ignoreCase = true) }would auto-extend to new enum values.♻️ Suggested refactor
companion object { - fun fromConfig(value: String?): Theme = when (value?.lowercase()) { - "amoled" -> AMOLED - else -> DEFAULT - } + fun fromConfig(value: String?): Theme = + entries.find { it.name.equals(value, ignoreCase = true) } ?: DEFAULT }
app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
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/ui/screen/settings/SettingsGroupInterface.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt:433">
P2: Dropdown selection state is no longer updated, so the displayed selection can remain stale after the user picks a theme.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
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/ExternalDisplayInputController.kt">
<violation number="1" location="app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt:49">
P2: Theme.fromConfig no longer recognizes the previously stored "AMOLED" preference value, so existing users with that saved theme will fall back to DEFAULT after the rename.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| companion object { | ||
| fun fromConfig(value: String?): Theme = when (value?.lowercase()) { | ||
| "black" -> BLACK |
There was a problem hiding this comment.
P2: Theme.fromConfig no longer recognizes the previously stored "AMOLED" preference value, so existing users with that saved theme will fall back to DEFAULT after the rename.
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/ExternalDisplayInputController.kt, line 49:
<comment>Theme.fromConfig no longer recognizes the previously stored "AMOLED" preference value, so existing users with that saved theme will fall back to DEFAULT after the rename.</comment>
<file context>
@@ -27,26 +27,26 @@ class ExternalDisplayInputController(
companion object {
fun fromConfig(value: String?): Theme = when (value?.lowercase()) {
- "amoled" -> AMOLED
+ "black" -> BLACK
else -> DEFAULT
}
</file context>
1 participant
I recently posted a #feature-request regarding adding AMOLED theme for External Display Input mode. I decided to give it a shot - let me know what you think.
Summary by cubic
Adds theme support to External Display Input with a new Black theme for true blacks and reduced glare. Themes apply across touchpad, keyboard, hybrid modes, and overlay icons.
Written for commit d62c7d1. Summary will update on new commits.
Summary by CodeRabbit
New Features
Settings
UI