Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YouTube-like UI #2882

Closed
avently opened this issue Dec 20, 2019 · 33 comments · Fixed by #2907
Closed

YouTube-like UI #2882

avently opened this issue Dec 20, 2019 · 33 comments · Fixed by #2907
Labels
discussion This needs to be discussed before anything is done feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface

Comments

@avently
Copy link
Contributor

avently commented Dec 20, 2019

Hello, guys. I'm making Youtube-like page for viewing videos. So will be possible to watch a video, to read comments and to see related videos on the same page. I already did this job previously in #834 and used that version since February 2018 (and it still works fine with some changes of extractor). I couldn't find common ground with contributors of NewPipe in 2018 so this will be a new beginning for us.

This issue is for tracking progress and for exchange of ideas.

I'm planning to:

  • include a video inside a view that already contains comments, description, etc
  • remove app bar from the top of the view
  • place a three dot button and share button inside the video itself
  • unify all players into one logical class (main player, background player, popup in one place) so you will be able to switch between players instantly without loosing playback progress
  • make the whole video fragment hidable via gesture like Android version of Youtube
  • make possible to view the description and comments in fullscreen mode like desktop version of YouTube.

If you have ideas or improvements over the list of tasks you read just tell me here.

@Stypox Stypox added discussion This needs to be discussed before anything is done feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface labels Dec 20, 2019
@notanewbie
Copy link

If no one wants to join you, you could always make your own fork tho, right?

@avently
Copy link
Contributor Author

avently commented Dec 21, 2019

@notanewbie i'm not interested in maintaining a fork so it is not an option.

@justanidea
Copy link
Contributor

Oh hey, welcome again Avently
I would be interested to see where this project would lead to, especially about this big unified logical class

@avently
Copy link
Contributor Author

avently commented Dec 27, 2019

@justanidea hello!
I made a video after a week of work. Youtube screencast
Most of the features for a PR are done so now I'm fixing bugs and making the app more stable.
Good to know that this feature is what people are interested in now.

Please, connect to our conversation @theScrabi @mauriciocolli @TobiGr @karyogamy

@justanidea
Copy link
Contributor

I like how it seems :) Really
But i definitively dont have any weight in these opinions compared to @Stypox for example.
Anyways the work you have done seems full of promises, and i hope for you and this project that it will go better than the last time :)
Could you provide some apk? Your video is well made but i would personnaly prefer trying functionnalities by myself 👍So i guess im not the only one

@avently
Copy link
Contributor Author

avently commented Dec 27, 2019

Here is the latest apk
apk_inside.zip

@bew
Copy link

bew commented Dec 27, 2019

Really nice overview video! 💯
I noticed that when the normal player is active and you scroll the comment or videos suggestions, the video is hidden.
I think we should try to copy the YouTube app and let the comments/suggestions scroll behind the video, wdyt?

@TobiGr
Copy link
Contributor

TobiGr commented Dec 27, 2019

Welcome back @avently! This looks awesome ❤️
I could not install your apk, but the screencast looks nice 👍

make the whole video fragment hidable via gesture like Android version of Youtube

Good 👍

image

I love that you can minimize the player and pin it to the bottom ❤️

remove app bar from the top of the view

This is okay as long as we ensure that all necessary actions are available. When viewing a video page, the app bar contains four things atm:
the up/back button, video resolution, share button and a menu with the options to open the video in a browser, open the downloads, history or settings.
You already removed the downloads, history and settings entries which I would have suggested, too. These functions are quite quick accessible by pulling down the current view and opening the drawer. The up button was replaced by the gesture, too 👍
The question is what to do with the "open in browser" and share buttons on the video detail page. Looking at the screencast, I saw that there is now duplicated info on the screen. The video title and the related channel name are displayed in the video and right under the video. I'd suggest to remove the info from the video to free space (apart from that, the text is hard to read and the text is moving when it is too long).

