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

Hide 5, 15, 25 second seek options if inexact seek is enabled #3160

Merged
merged 9 commits into from
Mar 3, 2020

Conversation

XiangRongLin
Copy link
Collaborator

@XiangRongLin XiangRongLin commented Feb 29, 2020

closes #3127

Hides 5, 15, 25 second seek options if inexact seek is enabled. If one of those values was selected while activating inexact seek, it automatically gets set to 10 seconds.

Will the edited string resource make any problems concerning translation with weblate, since it was not a rewrite but adding additional info.

I tested it on my Android 10 and 7.1 devices.
Updated apk: app-debug.zip

@opusforlife2
Copy link
Collaborator

Works as expected. 👍

@Stypox
Copy link
Member

Stypox commented Mar 2, 2020

If one of those values was selected while activating inexact seek, it automatically gets set to 10 seconds.

I think it should be rounded up instead of always being set to 10:

  • 5 -> 10
  • 15 -> 20
  • 25 -> 30

Also, would it be possible to add a description to "Fast-forward/-rewind seek duration", too, so that the user knows why 5,15,25 seek durations are not available?
Another option would be to show a Toast to the user when the "Fast-forward/-rewind seek duration" setting was changed automatically after he activated "Use fast inexact seek".

@XiangRongLin

@XiangRongLin
Copy link
Collaborator Author

I think it should be rounded up instead of always being set to 10:

5 -> 10
15 -> 20
25 -> 30

Can be done.

Also, would it be possible to add a description to "Fast-forward/-rewind seek duration", too, so that the user knows why 5,15,25 seek durations are not available?

The description field is already used to display the selected value.

Another option would be to show a Toast to the user when the "Fast-forward/-rewind seek duration" setting was changed automatically after he activated "Use fast inexact seek".

I would do this then.

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Seems good, apart from an issue with strings. Thank you for implementing the suggestions :-D
Could you provide an updated debug apk, please?

@@ -593,6 +593,7 @@
<string name="app_language_title">App language</string>
<string name="systems_language">System default</string>
<string name="dynamic_seek_duration_description">%s seconds</string>
<string name="new_seek_duration_toast">Due to ExoPlayer contraints the seek duration was set to</string>
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this have a %s indicating where the "X seconds" string should be inserted? For English it is ok for this to be at the end, but for other languages this could be different.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, didn't think of other languages having a different arrangement

@XiangRongLin
Copy link
Collaborator Author

I'll update the apk tomorrow, I'm on mobile now.

@Stypox
Copy link
Member

Stypox commented Mar 2, 2020

No problem, thank you for the time you are putting into contributing ;-)

@XiangRongLin
Copy link
Collaborator Author

@Stypox I updated the apk and rebased the branch onto dev

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

This is ready, thank you! If you agree with the last change I suggested, just click on "Commit suggestion" directly from github/mobile and I will merge this.

@Stypox Stypox self-assigned this Mar 3, 2020
Stypox
Stypox previously approved these changes Mar 3, 2020
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Again, thank you :-D

@Stypox Stypox merged commit d5c29bf into TeamNewPipe:dev Mar 3, 2020
This was referenced Mar 25, 2020
@XiangRongLin XiangRongLin deleted the b3127 branch March 29, 2020 08:12
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.

[Bug] Seek duration of 5 seconds makes audio rewind impossible
3 participants