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

Clicking on title should open video details #3808

Closed
wants to merge 3 commits into from

Conversation

budde25
Copy link
Contributor

@budde25 budde25 commented Jun 23, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Clicking the video title on the Main Player and Background Player opens the details page for that video.

Fixes the following issue(s)

Testing apk

app-debug.zip

Agreement

Copy link
Member

@B0pol B0pol left a comment

Choose a reason for hiding this comment

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

It doesn't work on Android TV, I believe it's because you have removed android:focusable=true

And please next time compress the apk with a zip file, don't just rename the file extension. 1) compression means less to download 2) makes more sense

@budde25
Copy link
Contributor Author

budde25 commented Jun 27, 2020

@B0pol

It doesn't work on Android TV, I believe it's because you have removed android:focusable=true

I added focus back in for the children so that it should be selectable from from a device without a touchscreen. I also realized I forgot to make the changes to the large-land.xml which might be the reason too. I don't have an android TV to test on but plugging a keyboard into my phone did work. I've updated the testing apk.

And please next time compress the apk with a zip file, don't just rename the file extension. 1) compression means less to download 2) makes more sense

Thanks for the tip! For some reason I was under the impression that .apk files where already compressed. I will make sure to do that going forward!

@Stypox
Copy link
Member

Stypox commented Jun 27, 2020

Another tip: use closing keywords when pointing out a fixed issue, so that GitHub closes it automatically on merge. I edited the description.

@hunkjazz
Copy link

hunkjazz commented Jul 8, 2020

This is a good PR, but needs enhancement on:

  • main player;
  • popup player.

Main Player

When clicking on title, while using the main player, this happens:

  1. main player closes;
  2. details page is shown.

I don't think this is quite a good UX.
Instead, this should be the flow:

  • switch to popup player;
    • if phone is in portrait:
      • fit popup's width to phone's;
      • move popup to the top of the screen.
  • details page is shown underneath popup;
  • user expands popup (back to main player);
  • NewPipe restores and saves popup width and position back what it was before user clicked to see details;
  • page that was active before replacement by details page is restored.

Popup Player

I think it's important to add a details icon under expand icon. In this
way popups opened by other apps can redirect users to further details,
for example.

This same icon can be hidden, if popup gets too small.

This icon should be hidden, when clicking to see details in
main player (if former enhancement suggestion is accepted).

@MD77MD
Copy link

MD77MD commented Jul 14, 2020

I think this should be looked at after the unified player feature

@MD77MD
Copy link

MD77MD commented Aug 14, 2020

this has suddenly became less useful after unified player or am I missing something?

@TobiGr
Copy link
Member

TobiGr commented Aug 14, 2020

@MD77MD Correct, turning the screen opens the details now.
@budde25 Thank you for your contribution!

@TobiGr TobiGr closed this Aug 14, 2020
@TobiGr
Copy link
Member

TobiGr commented Aug 14, 2020

Oh, my bad. This is still a good feature.
@budde25 Please rebase your PR.

@TobiGr TobiGr reopened this Aug 14, 2020
@TobiGr TobiGr added the player Issues related to any player (main, popup and background) label Aug 14, 2020
@MD77MD
Copy link

MD77MD commented Aug 18, 2020

@TobiGr sorry but I don't get it. when could you use this? the title does not show up in pop mode. in portrait mode, info is already there. that's the same for background mode. the only time you can click the title is in landscape mode, in which pressing the back button would give you the same thing.

However, a new implementation of this feature would be to add an info button in the pop-player. the button -when pressed- can take us back (reopen) the video info. now that's useful.

p.s: another implementation would be in notification. when pressing on the video title the video info would show up instead of play queue. which I would rather not but mentioned it for argument sake.

@opusforlife2
Copy link
Collaborator

The lad has a point. Even for landscape mode, there is now the ability to swipe up on the fullscreen button to view video details (thanks, avently!) . And if system rotation is unlocked, then the user can just turn the phone to see them.

@MD77MD
Copy link

MD77MD commented Aug 18, 2020

Even for landscape mode, there is now the ability to swipe up on the fullscreen button to view video details (thanks, avently!) 

@opusforlife2 please tell me how to enable this😰! ... are you referring to what happens when you press back button in landscape mode... or do you mean video info would show up on top of video like what happens when you press on playlists button, I wish you mean it's the second one 😅. that would just be awesome.

@justanidea
Copy link
Contributor

justanidea commented Aug 19, 2020

@MD77MD Watch the end of #4121
#4121 (comment)

@opusforlife2
Copy link
Collaborator

@justanidea You could always link directly to the relevant comment.

@MD77MD #4121 (comment)

@justanidea
Copy link
Contributor

Ok i didnt think my github client actually could, but it does
TY

@MD77MD
Copy link

MD77MD commented Aug 19, 2020

@justandidea @opusforlife2
thank you guys

@hunkjazz
Copy link

@MD77MD @opusforlife2

Would you happen to have a link to download a working apk with the unified player?

@opusforlife2
Copy link
Collaborator

@Stypox
Copy link
Member

Stypox commented Sep 3, 2020

Yeah, I don't see why this should be merged, now that we have unified ui @TobiGr

@budde25
Copy link
Contributor Author

budde25 commented Sep 3, 2020

@Stypox Yeah that makes sense to me

@TobiGr TobiGr force-pushed the dev branch 2 times, most recently from 679bc75 to 2aeccc0 Compare March 16, 2021 08:24
@triallax
Copy link
Contributor

@budde25 I'm closing this PR since it seems like it's not necessary anymore, but please make comment if I turn out to be wrong so I can re-open it. Thanks!

@triallax triallax closed this Jun 13, 2021
@budde25 budde25 deleted the click-open-details branch June 13, 2021 13:22
ShareASmile added a commit to ShareASmile/FoxPipe that referenced this pull request Jun 14, 2024
…s of 15-01-2024 into pre-unified-legacy

This is a fork of TeamNewPipe/NewPipe-legacy that I have been patching for a few months now. There is no commit history as this has been a personal project up until today.

Pulled in a major chunk of related commits for NewPipe Preunified from upstream TeamNewPipe/NewPipe repository:
1. Update NewPipe extractor to fetch likes count Fix TeamNewPipe/NewPipe#10624
2. Access Background Player Queue from Main menu by HybridAU
TeamNewPipe/NewPipe#8419
3. Added Bandcamp Music Support Into NewPipeLegacy, Improve Bandcamp intent filters
TeamNewPipe/NewPipe#3741
TeamNewPipe/NewPipe#6373
TeamNewPipe/NewPipe#6456
4. Disable sending metrics to Google when using Android System WebView
TeamNewPipe/NewPipe#5337
5. Add basic resize functionality
TeamNewPipe/NewPipe#3948
6. [media.ccc.de] Add recent & live stream kiosk
TeamNewPipe/NewPipe#5251
TeamNewPipe/NewPipe#5286
[media.ccc.de] Fix service color
TeamNewPipe/NewPipe#5258
7. Ability to see Pinned Comment added by dkramer95
TeamNewPipe/NewPipe#7577
8. Clicking on Title In Background Player Should Open Video Details
TeamNewPipe/NewPipe#3808 by https://github.com/budde25
9. Added Swipe to Refresh for Channels New Videos in Subscription Feed
TeamNewPipe/NewPipe#4893
10. [peertube] implement sepia search
TeamNewPipe/NewPipe#5257
11. Allow installation on external storage by triallax in
TeamNewPipe/NewPipe#6037
12. Change UA to privacy.resistFingerprinting
TeamNewPipe/NewPipe#5649 by FireMasterK
13. Add formatting removal on paste for search TeamNewPipe/NewPipe#5912 by imericxu
14. Remember Last Selected Media Type For Downloads
TeamNewPipe/NewPipe#4038 by vmazoyer
15. Improve search suggestion experience when remote ones can't be fetched
TeamNewPipe/NewPipe#4029 by StyPox
16. Fix Download Button Not Visible After Playing Live Stream, Fix video detail controls visibility set inconsistently
Add this commit by StyPox
TeamNewPipe/NewPipe@dbb86d2 from
TeamNewPipe/NewPipe#4362
17. [Background Player] Fix very small thumbnails in Video Detail Page by TobiGR in
TeamNewPipe/NewPipe#5818
18. Add Always Expand Description In Appearance Settings
TeamNewPipe/NewPipe#2998 by B0pol
(Related) entries from search history Increased from 25 to 80 and Search Suggestions Entries from 3 to 60 commit copied from
TeamNewPipe/NewPipe#2666 thanks to ergor
20. Fixes snackbar error on disabled likes count
Fixes TeamNewPipe/NewPipe#7405 by TeamNewPipe/NewPipeExtractor#753
21. Fixed player controls not hiding after replay & Bluetooth headset button by Alexander--
TeamNewPipe/NewPipe#3547
22. update user agent in Downloader to firefox latest ESR 115 by Nickoriginal
TeamNewPipe/NewPipe#8269
23. Changed Dark Theme Colors To Darker Variant by sauravrao637
TeamNewPipe/NewPipe#6244
24. Support for PeerTube Short Links
TeamNewPipe/NewPipe#7353
25. Handle URLs for YouTube Shorts
TeamNewPipe/NewPipe#7181
26. Fix playback speed not being updated in Background Player & Fix TeamNewPipe/NewPipe#8058
Fixed Combinedly by TeamNewPipe/NewPipe#6421 by Tobius
and by seanzzy in
TeamNewPipe/NewPipe#8244
27. Added comments disabled functionallity by litetex in
TeamNewPipe/NewPipe#6483
28. [media.ccc.de] Fix service color
TeamNewPipe/NewPipe#5258
29. Ignore ContentNotSupportedException caused by Bandcamp fan pages
TeamNewPipe/NewPipeExtractor#1033
30. Crash when rotating device on unsupported channels
TeamNewPipe/NewPipe#6696
31. Correct Gigaget's license from GPLv2 to GPLv3
TeamNewPipe/NewPipe#4892
32. Add basic resize functionality [Samsung Dex Now Supported]
TeamNewPipe/NewPipe#3948
33. [Security] Update ktlint to 0.40.0
34. Fix security vulnerability update checkstyle / guava
35. Update ExoPlayer from 2.11.6 to 2.11.8
36. Update checkstyle, OkHttp, use Kotlin JDK8 by TacoTheDank added this commit
TeamNewPipe/NewPipe@79e2bb3
from TeamNewPipe/NewPipe#3909
37. Use user agent of DownloaderImpl also in ReCapthaActivity
TeamNewPipe/NewPipe#5215
38. Fix ACRA bug reports not containing stack trace, Do not init ACRA if inside its own process
TeamNewPipe/NewPipe#3982
39. Remove deprecated calls to set Sender class to ACRA
TeamNewPipe/NewPipe#3982
40.Use SubtitlesStream#getUrl instead of getURL
TeamNewPipe/NewPipe#4120
41. Remove pbj=1 parameter from YouYube urls in recaptcha activity
TeamNewPipe/NewPipe#5208
42. Set notification style in Android 11 to MediaStyle Thanks to XiangRongLin for Limted Support for preunified refer to this commit
XiangRongLin@aa55a09
43. Click on title in background player opens video details
TeamNewPipe/NewPipe#3808
44. Add 2K and 4K to the options list for default resolution
 TeamNewPipe/NewPipe#2968
