Skip to content

Fix carousel focus ring: hug the card and update without scroll delay#1692

Merged
utkarshdalal merged 1 commit into
utkarshdalal:masterfrom
VinceBT:fix/carousel-focus-ring
Jul 12, 2026
Merged

Fix carousel focus ring: hug the card and update without scroll delay#1692
utkarshdalal merged 1 commit into
utkarshdalal:masterfrom
VinceBT:fix/carousel-focus-ring

Conversation

@VinceBT

@VinceBT VinceBT commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes two issues with the animated focus ring on library game cards in carousel mode:

  • Ring didn't match the card. The ring was drawn on GridViewCard's outer box. In the carousel that box is fill-sized to the item slot while the visible Card sizes itself via fillMaxWidth().aspectRatio(2/3), so the ring didn't line up with the card. It's now drawn on the Card itself, so it hugs the visible bounds in every layout (grid unchanged).
  • Ring lagged when switching games. In navigateCarousel, requestFocus() ran only after the suspending animateScrollToItem() completed, so the ring waited out the whole scroll animation before moving. Focus is now requested first (the ring follows immediately) and the scroll animates afterwards.

Recording

Before After
Screenshot_20260710_105844_GameNative Screenshot_20260710_114804_GameNative

Type of Change

  • Bug fix
  • Performance / stability improvement
  • Compatibility improvements
  • Other (requires prior approval)

Checklist

  • If I have access to #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.
  • This change aligns with the current project scope (core functionality, stability, or performance). If not, it has been explicitly approved beforehand.
  • I have attached a recording of the change.
  • I have read and agree to the contribution guidelines in CONTRIBUTING.md.

Summary by cubic

Fixes the carousel focus ring so it hugs the visible card and updates immediately when changing selection, without waiting for scroll and avoiding focus exceptions.

  • Bug Fixes
    • Draw the ring on the inner Card instead of the outer slot so it aligns with the card’s 2:3 bounds; grid unchanged.
    • Request focus (guarded with try/catch) before animating scroll, with a small frame delay, so the ring moves right away while the list scrolls and avoids intermittent IllegalStateException.

Written for commit 3f33ed6. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes
    • Improved focus behavior when navigating the library carousel to make keyboard/remote navigation more reliable.
    • Updated focus visual rendering on library cards so the focus ring aligns consistently with the card itself.

@VinceBT VinceBT requested a review from utkarshdalal as a code owner July 10, 2026 09:46
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The changes adjust library UI focus behavior by requesting carousel focus before scrolling and moving the grid card focus ring from the outer wrapper onto the card.

Changes

Library focus behavior

Layer / File(s) Summary
Carousel focus handoff sequencing
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryCarouselPane.kt
navigateCarousel attempts focus after the existing delay, before animateScrollToItem, while retaining the post-animation focus request.
Grid card focus rendering
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryGridCard.kt
The focus halo remains on the outer Box, while focusRing is applied to the inner Card.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: utkarshdalal, phobos665

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change to the carousel focus ring behavior and card alignment.
Description check ✅ Passed The description follows the template with Description, Recording, Type of Change, and Checklist sections filled in.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@VinceBT VinceBT force-pushed the fix/carousel-focus-ring branch from b0e10f1 to 3f33ed6 Compare July 10, 2026 12:05
@utkarshdalal utkarshdalal merged commit 46da0e7 into utkarshdalal:master Jul 12, 2026
1 check passed
@VinceBT VinceBT deleted the fix/carousel-focus-ring branch July 12, 2026 20:00
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.

2 participants