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

chore: Reorder and update some strings #118

Conversation

ILoveOpenSourceApplications

@ILoveOpenSourceApplications ILoveOpenSourceApplications changed the base branch from revanced-extended to dev January 11, 2025 06:53
@ILoveOpenSourceApplications ILoveOpenSourceApplications changed the base branch from dev to revanced-extended January 11, 2025 06:54

<!-- SETTINGS: SHORTS_REPEAT_STATE_BACKGROUND
<ListPreference android:entries="@array/revanced_change_shorts_repeat_state_entries" android:title="@string/revanced_change_shorts_background_repeat_state_title" android:key="revanced_change_shorts_background_repeat_state" android:entryValues="@array/revanced_change_shorts_repeat_state_entry_values" />SETTINGS: SHORTS_REPEAT_STATE_BACKGROUND -->
<!-- SETTINGS: SHORTS_REPEAT_STATE
Copy link
Owner

Choose a reason for hiding this comment

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

Identifier SETTINGS: SHORTS_REPEAT_STATE_BACKGROUND is missing.

Choose a reason for hiding this comment

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

Is this being called somewhere, where if I renamed it something might fail?


<!-- PREFERENCE_SCREEN: VIDEO
<SwitchPreference android:title="@string/revanced_reject_av1_codec_title" android:key="revanced_reject_av1_codec" android:summary="@string/revanced_reject_av1_codec_summary" />
<SwitchPreference android:title="@string/revanced_replace_av1_codec_title" android:key="revanced_replace_av1_codec" android:summary="@string/revanced_replace_av1_codec_summary" />
Copy link
Owner

Choose a reason for hiding this comment

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

It's better to group video codec settings.

Choose a reason for hiding this comment

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

Can we give it a preference category of its own then?

@@ -790,6 +790,13 @@
<app.revanced.extension.youtube.settings.preference.ImportExportPreference android:title="@string/revanced_extended_settings_import_export_as_text_title" android:summary="@string/revanced_extended_settings_import_export_as_text_summary" android:inputType="textMultiLine" />
</PreferenceScreen>

<app.revanced.extension.youtube.settings.preference.OpenDefaultAppSettingsPreference android:title="@string/revanced_default_app_settings_title" android:key="revanced_default_app_settings" android:summary="@string/revanced_default_app_settings_summary" />
Copy link
Owner

Choose a reason for hiding this comment

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

The Open default app settings and Open GmsCore Settings settings are located between the Import / Export settings and Spoof streaming data categories.

Categories are sorted first, then settings.

Choose a reason for hiding this comment

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

Can't those two be considered as categories as they do redirect the user to a secondary page?

Copy link
Owner

Choose a reason for hiding this comment

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

I didn't group them because I couldn't think of a suitable category name.

Choose a reason for hiding this comment

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

Do they really need to be categorized for this? When I think of Import / Export settings or Spoof streaming data at the beginning, I'm thinking "its because they take me to another settings page with its own set of options", which is what Open GmsCore settings and Open default app settings does, they take to their own individual app settings page.

Choose a reason for hiding this comment

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

The same is being followed in ReVanced as well.

<string name="revanced_default_app_settings_title">Open default app settings</string>
<string name="revanced_default_app_settings_summary">To open YouTube links in RVX, enable \'Open supported links\' and enable the supported web addresses.</string>

<string name="gms_core_settings_title">Open GmsCore Settings</string>
Copy link
Owner

Choose a reason for hiding this comment

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

In Open default app settings, settings is lowercase, but in Open GmsCore Settings, Settings is uppercase.

Choose a reason for hiding this comment

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

When opening GmsCore, the page opened says microG Settings, which is why I gave it a capital S.

@inotia00 inotia00 changed the base branch from revanced-extended to dev January 13, 2025 08:16
@inotia00
Copy link
Owner

Unlike YouTube, YouTube Music cannot properly apply lexicographical order (due to limitations in patch structure).

@ILoveOpenSourceApplications
Copy link
Author

Oh, that's unfortunate. Anyways I've done the necessary changes for it if the limitation is bypassed at some point.

@inotia00
Copy link
Owner

Thanks

@inotia00 inotia00 merged commit 3604e92 into inotia00:dev Jan 13, 2025
@ILoveOpenSourceApplications ILoveOpenSourceApplications deleted the chore-update-and-reorder-strings branch January 16, 2025 04:53
@ILoveOpenSourceApplications
Copy link
Author

@inotia00, can you answer the unresolved reviews? I just want to get a clarity on why you reverted some of the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants