LBRYCommunity/lbry-desktop

Fork 0

Video: Mobile UI + overlay for keyboard shortcut feedback #5119

Merged

infinite-persistence merged 19 commits from ip-vjs-overlay into master

2020-12-14 17:40:59 +01:00

infinite-persistence commented

2020-12-01 15:09:41 +01:00

(Migrated from github.com)

New

Mobile version of the video play. Includes:
- Centered and larger play/pause button
- Tap on left or right area of screen to seek +/- 10s
Overlay for keyboard shortcut feedback:
- Playback-rate changes
- +/-10s seeking

Approach

ui/component/videoViewer/internal/plugin will contain the plugin files. Changes done to these files will be minimal so ease any future patching from the original source. The inline method is used to handle importing issues with Node modules.

Note to reviewer

In the last commit, I disabled the mobile UI for IOS and only enabled it for Android. I believe the native UI was intentionally used before this, so I tried to retain the same behavior. It's also because I don't have an iOS to actually confirm that it works, although the emulator in the browser seems to show that it does.
For the overlays, I have not tried on a Mac/Safari/iOS -- again, the native player is used there, but should try it just in case something goes bad.

Also

Closes #4889: Alt-Left shouldn't impact video

## New - Mobile version of the video play. Includes: - Centered and larger `play/pause` button - Tap on left or right area of screen to seek +/- 10s ![image](https://user-images.githubusercontent.com/64950861/101360533-ba065280-38d8-11eb-877d-1e275971b347.png) - Overlay for keyboard shortcut feedback: - Playback-rate changes ![image](https://user-images.githubusercontent.com/64950861/101360425-96430c80-38d8-11eb-8c76-da5df394a500.png) - +/-10s seeking ## Approach - `ui/component/videoViewer/internal/plugin` will contain the plugin files. Changes done to these files will be minimal so ease any future patching from the original source. The inline method is used to handle importing issues with Node modules. ## Note to reviewer - [ ] In the last commit, I disabled the mobile UI for IOS and only enabled it for Android. I believe the native UI was intentionally used before this, so I tried to retain the same behavior. It's also because I don't have an iOS to actually confirm that it works, although the emulator in the browser seems to show that it does. - [ ] For the overlays, I have not tried on a Mac/Safari/iOS -- again, the native player is used there, but should try it just in case something goes bad. ## Also Closes #4889: Alt-Left shouldn't impact video

neb-b (Migrated from github.com) reviewed 2020-12-01 15:09:41 +01:00

infinite-persistence commented

2020-12-03 07:06:29 +01:00

(Migrated from github.com)

Update

Re-factored the method to add "one-off" overlays
Added overlay for "seeked forward/backwards".

`Update` - Re-factored the method to add "one-off" overlays - Added overlay for "seeked forward/backwards".

neb-b commented

2020-12-03 18:47:14 +01:00

(Migrated from github.com)

Will look at this after the desktop release is out. Hopefully soon 😞

infinite-persistence commented

2020-12-07 15:17:08 +01:00

(Migrated from github.com)

Update

Added mobile UI and updated the main description of this PR to reflect the sum of changes.

`Update` - Added mobile UI and updated the main description of this PR to reflect the sum of changes.

neb-b (Migrated from github.com) requested changes 2020-12-09 20:41:40 +01:00

neb-b (Migrated from github.com) left a comment

Minor comments. Looks great

ui/component/viewers/videoViewer/internal/plugins/videojs-overlay/LICENSE

neb-b (Migrated from github.com) commented

2020-12-09 20:37:04 +01:00

This one doesn't need to be included.

ui/component/viewers/videoViewer/internal/videojs.jsx

neb-b (Migrated from github.com) commented

2020-12-09 20:41:11 +01:00

I think it's still helpful to bring up the control bar when someone skips forward

infinite-persistence (Migrated from github.com) reviewed 2020-12-09 21:02:01 +01:00

ui/component/viewers/videoViewer/internal/plugins/videojs-overlay/LICENSE

infinite-persistence (Migrated from github.com) commented

2020-12-09 21:02:01 +01:00

Will that be the one line, or the entire file?

infinite-persistence commented

2020-12-10 04:53:08 +01:00

(Migrated from github.com)

Update

Rebased to latest master.
Remove the LICENSE file for both plugins.
Bring up the control-bar when "seek" and "rate" is changed via keyboard.
Updated CHANGELOG to place the item above 0.49.0
Combine #5136 Don't seek video on 'alt+left/right' since they are related and there'll be conflicts if the PRs were separated.

`Update` - Rebased to latest master. - Remove the LICENSE file for both plugins. - Bring up the control-bar when "seek" and "rate" is changed via keyboard. - Updated CHANGELOG to place the item above 0.49.0 - Combine #5136 [Don't seek video on 'alt+left/right'](https://github.com/lbryio/lbry-desktop/pull/5136) since they are related and there'll be conflicts if the PRs were separated.

neb-b commented

2020-12-11 16:20:02 +01:00

(Migrated from github.com)

I think we still need one of the licenses (don't remember which one). It said it had to be included with the code I think.

infinite-persistence commented

2020-12-13 16:04:26 +01:00

(Migrated from github.com)

Update