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

Improve strings (2.0) #42

Merged
merged 19 commits into from
Jan 17, 2024
Merged

Improve strings (2.0) #42

merged 19 commits into from
Jan 17, 2024

Conversation

KobeW50
Copy link
Collaborator

@KobeW50 KobeW50 commented Jan 14, 2024

Hopefully, these are the final changes to Reddit and YT Music. I'm doing the YT changes on a different branch so it will probably be in a separate PR.

Sorry about the commit history. I've no idea what I'm doing :)

@@ -20,18 +20,18 @@
<string name="revanced_change_start_page_entry_home">Home</string>
<string name="revanced_change_start_page_entry_library">Library</string>
<string name="revanced_change_start_page_entry_subscription">Subscriptions</string>
<string name="revanced_change_start_page_summary">Changes the start page of the app.</string>
<string name="revanced_change_start_page_summary">Select which page the app opens in.</string>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to make this summary match the patch description, but there is an issue.

The new patch description is Adds an option to set which page the app opens in instead of the homepage.

But "instead of the homepage" wouldn't make sense in the summary because one of the radio buttons the user can select is Homepage. Therefore this summary only partially matches the patch description.

<string name="revanced_ryd_failure_connection_timeout">Dislikes are temporarily unavailable (API timed out).</string>
<string name="revanced_ryd_failure_connection_status_code" formatted="false">Dislikes are unavailable (status %d).</string>
<string name="revanced_ryd_failure_client_rate_limit_requested">Dislikes are unavailable (client API limit reached).</string>
<string name="revanced_ryd_failure_generic">Dislikes are unavailable (%s).</string>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure if adding the word "are" to these toasts makes them too long. I know that toasts will be truncated if they can't fit on-screen which should be considered.

@KobeW50 KobeW50 changed the title Improve YT Music and Reddit strings (2.0) Improve strings (2.0) Jan 14, 2024
Copy link
Collaborator Author

@KobeW50 KobeW50 left a comment

Choose a reason for hiding this comment

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

Notes on YT changes

@KobeW50 KobeW50 marked this pull request as draft January 15, 2024 00:17
@inotia00
Copy link
Owner

inotia00 commented Jan 15, 2024

These are strings that will be added to the final build.
If possible, please also review the following strings:

    <string name="revanced_enable_default_playback_speed_shorts_summary_off">Default playback speed does not apply to shorts.</string>
    <string name="revanced_enable_default_playback_speed_shorts_summary_on">Default playback speed applies to shorts.</string>
    <string name="revanced_enable_default_playback_speed_shorts_title">Enable shorts default playback speed</string>

@KobeW50
Copy link
Collaborator Author

KobeW50 commented Jan 16, 2024

These are strings that will be added to the final build. If possible, please also review the following strings:

    <string name="revanced_enable_default_playback_speed_shorts_summary_off">Default playback speed does not apply to shorts.</string>
    <string name="revanced_enable_default_playback_speed_shorts_summary_on">Default playback speed applies to shorts.</string>
    <string name="revanced_enable_default_playback_speed_shorts_title">Enable shorts default playback speed</string>

I believe "shorts" in the summaries should be capitalized for consistency with other toggle descriptions. (I'm guessing the reason why it isn't ever capitalized in toggle titles is bec it messes with lexicographic sorting)

<string name="revanced_enable_default_playback_speed_shorts_summary_off">Default playback speed does not apply to Shorts.</string>
<string name="revanced_enable_default_playback_speed_shorts_summary_on">Default playback speed applies to Shorts.</string>
<string name="revanced_enable_default_playback_speed_shorts_title">Enable shorts default playback speed</string>

Will this feature be added into the Default playback speed patch?

@inotia00
Copy link
Owner

I believe "shorts" in the summaries should be capitalized for consistency with other toggle descriptions. (I'm guessing the reason why it isn't ever capitalized in toggle titles is bec it messes with lexicographic sorting)

It is appropriate to write Shorts as an uppercase, but there are quite a lot of changes.

Will this feature be added into the Default playback speed patch?

Yeah it will be included in the Default playback speed patch and will be added to the Experimental Flags of Video setting.

video

I couldn't find a way to open the playback panel on the Shorts player, so I simply made a patch that applies the playback speed specified in Default playback speed in Shorts.

@inotia00
Copy link
Owner

For YouTube Music, the following patch/settings have been added.

inotia00/ReVanced_Extended#1878
inotia00/ReVanced_Extended#1891
inotia00/ReVanced_Extended#1903

Strings

    <string name="revanced_hide_action_button_like_dislike_summary">Hides like and dislike buttons. It does not work in old player layout.</string>
    <string name="revanced_hide_action_button_like_dislike_title">Hide like and dislike buttons</string>
    <string name="revanced_hide_share_button_summary">Hides the share button from the fullscreen.</string>
    <string name="revanced_hide_share_button_title">Hide share button</string>
    <string name="revanced_replace_player_cast_button_summary">"Replace the cast button in the player with the "Open music" button. (Experimental)
Known issues: If one or more music that cannot be played in the playlist, it does not work normally."</string>

Patch

Hide share button
: Add an option to hide the share button from the fullscreen.

@KobeW50
Copy link
Collaborator Author

KobeW50 commented Jan 16, 2024

It is appropriate to write Shorts as an uppercase, but there are quite a lot of changes.

Wdym? In all other summaries "Shorts" is already capitalized.

Strings

<string name="revanced_hide_action_button_like_dislike_summary">Hides the like and dislike buttons. It does not work in the old player layout.</string>
<string name="revanced_hide_action_button_like_dislike_title">Hide like and dislike buttons</string>
<string name="revanced_hide_share_button_summary">Hides the share button in the fullscreen player.</string>
<string name="revanced_hide_share_button_title">Hide fullscreen share button</string>

