android / tv-samples

Multiple samples showing best practices in app development on Android TV.
Apache License 2.0
1.02k stars 329 forks source link

Fix so focus stays on Featured Movies carousel while flipping rapidly [JetStreamCompose] #155

Closed randroid88 closed 3 months ago

randroid88 commented 3 months ago

Found an issue when rapidly cycling through the Featured Movies carousel. The focus would jump either up to the TopTabBar or below to the Trending rail (depending on the direction the carousel is being cycled).

Removing the Watch Now button avoids the issue, but it isn't clear why. I didn't go with this fix because it would be removing functionality.

Instead I found that replacing the type of ContentTransform for contentTransformStartToEnd and contentTransformEndToStart animations worked around the issue. Specifically using slide instead of fade and shortening the transition to 100ms instead of 1000ms.

Unclear exactly why the slide transitions don't have the same issue (at least with 100ms duration) as the fade transitions, but this seems like an acceptable cosmetic-only change. I tried reducing the fade transitions to 100ms, but the issue still some times occurs and no animation is noticeable at such a small duration. Increasing the new slide transition to 1000s does cause the same focus issue, but the animation looks good at 100ms so there is no problem.

Example of issue and fix: https://github.com/android/tv-samples/assets/11371505/9c51f6b8-ca43-4fff-b73a-7f8db504c2ca

Device:

randroid88 commented 3 months ago

Root issue might related to having the button inside of the carousel. As per point #2 in this comment: https://issuetracker.google.com/issues/322858968#comment3

You have made use of Buttons within a ClickableSurface which is probably not what you want. Consider using NonInteractiveSurface instead.

chikoski commented 3 months ago

Thanks for sending this pull request. I confirmed the behavior you mentioned in the original comment. I filed the issue as #158.

This pull request replaces the fadeIn / fadeOut with slideIn / slideOut, which can be a workaround, and I would keep the original animation for now.

Thanks again for creating the pull request.