Make long-press menu consistent across views #2368

Stypox · 2019-05-29T19:21:36Z

[v] I carefully read the contribution guidelines and agree to them.

The order of the items in the stream long-press menu should be the same in every view (playlist, search, info...), so that the user can subconsciously memorize it, without having to read the same info over and over again. Also, I think consistency is a good thing. See #2354 for more info.

I fixed the menu so that it is the same across the app, and I chose this order:

enqueue-background
enqueue-popup
new-background
new-popup
[  ...  specific actions, such as delete-from-history]
add-to-playlist
share

The subscription long-press menu (in the local subscription list) was also modified to comply with the above order: it is now unsubscribe; share; instead of share; unsubcribe;.

I decided to remove the new-on-main-player option since it was mostly useless. The other play options are useful to (for example) enqueue while carrying on browsing flawlessly. But opening the main player interrupts what the user was doing anyway. And, from a time point of view, one could open the stream details and open the main player in roughly the same time it would take to long-press and then click.

Before merging I'd like to improve the menu labels: those were inconsistent, too. The new-background option had two different strings used in different views: "Play directly in background" (direct_on_background) and "Start playing in the background" (start_here_on_background). So the direct_on_background string should be removed, along with start_here_on_main which is no longer used (as explained above).
Soundcloud has no video playback, but the entry to play on popup appears in the menu anyway. This has to be fixed.
Remove the ugly if-else-cascade
Fix crash when closing a starting/loading popup.

Testing APK: app-debug.zip

Fixes #1142
Closes #2354

Also made the code that creates the menus consistent across files.

VideoDetailFragment already borrows a consistant menu from the stream list it holds.

They were exactly the same as the base class function

Inverted unsubscribe with share, since share has always been put after content-specific actions.

start_here_on_background has the same meaning start_here_on_main is now unused, but I left it there so that if it ever becomes useful again, it is ready to be used.

Stypox · 2019-05-30T13:33:54Z

I addressed the duplicate direct_on_background string in b6cfb8a, but I decided to leave the unused start_here_on_main there, so that if it ever becomes useful again it's ready to be used.

…alls. `resumePlayback`'s value is `false` when the video is enqueued, `true` otherwise. Also make the use of getContext() and getActivity() more consistant.

Stypox · 2019-07-22T10:01:16Z

There are no popup entries anymore when the long-pressed stream is audio-only. I had to replace the switch block with the ugly if - else if, but I don't think there was anything that could be done.

Redirion · 2019-07-23T10:29:29Z

There are no popup entries anymore when the long-pressed stream is audio-only. I had to replace the switch block with the ugly if - else if, but I don't think there was anything that could be done.

you could work with enums: https://schneide.blog/2010/12/13/avoid-switch-use-enum/

Common actions and labels are now in a unique enum: StreamDialogEntry If an action is not common to every long-press menu (e.g. delete) a custom action has to be provided using e.g. delete.setAction(...)

…logEntry

Stypox · 2019-07-24T22:58:19Z

@Redirion fixed. Is it ok now? I did my best, and I really love Java enums! 😍

showStreamDialog
StreamDialogEntry

Btw 9df27f4 is just to increase safety for any potential future user/reader of StreamDialogEntry code. If you think I should remove it, tell me :-)

Stypox · 2019-07-24T23:01:05Z

Also, I found a bug: the app crashes when the popup player is closed while starting. I don't know if this problem is caused by my code.

To improve readability

Redirion · 2019-07-25T07:07:13Z

@Redirion fixed. Is it ok now? I did my best, and I really love Java enums! heart_eyes

It looks beautifully!

You have some unused imports in BaseListFragment that could be cleaned up.

I will test the code later (in about 8 hours).

Stypox · 2019-07-25T09:55:52Z

@Redirion I added a test APK to the description

Stypox · 2019-07-25T10:41:02Z

Ok, fixed the crash. The problem was caused by the recent resume playback pr #2288: the popup's BasePlayer tried to re-init when closing, since the loaded-listener was called both on video loading completion and when a not-yet-loaded popup is closed.

@nv95 can you please confirm that what I did in 7c9ef58 was ok?

justanidea · 2019-07-25T11:12:45Z

I tried the Apk and all seem to be good for me

Koitharu · 2019-07-25T11:27:35Z

@Stypox I'll look on this on weekends

Koitharu · 2019-07-30T06:14:31Z

@Stypox I was try it and looks like everything is ok

Redirion · 2019-07-31T08:55:44Z

I have used it on daily basis since you provided the apk and only had two unrelated crashes. This should be fine for merge.

TobiGr

The code is a beauty ❤️

kurobon-jp · 2019-08-03T08:56:38Z

I don’t understand why Start playing here and Start playing in the background deleted.
Did you decide that you don't need the action that plays continuously from the middle of the list?

Stypox · 2019-08-03T09:40:40Z

I did not delete either of them. They are still there and are being used (see the apk in release 0.17.0). I only deleted direct_on_background since it was a duplicate of start_here_on_background.

kurobon-jp · 2019-08-03T13:47:48Z

In my environment, after merging the Stypox:menu-consistency, the Start playing here are gone.

e7b068e

4e5a20e

Start playing in the background items remain, but it seems to be different from the previous behavior.

e7b068e

4e5a20e

Stypox · 2019-08-04T06:16:49Z