Regarding the revanced_replace_player_cast_button_summary string, I'm not understanding what the "Open music" button is supposed to do.

Also, keep in mind that "Open music" should only be in quotes if the button will say "Open music" as text (meaning it isn't just an icon button).

If you are able to provide a screen recording or screenshots along with an explanation of the feature it would be helpful.

Patch

I would change the patch name to be more specific regarding which share button it hides, since most people would expect it to be talking about the action bar or flyout panel share button (until they read the description). The toggle title should follow the same reasoning, in addition to the fact that there is already a toggle title named Hide share button (in the action bar settings). It's best practice to use unique toggle titles.

Patch name: Hide fullscreen share button

Patch description: Adds an option to hide the share button in the fullscreen player.

@inotia00
Copy link
Owner

I'm not understanding what the "Open music" button is supposed to do.

Also, keep in mind that "Open music" should only be in quotes if the button will say "Open music" as text (meaning it isn't just an icon button).

If you are able to provide a screen recording or screenshots along with an explanation of the feature it would be helpful.

The description of this patch can be found in the following issues: inotia00/ReVanced_Extended#1431

@inotia00 inotia00 marked this pull request as ready for review January 17, 2024 10:09
@inotia00
Copy link
Owner

To reflect the changes in the private repository (dev branch), I will merge this PR first

If you have any additional changes, please open a new PR

@inotia00 inotia00 merged commit 34e5606 into inotia00:revanced-extended Jan 17, 2024
@inotia00
Copy link
Owner

For YouTube, the following patch/settings have been added.

inotia00/ReVanced_Extended#1920

Strings

    <string name="revanced_keep_landscape_mode_summary">Keeps landscape mode when turning the screen off and on in fullscreen.</string>
    <string name="revanced_keep_landscape_mode_title">Keep landscape mode</string>
    <string name="revanced_keep_landscape_mode_timeout_summary">The amount of milliseconds the landscape mode is forced.</string>
    <string name="revanced_keep_landscape_mode_timeout_title">Keep landscape mode timeout</string>

Patch

Keep landscape mode
: Adds an option to keep landscape mode when turning the screen off and on in fullscreen.

@KobeW50
Copy link
Collaborator Author

KobeW50 commented Jan 19, 2024

Will this feature work to retain landscape orientation when returning to the app from PiP?

Also, can you explain more about the timeout? And what happens if you set it too low or too high?

PS: I just got accepted into university and i already missed a few classes bec i applied late so i have a lot of studying to do. I don't think I'll be able to get around to the 80+ YT patches that I haven't changed the strings for. I understand if you dont want to include any of the YT changes i made in order to keep consistency between the YT patches.

Edit: too late lol

@inotia00
Copy link
Owner

Will this feature work to retain landscape orientation when returning to the app from PiP?

Also, can you explain more about the timeout? And what happens if you set it too low or too high?

  1. Switch to fullscreen.
  2. Turn off the screen.
  3. Turn on the screen.
  4. Fullscreen will exit automatically.

The Keep landscape mode patch prevents 4. Fullscreen will exit automatically.

To add a technical explanation for this patch..
This patch is a patch that forces you to hold the fullscreen during the set timeout after the screen is turned on

I recommend you to apply the patch and test it.

@inotia00
Copy link
Owner

PS: I just got accepted into university and i already missed a few classes bec i applied late so i have a lot of studying to do. I don't think I'll be able to get around to the 80+ YT patches that I haven't changed the strings for. I understand if you dont want to include any of the YT changes i made in order to keep consistency between the YT patches.

I think it's more important to keep the patch consistent, but in some cases it may be necessary.

@KobeW50
Copy link
Collaborator Author

KobeW50 commented Jan 19, 2024

Okay, I'll check out the patch when I have time.

I think it's more important to keep the patch consistent, but in some cases it may be necessary.

I apologize for my lack of communication about this possibility until now. It is possible I'll have time to complete the YT changes after project is discontinued, but I'm sure you wouldn't want to re-open the project just to push a release for string updates lol

@inotia00
Copy link
Owner

inotia00 commented Jan 23, 2024

For Reddit, the following patch/settings have been added.

Remove NSFW warning dialog
Remove notification suggestion dialog

Patch

name: Remove dialog
description: Removes the NSFW community warning dialog or the notifications suggestion dialog by dismissing it automatically.

Strings

title: Remove NSFW warning dialog
summary: Removes the NSFW warning dialog that appears when visiting a subreddit by accepting it automatically.

title: Remove notification suggestion dialog
summary: Removes notifications suggestion dialog that appears when visiting a subreddit by dismissing it automatically.

@KobeW50
Copy link
Collaborator Author

KobeW50 commented Jan 23, 2024

For Reddit, the following patch/settings have been added.

Remove NSFW warning dialog
Remove notification suggestion dialog

Patch

name: Remove dialog
description: Removes the NSFW community warning dialog or the notifications suggestion dialog by dismissing it automatically.

Strings

title: Remove NSFW warning dialog
summary: Removes the NSFW warning dialog that appears when visiting a subreddit by accepting it automatically.

title: Remove notification suggestion dialog
summary: Removes notifications suggestion dialog that appears when visiting a subreddit by dismissing it automatically.

Maybe rename the patch to Remove subreddit dialog? I just feel like the current name is vague.

For the patch description, i hope this makes sense:

Adds options to remove the NSFW community warning and notifications suggestion dialogs by dismissing them automatically.

NSFW strings are good.

title: Remove notification suggestion dialog

summary: Removes the notifications suggestion dialog that appears when visiting a subreddit by dismissing it automatically.

@KobeW50 KobeW50 deleted the patch-descriptions branch May 10, 2024 19:27
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