I think the option to change the aspect ratio (in the screenshot via the "fit" button) is also quite useless. We can remove it, too. In the end, there should be no items in the collapsible menu, so the button for opening it can also be removed. What do you think?

[Please make sure to apply my suggestions only to the embedded video player :)]

unify all players into one logical class (main player, background player, popup in one place) so you will be able to switch between players instantly without loosing playback progress

I understand your reason to do this. But maybe it might be better to create methods to pass the player data to another player. I need to see the implementation before I am able to comment on this, though. I'd be glad to hear @mauriciocolli's, @Stypox's and @Redirion's opinion in this topic, too.

make possible to view the description and comments in fullscreen mode like desktop version of YouTube.

This sounds good for tablets, but I am not sure about phones.


All in all this sounds like a massive code change. Therefore please, please, please do not make one big PR which changes everything, but create smaller PRs addressing only as few points as possible. This will speed up the review.

@avently
Copy link
Contributor Author

avently commented Dec 27, 2019

@TobiGr

The question is what to do with the "open in browser" and share buttons on the video detail page. Looking at the screencast, I saw that there is now duplicated info on the screen.

They are not duplicated, they are moved to embed player (they are not exist under three dots menu since no appbar exists in VideoDetailFragment) . There is no place for them in VideoDetailFragment or I just don't know where to place these buttons.

channel name are displayed in the video and right under the video. I'd suggest to remove the info from the video to free space

Did you mean to remove name & title completely from embed player? Or to show them only in fullscreen mode? Because without showing them in fullscreen mode we will really miss them, I know because I use similiar UI for two years:) So the idea to hide name & title in non-fullscreen player is good while showing them in fullscreen mode.

"fit" button is also quite useless. We can remove it, too. In the end, there should be no items in the collapsible menu, so the button for opening it can also be removed. What do you think?

Now under collapsible menu you can find two additional buttons: screen rotation and play with Kodi.
I'll remove first one since there is no point of using this feature with expandable player. But what to do with Play with Kodi? Where to place open in browser and share?
I can safety hide "Fit" to screen button in embed player but to leave it in popup player (all players use one xml file for UI).
There is another button under collapsible menu and that is for subtitles.
So, there is no way to remove collapsible menu. The only way I see here is to show three dots button inside embed player with all actions inside BottomSheetFragment like YouTube did. But it's not for the first PR for sure:)

But maybe it might be better to create methods to pass the player data to another player

This means to have three similiar classes and to have a method to somehow communicate with each other, to have three simpleExoPlayer with no instant player change (there will be long loading when you switch from one player to another). So I don't understand why somebody may want to have three similiar class which do similiar logic when there is a possibility to have just one class with all logic inside it. And this class was already been made). You can see inside PopupVideoPlayer and BackgroundPlayer and you'll find that many methods have duplicates. By having single class for player will be easy to maintain the code.

This sounds good for tablets, but I am not sure about phones

I didn't implemented it yet but it will not be too hard to do (in another PR). I mean only the ability to scroll down in fullsceen mode which will allow to see description and content under it like you can do in non-fullscreen mode. That's it.

Therefore please, please, please do not make one big PR which changes everything, but create smaller PRs addressing only as few points as possible

Unfortunately it's impossible. Because the making of such player involves too many changes in different classes (MainActivity, VideoDetailFragment, NavigationHelper, ServicePlayerActivity, BackgroundPlayerActivity, XML's for players, activity, VideoDetailFragment). This is architectural change and I can't change only VideoDetailFragment without changing other pieces of puzzle, for example.
To make review easier I will not remove useless code (old players, XML and pieces of code that doesn't make sense now, like code for toolbar in VideoDetailPlayer). Then, after review, I'll remove the useless code. Also I will not include other functionallity I made in first PR (like video progress changing via swipe gesture). There will be no useless changes in code like formatting, etc. I'll do my best to make the review easier.

For now 29 files changed, 4870 insertions(+), 629 deletions. I didn't delete any file yet. PR is large. Maybe a half from it is just a copy-paste from another players and also I will made code optimization before making the PR.

Thank you for your detailed comment!

@bew

I think we should try to copy the YouTube app and let the comments/suggestions scroll behind the video, wdyt?

What are pros and cons of this suggestion? I see only cons because it would allow a limited place for comments. It would be hard to scroll down and to read comments. Also please note that player can be used on phones with a small screen. Also there is such feature as multiwindow support. It means you can open two applications on one screen and every application will have only half of the screen (or even less). It would be painfull to scroll a content under a video. Do you agree or I didn't see something?

@bew
Copy link

bew commented Dec 28, 2019

What are pros and cons of this suggestion?

I like playing a video and scroll to the comment section during the intro, and when it becomes interesting I can watch the video right away without losing my position in the comment section.

Also there is such feature as multiwindow support.

I know, and I use it often with the YT app!
When you screen split, there is 3 split configuration: (1) equal spaces for the 2 apps, (2) more space for the top app or (3) more space for the bottom app.
What YT does is that when the screen is split in (1) or (2) config, only the video is shown. Only in config (3) is the video + comments/suggestions section shown.

It's a habit to take, but it's not that bad, just have to remember to use config (3) if you want more that just the video.

I don't know for small screens, never had any.
But I'm thinking it'll not be a problem, because if you want the comment section, you could always enable the background player to hide its ui (and only have comments/suggestions section)

@bew
Copy link

bew commented Dec 28, 2019

For the review, maybe try to first make a PR with only the unification of the players (copying files and methods) but without too much improvement on the UI, then the next PRs would build on that and only show revelant changes?

@TobiGr
Copy link
Contributor

TobiGr commented Dec 28, 2019

The question is what to do with the "open in browser" and share buttons on the video detail page. Looking at the screencast, I saw that there is now duplicated info on the screen.

They are not duplicated, they are moved to embed player (they are not exist under three dots menu since no appbar exists in VideoDetailFragment) . There is no place for them in VideoDetailFragment or I just don't know where to place these buttons.

The duplicated info was related to the video title and channel name. Sorry I inserted a line break at the wrong position.

channel name are displayed in the video and right under the video. I'd suggest to remove the info from the video to free space

Did you mean to remove name & title completely from embed player? Or to show them only in fullscreen mode? Because without showing them in fullscreen mode we will really miss them, I know because I use similiar UI for two years:) So the idea to hide name & title in non-fullscreen player is good while showing them in fullscreen mode.

Yes, please only show it in the fullscreen mode.

This sounds good for tablets, but I am not sure about phones

I didn't implemented it yet but it will not be too hard to do (in another PR). I mean only the ability to scroll down in fullsceen mode which will allow to see description and content under it like you can do in non-fullscreen mode. That's it.

I think scrolling down will be the problem here. There are gestures to change volume and brightness already. There should be enough space to split the area where we are waiting for those gestures and the gesture to view the comments, but the display is smaller on phones and therefore we cannot rearrange the gestures. We should discuss this in a new issue to hear more opinions.

For the review, maybe try to first make a PR with only the unification of the players (copying files and methods) but without too much improvement on the UI, then the next PRs would build on that and only show revelant changes?

That's what I though about, but if this is too much work to split everything up again, we'll have to deal with it. However this could slow down the review, because this would require us to take care of too many aspects.

@avently
Copy link
Contributor Author

avently commented Dec 28, 2019

I think scrolling down will be the problem here.

Did you use SkyTube? Gestures in it can show description, comments, change brightness, volume, and playback position. So I can make only small area on the bottom of the screen that will react on swipe from bottom to top triggering scroll to bottom.
BUT I have a better workaround and it's more unexpected. Since we have a down arrow at the right of the screen I will bind it to a long click that will trigger the scroll to bottom a little bit without stopping a playback. Should test this method first since it requires a couple of lines of code :)

@bew

For the review, maybe try to first make a PR with only the unification of the players

Agree

