Repository navigation
Migrate Content settings screen to Compose (KMP) - #13931
awaisdev5765 wants to merge 3 commits into
Conversation
Implements TeamNewPipe#8981 (long-press Clear cache action on the app icon) and partially implements TeamNewPipe#10448 (keyboard shortcuts: space/media keys for play-pause, J/L for seek, M for mute) as a gift to the project.
Hardware keyboard users reported that arrow keys initiate focus jumps between player UI elements instead of seeking. On non-TV devices, LEFT/RIGHT now seek and D-pad-center toggles playback. TV D-pad navigation behavior is unchanged.
Adds ContentSettingsScreen/ViewModel/Preferences to the shared module following the established settings-screen pattern, wires it into the settings home navigation (SettingsCategoryType.CONTENT), and introduces ContentActions for platform-specific behavior. Image cache wipe, restricted-mode cookies and localization reload are marked as TODOs — they need app-module hooks. Legacy ContentSettingsFragment behavior is preserved until full migration.
github-actions
Bot
added
size/large
|
Please link this PR to an existing issue you are assigned to. Assignees are expected to discuss the details of the PR beforehand. This PR will be closed if it is not linked to an issue. |
|
|
This PR is part of the Compose migration tracked in #11198 — it migrates the Content settings screen, resolving one of the TODO: wire remaining sub-screens cases in SettingsHomeScreen (SettingsCategoryType.CONTENT). |
|
@awaisdev5765 thanks for the PR. I see you agreed to the contribution guidelines but did not really follow it. Specifically:
|
|
Did you use any AI assistants to help you when working on this PR? If so, how much of the work was done with it? |
| * "Clear cache" long-press shortcut on the app icon, so that users can wipe the | ||
| * caches without having to open the app and dig through the settings. | ||
| * | ||
| * See https://github.com/TeamNewPipe/NewPipe/issues/8981 |
There was a problem hiding this comment.
Doesn't seems connected to the migration nor I see any discussion on this issue as well that we want to work on this
|
I don't see any implementation done for non-android part of the screens (jvm and iOS). Was there a reason those weren't migrated? |
TobiGr
added
rewrite
theimpulson
added
the
spam
|
Closing due to violating contribution guidelines of NewPipe. |
What is it?
Description of the changes in your PR
This migrates the Content settings screen to the new Compose/KMP shared module, following the established
VideoAudioSettingsScreenpattern:ContentSettingsScreen(Compose) +ContentSettingsViewModel(@KoinViewModel) +ContentPreferences: app language (Android 13+ system locale settings), image quality (values matching the legacysettings_keys.xml), and YouTube restricted mode.ContentActionsinterface with anAndroidContentActionsactual — opens the system app-locale settings and reads the current language label. Image-cache wipe, restricted-mode cookie update, and localization reload are deliberately marked TODOs: they depend on app-module classes (CoilHelper,DownloaderImpl,NewPipe.setupLocalization) which the shared module can't reference yet. Happy to add hooks in a follow-up if a different direction is preferred.Destination+navModule()+ the settings home:SettingsCategoryType.CONTENTnow resolves one of the// TODO: wire remaining sub-screenscases inSettingsHomeScreen.strings.xml.This is an incremental migration — the legacy
ContentSettingsFragmentremains untouched and authoritative until full migration.Before/After Screenshots/Screen Record
Fixes the following issue(s)
// TODO: wire remaining sub-screenscases inSettingsHomeScreen).Relies on the following changes
Testing/QA
Devices
Steps for testing
AndroidContentActions)APK for testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence