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

Make skip silence configurable #7185

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

MartinNowak
Copy link

fixes #4229

Description

Checklist

- followup feed-specific skip silence config (AntennaPod#6910)
- toggle buttons to configure
- medium 300ms gap
- aggressive 50ms gap
@ByteHamster ByteHamster changed the title fix #4229 - configurable skip silence Make skip silence configurable May 18, 2024
Copy link
Member

@ByteHamster ByteHamster left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! I added some comments below :)

OFF(0), GLOBAL(1), AGGRESSIVE(2);
GLOBAL(0), OFF(1), MEDIUM(2), AGGRESSIVE(3);
Copy link
Member

Choose a reason for hiding this comment

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

Please keep the IDs as they are. They don't have to be sorted but the meaning should stay the same. Otherwise this changes the existing settings for users when they upgrade.

FeedPreferences.SkipSilence opt;
final int id = group.getCheckedButtonId();
if (id == R.id.skipSilenceOff) opt = FeedPreferences.SkipSilence.OFF;
else if (id == R.id.skipSilenceMedium) opt = FeedPreferences.SkipSilence.MEDIUM;
Copy link
Member

Choose a reason for hiding this comment

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

Please use braces for all if/switch/etc statements. (I'm a bit surprised that the CI does not complain about this. Will have to check this)

style="@style/OutlinedButtonBetterContrast" />

<Button
android:id="@+id/skipSilenceMedium"
Copy link
Member

Choose a reason for hiding this comment

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

Please add the button suffix, similar for the other buttons. See also the code style guidelines: https://github.com/AntennaPod/AntennaPod/wiki/Code-style

Suggested change
android:id="@+id/skipSilenceMedium"
android:id="@+id/skipSilenceMediumButton"

android:layout_width="0dp"
android:layout_height="match_parent"
android:layout_weight="1"
android:text="Medium"
Copy link
Member

Choose a reason for hiding this comment

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

Please make this a @string/ resource, so it can be translated. Strings are stored in the ui/i18n module

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.

Tweak skip silence to make it less abrupt
2 participants