Android TV compatibility (fixes #1363, #899) #2701

Alexander-TX · 2019-10-08T02:46:42Z

I carefully read the contribution guidelines and agree to them.

Most development for Android TV is done with "Leanback" support library. There are two issues with it:

it kind-of sucks (won't delve deeply into that here)
retrofitting an existing application with it is virtually impossible

In absence of Leanback one has to directly face challenges of designing for TV devices: handling focus and TV-friendly design.

Focus-based (aka "directional") navigation is a must because TV devices are remote-driven. Many standard View/ViewGroup subclasses are poorly suited for directional navigation and must be heavily augmented to work with it. Some components have notional support for focus handling but contain annoying bugs due to code rot or lack of testing.

TV-friendly design means two things: clearly highlighting focused element and using big readable fonts everywhere (since most big TV screens have low resolution and questionable LCD panel quality). This pull request does not address font size issues, but does contain a workaround to visually distinguish currently focused element.

There are many known issues with this PR:

There is a fix for bug in RecyclerView, that makes it largely unusable with focus-based navigation, but the fix is applied only to couple of RecyclerView instances: initial screen with videos and comments list
Focus highlight overlay is unsuitable for light theme (should be easy to fix)
Inconsistent use of isFireTV/isInTouchMode (more on that further down)
Drawer is still a bit buggy
When focus reaches a bottom of comments list (and "loading more" progress bar appears) pressing DOWN again moves it elsewhere. This will likely require a bit more customization to fix.
When focus moves from video preview to comments list, the transition is not smooth (this is result of architectural flaw in AppBarLayout)
Popup player still can't be interacted with

In order to apply fix from 3c4d62c everywhere, every RecyclerView in NewPipe and every adapter must be replaces with NewPipeRecyclerView and FixedGridAdapter/SuperScrollAdapter. I avoided doing so to reduce number of conflicts during rebasing, so please do it yourself if you merge this PR

Regarding focus-based navigation, — it needs extra explanation.

The general approach to handling focus in Android is to call isInTouchMode(), and make decisions depending on returned value. Ideally, there should be no need to distinguish between Android TV devices, Android Auto, Android phones with dpad and other kinds of devices — directional navigation should work that same way everywhere. In real world this is not so simple: for example, hiding side drawer on back press might not be useful on most Android devices with touchscreens (because it is easier to dismiss it with finger), but is absolutely mandatory on TV devices, controlled with IR remote. And we can't easily check if device has touchsreen — some devices lie that they have it, because apps refuse to work or install otherwise, but in reality they have some kind of less-that-perfect touchscreen emulator. So there are still some checks for isFireTV() (which now detects any TV devices) and some checks for isInTouchMode().

fixes #1363, #899

AppBarLayout mostly gets it, but we still need to uphold our own part - expanding it back after focus returns to it

We don't hide MainFragment when search is show, so FocusFinder sometimes gives focus to (obscured) main content

GridLayoutManager is buggy - https://issuetracker.google.com/issues/37067220: it randomly loses or incorrectly assigns focus when being scrolled via direction-based navigation. This commit reimplements onFocusSearchFailed() on top of scrollBy() to work around that problem. Ordinary touch-based navigation should not be affected.

Focus isn't needed for marquee, only selection

The same logic is present in RecyclerView, ScrollView etc. Android really should default to this behavior for all Views with isScrollContainer = true

* Hide player controls when back is pressed (only on TV devices) * Do not hide control after click unless in touch mode * Show player controls on dpad usage * Notably increase control hide timeout when not in touch mode

Buttons are more likely to have "correct" styling and are focusable/clickable out of box

Upstream DrawerLayout does override addFocusables, but incorrectly checks for isDrawerOpen instread of isDrawerVisible

It is still buggy because of NavigationView (why the hell is NavigationMenuView marked as focusable?) but at least initial opening works as intended

Redirion · 2019-10-08T08:15:21Z

could you try whether the bugs of RecyclerView and GridLayoutManager are still present with the latest beta version?
in build.gradle for module app replace the current recyclerview dependency with this:
'androidx.recyclerview:recyclerview:1.1.0-beta04'

Changelog: https://developer.android.com/jetpack/androidx/releases/recyclerview

Alexander-TX · 2019-10-08T09:03:29Z

@Redirion

could you try whether the bug of RecyclerView is still present with the latest beta version?
in build.gradle for module app replace the current recyclerview dependency with this:
'androidx.recyclerview:recyclerview:1.1.0-beta04'

I re-checked with androidx.recyclerview:recyclerview:1.1.0-beta04 and the bug is still reproducible.

I believe, that it is described in this Google issue: https://issuetracker.google.com/issues/37067220.

See attached video file for example of problematic behavior. This is how it works after applying my workaround.

Stypox

Code looks good to me, though I have never done anything with TVs and devices without touch. I only asked for some clarifications.
I couldn't test this since I do not have an Android TV.

Stypox · 2019-10-13T09:45:10Z

app/src/main/java/com/google/android/material/appbar/FlingBehavior.java

    public FlingBehavior(Context context, AttributeSet attrs) {
        super(context, attrs);
    }

+    @Override
+    @SuppressLint("NewApi")


Couldn't this cause crashes in Android versions where the new api is used?
https://stackoverflow.com/questions/16601601/understanding-suppresslintnewapi-annotation

This specific @SuppressLint looks like something, that was added a while ago, and have been rendered obsolete without me noticing. No idea, what is it supposed to suppress.

Stypox · 2019-10-13T09:53:56Z

app/src/main/java/org/schabi/newpipe/util/FireTvUtils.java

 import org.schabi.newpipe.App;

 public class FireTvUtils {
+    @SuppressLint("InlinedApi")


Are you sure this is ok? https://stackoverflow.com/questions/19291844/what-is-the-use-of-suppresslintinlinedapi

This code passes inlined constant "android.software.leanback" to PackageManager in order to check whether current device supports Leanback (e,g. it is TV). If it runs on Android version, that does not known what "android.software.leanback" is, hasSystemFeature will simply return false.

The Lint warning there is meaningless.

Stypox · 2019-10-13T09:58:16Z

app/src/main/java/org/schabi/newpipe/views/FocusAwareSeekBar.java

+
+    @Override
+    public void setOnSeekBarChangeListener(OnSeekBarChangeListener l) {
+        this.listener = l == null ? null : new NestedListener(l);


Please add some parenthesis.

Alexander-TX · 2019-10-14T03:14:51Z

Code looks good to me, though I have never done anything with TVs and devices without touch. I only asked for some clarifications.
I couldn't test this since I do not have an Android TV.

Android Studio has presets and system images for Android TV devices, you can use those to test.

TobiGr · 2019-10-14T09:25:56Z

I made some emulator tests with an Android 9 image. No videos play and the player is squashed to phone format. I'll upload some screenshots later. I know that playing videos was no problem when I did some tests with a KitKat image last year. I'll investigate further when I have more spare time.

Alexander-TX · 2019-10-14T09:33:22Z

I made some emulator tests with an Android 9 image. No videos play and the player is squashed to phone format. I'll upload some screenshots later. I know that playing videos was no problem when I did some tests with a KitKat image last year. I'll investigate further when I have more spare time.

This is very odd. I have tested my changes on real hardware with Android 4 and Android 7. Former does not even have "Leanback" feature, so I had to use custom patches (not included in this PR) to check for specific model in isFireTv. Neither of devices had issues you described.

Are you sure, that you tested on TV virtual device? If so, could you try with Android 7?

Alexander-TX · 2019-11-13T09:59:30Z

Closed since I don't have to for this anymore.

I will try to revisit this in my spare time.

Alexander added 16 commits October 7, 2019 17:05

Fix scrolling comments list

3a985a5

AppBarLayout mostly gets it, but we still need to uphold our own part - expanding it back after focus returns to it

Correctly move focus from toolbar search bar to dropdown

9f0e689

We don't hide MainFragment when search is show, so FocusFinder sometimes gives focus to (obscured) main content

Close DrawerLayout on back button press

40e4839

MainPlayer: make title and subtitle non-focusable

fefe402

Focus isn't needed for marquee, only selection

When child of CoordinatorLayout wants focus, show it!

59128aa

The same logic is present in RecyclerView, ScrollView etc. Android really should default to this behavior for all Views with isScrollContainer = true

Do not discriminate against non-Amazon TV boxes

fe3b765

Make description focusable, so TV users can scroll it

b6992e2

Improve usability of MainVideoActivity with directional navigation

8e0926b

* Hide player controls when back is pressed (only on TV devices) * Do not hide control after click unless in touch mode * Show player controls on dpad usage * Notably increase control hide timeout when not in touch mode

Make player screen controls into buttons

11ccc17

Buttons are more likely to have "correct" styling and are focusable/clickable out of box

Do not allow focus to escape from open DrawerLayout

be7e5dd

Upstream DrawerLayout does override addFocusables, but incorrectly checks for isDrawerOpen instread of isDrawerVisible

Support for seeking videos in directional navigation mode

ca843f1

Focus drawer when it opens

21f1e7f

It is still buggy because of NavigationView (why the hell is NavigationMenuView marked as focusable?) but at least initial opening works as intended

Implement global focus highlight

f37adfb

Focus video view thumbnail after it is loaded

962eaa9

Port focus scroll fixes to SuperScrollLayoutManager

b675d29

Alexander added 2 commits October 9, 2019 17:11

Fix scroll when a view within comment item is focused

4926a59

Allow comment links (if any) to gain focus

c5b818c

Alexander-TX force-pushed the eltex-tv-fixes branch from 5e70744 to c5b818c Compare October 9, 2019 10:20

Stypox requested changes Oct 13, 2019

View reviewed changes

TobiGr mentioned this pull request Oct 15, 2019

Release 0.17.4 #2715

Closed

Stypox mentioned this pull request Oct 15, 2019

Newpipe Default player issue #2728

Closed

Alexander-TX closed this Nov 13, 2019

Alexander-- mentioned this pull request Nov 16, 2019

Android TV support #2806

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android TV compatibility (fixes #1363, #899) #2701

Android TV compatibility (fixes #1363, #899) #2701

Alexander-TX commented Oct 8, 2019 •

edited

Loading

Redirion commented Oct 8, 2019 •

edited

Loading

Alexander-TX commented Oct 8, 2019 •

edited

Loading

Stypox left a comment

Stypox Oct 13, 2019

Alexander-TX Oct 14, 2019 •

edited

Loading

Stypox Oct 13, 2019

Alexander-TX Oct 14, 2019

Stypox Oct 13, 2019

Alexander-TX commented Oct 14, 2019

TobiGr commented Oct 14, 2019

Alexander-TX commented Oct 14, 2019 •

edited

Loading

Alexander-TX commented Nov 13, 2019

Android TV compatibility (fixes #1363, #899) #2701

Android TV compatibility (fixes #1363, #899) #2701

Conversation

Alexander-TX commented Oct 8, 2019 • edited Loading

Redirion commented Oct 8, 2019 • edited Loading

Alexander-TX commented Oct 8, 2019 • edited Loading

Stypox left a comment

Choose a reason for hiding this comment

Stypox Oct 13, 2019

Choose a reason for hiding this comment

Alexander-TX Oct 14, 2019 • edited Loading

Choose a reason for hiding this comment

Stypox Oct 13, 2019

Choose a reason for hiding this comment

Alexander-TX Oct 14, 2019

Choose a reason for hiding this comment

Stypox Oct 13, 2019

Choose a reason for hiding this comment

Alexander-TX commented Oct 14, 2019

TobiGr commented Oct 14, 2019

Alexander-TX commented Oct 14, 2019 • edited Loading

Alexander-TX commented Nov 13, 2019

Alexander-TX commented Oct 8, 2019 •

edited

Loading

Redirion commented Oct 8, 2019 •

edited

Loading

Alexander-TX commented Oct 8, 2019 •

edited

Loading

Alexander-TX Oct 14, 2019 •

edited

Loading

Alexander-TX commented Oct 14, 2019 •

edited

Loading