45. Fix crash when opening video in local playlist tab
Fixes TeamNewPipe/NewPipe#3887
by TeamNewPipe/NewPipe#3892 by wb9688
46.  Handle ContentNotSupportedException
TeamNewPipe/NewPipe#3300
47. [YouTube] Improve download speed by Theta-dev
TeamNewPipe/NewPipe#9948
48. Disable commenter image when disabling thumbnails loading by 4D17Y4
Fixes TeamNewPipe/NewPipe#4205
TeamNewPipe/NewPipe#4350
49. Adapt opacity of popup close button to allow touches in other apps on Android >=11
Fixes TeamNewPipe/NewPipe#6770
TeamNewPipe/NewPipe#8279
50. Better error messages for SoundCloud and YouTube unavailable contents
TeamNewPipe/NewPipe#5385
51. Mitigating long buffering on initial video playback by using custom progress-load-interval in exoplayer,
Use 16 KiB as the default progressive load interval by karyogamy
TeamNewPipe/NewPipe#7919
52. Support for opening YouTube Live URLs
TeamNewPipe/NewPipe#9725

Co-Authored-By: Tobi <17365767+tobigr@users.noreply.github.com>
Co-Authored-By: Stypox <stypox@pm.me>
Co-Authored-By: Audric V. <74829229+audricv@users.noreply.github.com>
Co-Authored-By: bopol <58657617+b0pol@users.noreply.github.com>
Co-Authored-By: Isira Seneviratne <31027858+isira-seneviratne@users.noreply.github.com>
Co-Authored-By: opusforlife2 <53176348+opusforlife2@users.noreply.github.com>
Co-Authored-By: fynngodau <fynngodau@mailbox.org>
Co-Authored-By: David Kramer <6166095+dkramer95@users.noreply.github.com>
Co-Authored-By: Ethan Budd <budde25@protonmail.com>
Co-Authored-By: triallax <triallax@tutanota.com>
Co-Authored-By: Kavin <20838718+firemasterk@users.noreply.github.com>
Co-Authored-By: Eric Xu <xeric.2002@gmail.com>
Co-Authored-By: Vincent Mazoyer <17800856+vmazoyer@users.noreply.github.com>
Co-Authored-By: Erol Gorancic <erol@gorancic.no>
Co-Authored-By: Alexander-- <1107390+alexander--@users.noreply.github.com>
Co-Authored-By: Saurav Rao <56369484+sauravrao637@users.noreply.github.com>
Co-Authored-By: Nickoriginal <85299944+nickoriginal@users.noreply.github.com>
Co-Authored-By: Ziyan Zhang <71145592+seanzzy@users.noreply.github.com>
Co-Authored-By: litetex <40789489+litetex@users.noreply.github.com>
Co-Authored-By: XiangRongLin <41164160+xiangronglin@users.noreply.github.com>
Co-Authored-By: wb9688 <46277131+wb9688@users.noreply.github.com>
Co-Authored-By: Okan25 <92695587+okan25@users.noreply.github.com>
Co-Authored-By: Michael Van Delft <1610265+hybridau@users.noreply.github.com>
Co-Authored-By: Taco <32376686+tacothedank@users.noreply.github.com>
Co-Authored-By: ThetaDev <thetadev@magenta.de>
Co-Authored-By: Aditya-Srivastav <54016427+4d17y4@users.noreply.github.com>
Co-Authored-By: John Zhen Mo <zhenmogukl@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
player Issues related to any player (main, popup and background)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Click on title in background player should open video details
9 participants