then the next PRs would build on that and only show revelant changes?

Yes

@DI555
Copy link

DI555 commented Dec 28, 2019

@avently
Great ui! But , please, make your own NP fork, the same way how NP-Legacy one was organized by creating own subrepo here in the main repo!!!
Many of us are very used to use a usual ui of NP, but yours YT edition is very nice either!!! So an own fork here would be the great way!!!

and, please, take a look on my some feature suggestions:

• could you please set min sdk to 19 (android 4.4) like support by NP now, or if not possible - at least to min sdk 21 (android 5.0)
(there are many devices of it yet)

• i would like to see in your ui - TABS (Layers) !!! ... just to have possibility to use a lots of them(pages) ;) ! i hope this one will be in great demand!

@avently
Copy link
Contributor Author

avently commented Dec 28, 2019

@DI555

make your own NP fork

I don't have time on supporting a new fork. So if you want it you will be able to fork my repo when I upload it for a PR. But since the main idea of making the PR is to get it merged upstream there will be no need in the fork:)

could you please set min sdk to 19

The min sdk version is 19, I didn't change it

i would like to see in your ui - TABS

What do you mean? Can you show some examples from other apps?

@avently avently closed this as completed Dec 28, 2019
@avently avently reopened this Dec 28, 2019
@DI555
Copy link

DI555 commented Dec 28, 2019

• ok, then i hope the new YT-UI will be like a switchable option!
is it possible to announce ?

• about the tabs... i mean like in browser-like stile, you know... exactly the way like in opera browser(for android) it is.
(but if yt-ui will be i'plemented like an option, may be better to use tabs globally with any ui stile...)

• about min sdk..., i'm tryed to install the debug apk... and got error (((

@avently
Copy link
Contributor Author

avently commented Dec 28, 2019

is it possible to announce

It's possible to annonce but impossible to make it switchable:) No switchable option will exist.

i'm tryed to install the debug apk... and got error

That's probably because you are trying to install an apk over you existing apk. For some reason my debug key is not the same as yours. Try to sign the apk with you debug key and everything will be fine.
(I don't know about your skills but if you can't sign it with your debug key I will make a PR soon. Hope in this year:))

@DI555
Copy link

DI555 commented Dec 29, 2019

ok, if it's a chance to make things better i'll try to suggest many ways to feel comfortable like with the old one...

so, please, i mean tabs in upper line like this..
https://androfon.ru/wp-content/uploads/2016/01/httpsandroid-smartfon.rusitesdefaultfilesopera1.png

this gives a possibility use several 'pages' and quickly switch between them!!!

@gkeegan
Copy link
Contributor

gkeegan commented Dec 29, 2019

I don't think adding tabs to switch between pages is of much benefit to anyone besides a minority of users.

@DI555
Copy link

DI555 commented Dec 29, 2019

@gkeegan
oh, it'll help to view a big amount of media, to do this (partly) now you'd need to make a several copies installed on your device ;) !!!

but, anyways i hope that tabs line could be optional to switch on/off,
so you could be using 'one page view' also.

@gkeegan
Copy link
Contributor

gkeegan commented Dec 29, 2019

To view large amounts of media you can just make a playlist. I'm confused as to what use case you want this for.

@avently
Copy link
Contributor Author

avently commented Dec 29, 2019

I uploaded my work here https://github.com/avently/NewPipe/tree/unifiedplayer
Today I made many code changes so you can meet bugs.
It's not a final version but a kind of it. Maybe today or tomorrow I'll create a PR with bugfixes if we find any bugs. But overall it's what I wanted to get when I started.
This commit doesn't have a drop of useless code and files - I'll made it when review team will be ready to merge the PR.

P.S. 3,991 additions and 488 deletions -- not so bad for review.
P.S.2. There is a hidden usefull thing: in fullscreen mode you can Long click on arrow down (from right top corner) and the view will scroll right to the description without stopping a playback.

@avently
Copy link
Contributor Author

