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.

Sign up for GitHub

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

Jump to bottom

Add basic playback info dialog #25

Merged
merged 4 commits into from
Aug 20, 2020
Merged

Add basic playback info dialog #25

merged 4 commits into from
Aug 20, 2020

Conversation

Maxr1998
Copy link
Member

Closes #22.

@Maxr1998 Maxr1998 added enhancement New feature or request exoplayer Related to the ExoPlayer integration labels Aug 19, 2020
nielsvanvelzen
nielsvanvelzen reviewed Aug 19, 2020
View reviewed changes
Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

Code looks good but I have one comment on the strings.

next time screenshots of UI changes are appreciated

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
@Maxr1998
Copy link
Member Author

Here's a screenshot as well:
Screenshot

nielsvanvelzen
nielsvanvelzen requested changes Aug 20, 2020
View reviewed changes
Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

I think it would be better to move the text view to a corner (top left or top right) and remove some padding. Right now it's pretty much impossible to watch something with the playback info enabled.

The web player or other services (like YouTube) use the same approach.

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
@nielsvanvelzen nielsvanvelzen added this to In progress in v2.0.0-rc.2 via automation Aug 20, 2020
@Maxr1998
Copy link
Member Author

I think it would be better to move the text view to a corner (top left or top right) and remove some padding. Right now it's pretty much impossible to watch something with the playback info enabled.

The web player or other services (like YouTube) use the same approach.

The playback info is part of the playback interface though, and gets automatically hidden once you tap anywhere outside of it. I can make it that it can be shown independently from the playback interface, but I'm not sure whether we want that.

@Maxr1998
Add basic playback info dialog
1539f51
@Maxr1998
Split up playback info into multiple strings for easier extensibility
9044203
@Maxr1998
Move colon and newline from strings to code
2cb4baf
@Maxr1998
Separate playback info overlay from player controls
3f76675 
That makes it possible to always see the playback info while playing media.
Also move them to the top left corner, to not block the video itself so much.
@Maxr1998
Copy link
Member Author

Maxr1998 commented Aug 20, 2020 •
edited

This is how it looks now:
Player controls visible
Player controls hidden

You can close the playback info by tapping the info button again or tapping the info dialog itself.

@nielsvanvelzen nielsvanvelzen merged commit 1f13b47 into master Aug 20, 2020
v2.0.0-rc.2 automation moved this from In progress to Done Aug 20, 2020
@nielsvanvelzen nielsvanvelzen deleted the exoplayer-playback-info branch August 20, 2020 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nielsvanvelzen nielsvanvelzen nielsvanvelzen approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request exoplayer Related to the ExoPlayer integration
Projects
No open projects
v2.0.0-rc.2
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add dialog with playback/transcoding info
2 participants
@Maxr1998 @nielsvanvelzen