Remove large-land player layout: not actually used

[Stypox]

What is it?

• Bugfix (user facing)
• Feature (user facing)
• Codebase improvement (dev facing)
• Meta improvement to the project (dev facing)

Description of the changes in your PR

While debugging #7731 I found out that any change I made to the large-land layout of the player, didn't have any effect, which really confused me, since in the fullscreen/landscape player the large-land layout should have been used. But it seems like the normal layout is always used instead, so this PR is an experiment that removes the large-land layout completely. Removing it solves the problem of having 800 lines of duplicated layout code.

The diff between the normal layout and the large-land layout before removing it is below here, if it might come in handy for reviewing. As you can see, there are just some formatting differencies, and maybe a couple dp distances are not the same, but nothing big changes. So even if the large-land layout was being used at particular (unknown) times, removing it (which causes the normal layout to always be used) shouldn't make much of a difference.

diff app/src/main/res/layout/player.xml app/src/main/res/layout-large-land/player.xml

204c204,205
<                         tools:ignore="ContentDescription,RtlHardcoded" />
---
>                         tools:ignore="ContentDescription,RtlHardcoded"
>                         tools:visibility="visible" />
214c215
<                         android:paddingStart="6dp"
---
>                         android:paddingStart="3dp"
216c217
<                         android:paddingEnd="6dp"
---
>                         android:paddingEnd="3dp"
222c223,224
<                         tools:ignore="ContentDescription,RtlHardcoded" />
---
>                         tools:ignore="ContentDescription,RtlHardcoded"
>                         tools:visibility="visible" />
256a259
>                         android:textAllCaps="false"
496c499
<                 android:layout_height="40dp"
---
>                 android:layout_height="50dp"
511c514
<                 android:layout_height="60dp"
---
>                 android:layout_height="70dp"
522c525
<                 android:layout_height="40dp"
---
>                 android:layout_height="50dp"
539c542
<         android:layout_width="match_parent"
---
>         android:layout_width="380dp"
540a544
>         android:layout_alignParentEnd="true"
597a602
>                 android:src="@drawable/exo_controls_repeat_off"
599d603
<                 app:srcCompat="@drawable/exo_controls_repeat_off"
733d736
<         style="@style/Widget.AppCompat.Button.Borderless"

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

• I read the contribution guidelines.

[Atemu]

This does not affect the tablet mode player in case anyone's wondering.

[Stypox]

@B0pol @zubayy @EpIcInCoGnItO since you have a real Android TV and you are used to using it, could you test if this PR's apk causes any problem in the player (both fullscreen and not)?

[peat80]

Seems like the large-land-layout icludes some focus related stuff for android tv. 🤔

First picture is with this pr. The buttons at the top and the fullscreen button at the bottom are not reachable with remote control anymore.

[peat80]

Same behaviour within fullscreen player. Buttons are not focusable anymore. Other than that the layout looks the same as in nightlies / releases.

[Stypox]

Thank you for the feedback, I didn't notice there was some code to handle focus in the large-land layout but not in the normal one. I added such code to the normal one now.

Also, the title and channel views (the ones visible while in fullscreen) were clickable & focusable in the normal layout, but not in the large-land one; I made them not-clickable and not-focusable for everyone, since clicking on them actually did/does nothing. I tested that this does not prevent the text views from automatically scrolling when the title is too long, so the behavior is exactly the same as before.

I updated the diff in the PR description based on the above changes

Other actual things that change, by comparing the two files:

• some buttons are a little bit bigger, some a little bit smaller, but nothing that really has a motivation, so I'd leave it like this
• the play queue in large-land previously used only a part of the screen, now it uses the whole screen; can you tell me if this is bad? I personally prefer it covering the whole screen, so that you can read the whole stream titles. Also, I don't see why this behavior should be different between TVs and (fullscreen) phones, where there is plenty of space anyway. See the two screenshots below for a comparison, I tested on emulated API 30 Android TV.

	Before	After removing large-land
Non-fullscreen
Fullscreen

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[Atemu]

The spacing on non-fullscreen video and recommends is better on the left while, on the right, the playlist selector is better IMO.

How do the chapters look like? Those should probably be like playlists are on the left.

[peat80]

The focusing now works exactly as before. 👌
I also like the play queue using full width more than the actual layout. 😎

Mod note: Ignore not related to PR

While your at it could you also fix some focus / highlighting issues on ATV?

The top row on the play queue could only be highlighted as a whole. So no buttons are acessible with remote.

If you want to change the resolution on the fly the choosen resolution is not highlighted so one has to guess where you are (or count the button presses) to change to the desired resolution. But it works.

Also on the speedmenu the 2 checkboxes are focusable but they are only highlighted when unchecked. So another guess (or count). 🤔

Also highlighted items are generally a bit inconsistent marked. Some have only a transparent round or square overlay, some have only a frame, (and, as stated above, some none at all).
And then there are some buttons that have a frame and a transparent overlay. Looks a little strange ui wise. (Especially when there is a square frame with an round transparent overlay.) They should all have the same style probably. 🤔

These issues are of course not introduced with this pr. They exist in normal builds too. Just thought to mention them here.
Would be ok if they will not be fixed in this pr but it could maybe fit in here.

[litetex]

Thank you all for the feedback.

These issues are of course not introduced with this pr. They exist in normal builds too. Just thought to mention them here.
Would be ok if they will not be fixed in this pr but it could maybe fit in here.

This PR only removes the large-hand layout and nothing else.
You might open an issue or more for these problems so we can track them (if there aren't already some open).

[litetex]

LGTM

I like it when code gets deleted 👍

Did a quick test on the emulator and couldn't find any problems.

[Stypox]

@peat80 thank you for the feedback! I just opened a follow-up PR: #7963

Also on the speedmenu the 2 checkboxes are focusable but they are only highlighted when unchecked. So another guess (or count). thinking
Also highlighted items are generally a bit inconsistent marked. S

I will not fix these two things as they are caused by a misuse of themes throughout the app, which is a pretty big change ;-)

Did a quick test on the emulator and couldn't find any problems.

I also tested extensively (while creating #7963), so I can confirm everything works fine