Yes, I removed "start playing here" because it seemed mostly useless. See above.

About the differencies about arrows... I don't know why that happens, will look into it in a week (after vacation)

Stypox added 6 commits May 29, 2019 16:22

Make long-press menu consistent across views: fix TeamNewPipe#2354

8b584f3

Also made the code that creates the menus consistent across files.

Removed unused showStreamDialog from VideoDetailFragment

30eeef4

VideoDetailFragment already borrows a consistant menu from the stream list it holds.

Remove useless overrides of showStreamDialog

bdb1be9

They were exactly the same as the base class function

Make subscription long-press menu consistant in local sub list

8695466

Inverted unsubscribe with share, since share has always been put after content-specific actions.

Remove unused imports from modified files

6f028ec

Remove dupliacte direct_on_background string

b6cfb8a

start_here_on_background has the same meaning start_here_on_main is now unused, but I left it there so that if it ever becomes useful again, it is ready to be used.

Stypox mentioned this pull request May 30, 2019

Directplay on Background #2198

Merged

1 task

Stypox mentioned this pull request Jun 10, 2019

Share with "youtu.be" links #2387

Closed

Merge branch 'dev' into menu-consistency

f0ba6af

theScrabi added the feature request Issue is related to a feature in the app label Jun 26, 2019

Stypox changed the title ~~Make long-press menu consistent across views~~ Make long-press menu consistent across views Jul 18, 2019

Stypox mentioned this pull request Jul 19, 2019

Add New Menu To Add to Playlist for suggested videos #1251

Closed

Stypox added 2 commits July 21, 2019 11:11

Merge branch 'dev' into menu-consistency

064f0e4

Merge branch 'dev' into menu-consistency

ef070a4

Stypox mentioned this pull request Jul 22, 2019

Playback positions in lists #2288

Merged

1 task

Stypox added 2 commits July 22, 2019 10:28

Complete merge after TeamNewPipe#2288: add resumePlayback to player c…

bb50283

…alls. `resumePlayback`'s value is `false` when the video is enqueued, `true` otherwise. Also make the use of getContext() and getActivity() more consistant.

Fix showing popup options with audio-only streams

8edc332

Stypox added 3 commits July 24, 2019 17:21

Merge branch 'dev' into menu-consistency

3aeba7c

Remove ugly if-else-cascade in

759e984

Common actions and labels are now in a unique enum: StreamDialogEntry If an action is not common to every long-press menu (e.g. delete) a custom action has to be provided using e.g. delete.setAction(...)

Ensure default actions cannot be overwritten permanently in StreamDia…

9df27f4

…logEntry

Rename action to defaultAction in StreamDialogEntry

25fbbfa

To improve readability

Optimize imports in edited files

d076fe7

Fix crash when closing a not-yet-loaded popup.

7c9ef58

TobiGr approved these changes Jul 31, 2019

View reviewed changes

TobiGr merged commit 4e5a20e into TeamNewPipe:dev Jul 31, 2019

This was referenced Jul 31, 2019

Release v0.17.0 #2482

Merged

Release v0.17.0 #2483

Closed

Stypox mentioned this pull request Aug 14, 2019

Customize "start here" actions in playlist item views #2518

Merged

1 task

Stypox mentioned this pull request Aug 22, 2019

Unable to play YouTube playlist from specified track #2510

Closed

Stypox mentioned this pull request Aug 31, 2019

Fast actions from mini info item - UI / UX ideas #2583

Closed

Stypox deleted the menu-consistency branch September 12, 2019 13:34

Stypox mentioned this pull request Jan 8, 2020

Projects to keep track of similar issues #2937

Closed

BartFarley mentioned this pull request Nov 16, 2020

Start playing from here/ Enqueue from here #4805

Closed

3 tasks

Lurux mentioned this pull request Nov 26, 2020

[Idea] Context menu reworking with icons for cleaner experience #5011

Closed

3 tasks

ShareASmile mentioned this pull request Sep 12, 2021

Added a play next button to the long press menu #6872

Merged

5 tasks

Uh oh!

Make long-press menu consistent across views #2368

Make long-press menu consistent across views #2368

Uh oh!

Conversation

Stypox commented May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stypox commented May 30, 2019

Uh oh!

Stypox commented Jul 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Redirion commented Jul 23, 2019

Uh oh!

Stypox commented Jul 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stypox commented Jul 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Redirion commented Jul 25, 2019

Uh oh!

Stypox commented Jul 25, 2019

Uh oh!

Stypox commented Jul 25, 2019

Uh oh!

justanidea commented Jul 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Koitharu commented Jul 25, 2019

Uh oh!

Koitharu commented Jul 30, 2019

Uh oh!

Redirion commented Jul 31, 2019

Uh oh!

TobiGr left a comment

Choose a reason for hiding this comment

Uh oh!

kurobon-jp commented Aug 3, 2019

Uh oh!

Stypox commented Aug 3, 2019

Uh oh!

kurobon-jp commented Aug 3, 2019

Uh oh!

Stypox commented Aug 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Stypox commented May 29, 2019 •

edited

Loading

Stypox commented Jul 22, 2019 •

edited

Loading

Stypox commented Jul 24, 2019 •

edited

Loading

Stypox commented Jul 24, 2019 •

edited

Loading

justanidea commented Jul 25, 2019 •

edited

Loading