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

Use user setting for seeking on Android TV #342

Merged
merged 6 commits into from
Feb 8, 2023

Conversation

TerHano
Copy link
Contributor

@TerHano TerHano commented Jan 28, 2023

No description provided.

@LagradOst
Copy link
Contributor

@doteq

@bachig26
Copy link
Contributor

@TerHano I don't understand much (on coding), but looking at the changes. it looks same.
It looks like both left and right key executes the forward seek.

@TerHano
Copy link
Contributor Author

TerHano commented Jan 28, 2023

@TerHano I don't understand much (on coding), but looking at the changes. it looks same.
It looks like both left and right key executes the forward seek.

Sorry about that, I had done some edits prior to checking in and it looks like the minus was deleted. Should be fixed now.

@doteq
Copy link
Contributor

doteq commented Jan 29, 2023

When I was designing TV UI I wanted to have two ways of seeking.

  • If you couldn't hear something you can just rewind 10 seconds by clicking left button without interface opened
  • If you want to find some part in video you can seek by 30 seconds when interface is opened

I feel that my design is very intuitive because it gives you more control when seeking. You could add second "TV seeking time" setting, so users can tweak that and use it here, but using only one value is a bad idea.

@TerHano
Copy link
Contributor Author

TerHano commented Feb 1, 2023

I agree that having two settings would be nice. I made the change and added the settings, let me know if it's what you had in mind.

@doteq
Copy link
Contributor

doteq commented Feb 2, 2023

I would remove semicolons, because you don't need them in kotlin and it looks weird when only your lines have them, but overall looks good to me, great work :)

@LagradOst
Copy link
Contributor

LGTM, ready to merge?

@LagradOst LagradOst merged commit 6c646d6 into recloudstream:master Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants