Add an animated focus ring across the library and in-game UI#1655
Merged
utkarshdalal merged 1 commit intoJul 7, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a reusable ChangesFocus ring adoption across UI components
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Contributor
|
Thanks for this! Couple of tidbits:
Thanks! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryTabBar.kt (1)
554-561: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse clickable semantics for action buttons.
Line 555 says these are actions, but
selectable(selected = false)still exposes selectable-item semantics; Compose documentsselectableas normally for mutually exclusive selections. Preferclickable+focusable(interactionSource)here to preserve controller focus without toggle/selection semantics. (developer.android.com)♻️ Suggested refactor
- .selectable( - // These are actions, not toggles; feeding isFocused back as `selected` caused an - // extra recomposition on every focus change. - selected = false, - interactionSource = interactionSource, - indication = null, - onClick = onClick, - ), + .clickable( + interactionSource = interactionSource, + indication = null, + onClick = onClick, + ) + .focusable(interactionSource = interactionSource),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/screen/library/components/LibraryTabBar.kt` around lines 554 - 561, The action items in LibraryTabBar should not use selectable semantics because they are not mutually exclusive selections. Update the modifier chain on the tab bar action button to use clickable for the tap action and keep controller focus via focusable with the same interactionSource, replacing the current selectable(selected = false) usage in the relevant composable.app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt (1)
355-369: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
isFocusedstate inInfoCardis now dead.
isFocusedis set inonFocusChanged(Line 363) but never read anywhere else in the function — the border/color branch that used to consume it was replaced byfocusRing, which tracks focus viainteractionSourceinstead. Leaving it in place triggers a recomposition ofInfoCard's wholeColumnon every focus transition for no visual benefit.♻️ Proposed cleanup
- var isFocused by remember { mutableStateOf(false) } val interactionSource = remember { MutableInteractionSource() } val bringIntoViewRequester = remember { BringIntoViewRequester() } val scope = rememberCoroutineScope() val cardModifier = if (focusableForNavigation) { modifier .bringIntoViewRequester(bringIntoViewRequester) .onFocusChanged { state -> - isFocused = state.isFocused if (state.isFocused) { scope.launch { bringIntoViewRequester.bringIntoView() } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt` around lines 355 - 369, The InfoCard focus state is now unused, so remove the dead `isFocused` state and its assignment from `LibraryAppScreen`’s card modifier setup. Keep the focus behavior driven by `interactionSource`, `focusable`, and `focusRing`, and retain the `bringIntoViewRequester` logic only if it is still needed for scrolling on focus.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/component/AccentActionRow.kt`:
- Line 68: The AccentActionRow focus ring is using the default thickness because
the focusRing call omits width, unlike QuickMenu, LibraryTabBar, and
LibraryAppScreen. Update AccentActionRow’s modifier chain to pass width = 2.dp
into focusRing so the reset row matches the rest of the UI’s focus ring styling.
In `@app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt`:
- Around line 501-503: The tab-bar focus guard in LibraryScreen.kt only covers
the tab-change delay, while the list population effect still calls
requestContentFocusOrDefer() and can steal focus back after the user has moved
to the tab bar. Update the state.appInfoList.size LaunchedEffect path in
LibraryScreen to apply the same tabBarHasFocus check before requesting or
deferring content focus, so slow-loading tabs do not refocus the list
unexpectedly.
- Around line 995-1006: Move the tab-bar focus tracking from the wrapper Box in
LibraryScreen to the focus group that actually owns the tab buttons in
LibraryTabBar. Update the Row with .focusGroup() in LibraryTabBar.kt to use
onFocusChanged so tabBarHasFocus reflects real tab-bar focus enter/leave events,
and keep the existing pendingGridFocusRequest/pendingCarouselFocusRequest reset
logic there.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/screen/library/components/LibraryTabBar.kt`:
- Around line 554-561: The action items in LibraryTabBar should not use
selectable semantics because they are not mutually exclusive selections. Update
the modifier chain on the tab bar action button to use clickable for the tap
action and keep controller focus via focusable with the same interactionSource,
replacing the current selectable(selected = false) usage in the relevant
composable.
In `@app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt`:
- Around line 355-369: The InfoCard focus state is now unused, so remove the
dead `isFocused` state and its assignment from `LibraryAppScreen`’s card
modifier setup. Keep the focus behavior driven by `interactionSource`,
`focusable`, and `focusRing`, and retain the `bringIntoViewRequester` logic only
if it is still needed for scrolling on focus.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ae7a5bf-b13c-4c66-bb6f-f35a3aad667d
📒 Files selected for processing (9)
app/src/main/java/app/gamenative/ui/component/AccentActionRow.ktapp/src/main/java/app/gamenative/ui/component/FocusRing.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryGridCard.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryListCard.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryTabBar.kt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/screen/library/components/LibraryTabBar.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/library/components/LibraryTabBar.kt:610">
P2: The expanded tab items now force a fixed 40dp height and remove vertical padding. At large accessibility font scales, the tab text's line height can exceed 40dp, and because the Box is clipped with `RoundedCornerShape(20.dp)`, the overflowing text will be visually cut off at the top and bottom. This makes tab labels unreadable for users who rely on larger text sizes.
Consider using `heightIn(min = 40.dp)` instead of `height(40.dp)` so the capsule stays 40dp tall in normal use but can grow when needed. If the sliding pill must stay aligned, its height can be derived from the measured tab row rather than hardcoded.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
VinceBT
force-pushed
the
feature/focus-ring-standalone
branch
from
409d285 to
47cf023
Compare
Replaces the static gradient focus borders with a rotating sweep-gradient ring that highlights the focused element when navigating with a controller or D-pad. It only animates while focused, so unfocused elements schedule no frames. Applied to the library grid and list cards, the tab bar, the game page action buttons and info cards, the in-game quick menu, and the screen effects panel rows. The card ring is drawn on each card's outer box rather than the Material Card itself, otherwise the card's own background would cover it.
VinceBT
force-pushed
the
feature/focus-ring-standalone
branch
from
47cf023 to
0220307
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
3 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.
What
Replaces the static gradient focus borders with a rotating sweep-gradient ring that highlights the focused element when navigating with a controller or D-pad. It only animates while focused, so unfocused elements schedule no frames.
Where
Notes
The card ring is drawn on each card's outer box rather than the Material Card itself, since the card's own background would otherwise cover it.
Before:
https://github.com/user-attachments/assets/ea2855df-2c8d-4da5-87ad-7ff363544a4e
After:
https://github.com/user-attachments/assets/4bea6134-0470-4cea-821d-968151034a74
Summary by cubic
Adds an animated sweep‑gradient focus ring across the library and in‑game UI, replacing static borders. It runs only while focused and removes focus flicker and tab‑bar focus yanks.
New Features
Modifier.focusRing(...)inFocusRing.ktfor a rotating sweep‑gradient border; configurable shape and width; animation runs only while focused.AccentActionRow.Bug Fixes
graphicsLayer, and prevented grid focus from pulling back while the tab bar is focused by trackingtabBarHasFocusand canceling deferred focus requests.Written for commit 0220307. Summary will update on new commits.
Summary by CodeRabbit