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

Add play when locked setting #453

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Fjara-h
Copy link

@Fjara-h Fjara-h commented Oct 12, 2024

Play stream when locked setting Per Issue #450
Fixed a spelling error in Settings.java
Add missing default strings that was giving warnings on build

Fixed a spelling error in Settings.java
Play stream when locked setting
Add missing default strings (warnings)
Copy link
Collaborator

@samfundev samfundev left a comment

Choose a reason for hiding this comment

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

This looks like it would fix the issue! But you added some extra changes that I can't accept as is. If you fix those up, I can merge this in.

@@ -12,7 +12,7 @@ android {
minSdk 21
targetSdk 34
versionCode 534
versionName "2.11.0"
versionName "2.12.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't change the version number. I'll change it when I make the next release.

Suggested change
versionName "2.12.0"
versionName "2.11.0"

@@ -321,7 +324,7 @@

<!-- The first letter of each line of the changelog denotes if it's about a new Version, Addition, Fix or Change and should not be translated. -->
<string-array name="changelog_lines">
<item>V 2.11.0</item>
<item>V 2.12.0</item>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't change the version number. I'll change it when I make the next release.

Suggested change
<item>V 2.12.0</item>
<item>V 2.11.0</item>

Comment on lines +250 to +251
<string name="player_channel_name">Player</string>
<string name="player_channel_description">Twire Player</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

These strings were used in the past but are no longer needed. If you want to resolve the warning, you should remove these keys from other translation files.

Suggested change
<string name="player_channel_name">Player</string>
<string name="player_channel_description">Twire Player</string>

Comment on lines +742 to +746
public boolean getStreamPlayerAutoContinuePlayback() {
SharedPreferences preferences = getPreferences();
return preferences.getBoolean(this.STREAM_PLAYER_AUTO_PLAYBACK, false);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this added? It's not used.

Suggested change
public boolean getStreamPlayerAutoContinuePlayback() {
SharedPreferences preferences = getPreferences();
return preferences.getBoolean(this.STREAM_PLAYER_AUTO_PLAYBACK, false);
}

STREAM_PLAYER_AUTO_PLAYBACK = "streamPlayerAutoPlackbackOnReturn",
STREAM_PLAYER_AUTO_PLAYBACK = "streamPlayerAutoPlaybackOnReturn",
Copy link
Collaborator

Choose a reason for hiding this comment

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

These strings are used as keys to save user's settings. If you change the key without migrating user's who have the old key, the setting will be lost for users who changed it.

@Fjara-h
Copy link
Author

Fjara-h commented Dec 19, 2024

Cool! I will fix it up soon.

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.

2 participants