avently commented Dec 29, 2019

Made PR #2907. I fixed three little bugs since yesterday and if you have built the code force-pull new changes again.

@TobiGr
Copy link
Contributor

TobiGr commented Dec 30, 2019

@DI555 If you want tabs, please open a separate ticket for that. But to be honest, I don't see the point how they can be useful in NewPipe at the moment.

Did you use SkyTube? Gestures in it can show description, comments, change brightness, volume, and playback position. So I can make only small area on the bottom of the screen that will react on swipe from bottom to top triggering scroll to bottom.

@avently Yes and I faced problems because my screen is small and my swipes were recognised as a different gestures frequently.

BUT I have a better workaround and it's more unexpected. Since we have a down arrow at the right of the screen I will bind it to a long click that will trigger the scroll to bottom a little bit without stopping a playback. Should test this method first since it requires a couple of lines of code :)

This causes some lags on my phone, but it sounds like a good idea. The only con is that two actions are required, One tapping to make the button visible and another one to open the comments by long pressing it. However, I guess not so many people will use this feature. That's why two actions are okay for me 👍


I'd suggest to give further feedback in the open PR #2907 to allow a more straight conversation using threats

@mauriciocolli
Copy link
Contributor

Thanks for coming back to this.

make the whole video fragment hidable via gesture like Android version of Youtube

Very useful feature, really like what you did with the style of the minimized player.

Just to throw in here, some time ago I was testing the new MotionLayout (part of the coordinator layout library) and as a fun little experiment come up with this as well, was finding some time to polish it up and introduce to the app. Sill in the prototype stage though (for those curious about it: newpipe.prototype.apk.zip.)

[...] so you will be able to switch between players instantly

Yeah, sharing the instance IMO is better than the current approach. At the moment I can't think of a use case where someone would open multiple players at the same time (if someone can think of some valid case, feel free to tell it here).

I rather have a seamless transition between the interfaces: the player is always in a service which just binds to the interface that the user chose.

@avently
Copy link
Contributor Author

avently commented Dec 31, 2019

@mauriciocolli Oh, glad to see you here!
I tested your prototype and transitions looks amazingly smooth when video plays! You didn't even use BottomSheetBehavior. I must learn more about MotionLayout after seeing this.
Good to know that you thought about new UI for the player as well.

@Isayso
Copy link

Isayso commented Jan 5, 2020

Now under collapsible menu you can find two additional buttons: screen rotation and play with Kodi.
I'll remove first one since there is no point of using this feature with expandable player. But what to do with Play with Kodi? Where to place open in browser and share?
I can safety hide "Fit" to screen button in embed player but to leave it in popup player (all players use one xml file for UI).

I'm probably a 1% user of NewPipe, but "play with Kodi" is one of my most used buttons of NP, because all my TVs are equipped with Kodi devices. I would even like to have the Kodi play option one level up in the video lists and my ideal version, a direct connection to Kodi.
Thanks!

@bleedingcrow
Copy link

bleedingcrow commented Jan 24, 2020

This does feel awkward in my mouth but.. I prefer the np status quo

@vkhomenk
Copy link

MP4_20200212_120047.zip
Ui bug of apk from this thread

@avently
Copy link
Contributor Author

avently commented Feb 12, 2020

@vkhomenk
Thanks, man. You motivated me to make another try and to finally make scrolling awesome. I tried once some weeks ago but failed. Today I found a way. So, I uploaded an apk #2907 (comment) and a commit with changes.

@PeterHindes
Copy link
Contributor

One thing to keep in mind is when transitioning from the video player to the background player, video needs to stop being loaded to save bandwidth, since the user is only hearing audio.

@avently
Copy link
Contributor Author

avently commented Mar 2, 2020

video needs to stop being loaded to save bandwidth, since the user is only hearing audio

This is how I made it already

@PeterHindes
Copy link
Contributor

PeterHindes commented Mar 4, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This needs to be discussed before anything is done feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.