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

Convert notification actions to a custom preference #4652

Merged
merged 2 commits into from
Oct 31, 2020

Conversation

XiangRongLin
Copy link
Collaborator

@XiangRongLin XiangRongLin commented Oct 26, 2020

What is it?

  • Codebase improvement (dev facing)

Description of the changes in your PR

  • Convert NotificationSettingsFragment into a custom Preference
    • extract scale switch out of this
  • Create "standard" preference with scale switch and custom preference above

Fixes the following issue(s)

APK testing

app-debug.zip (updated with Stypox suggestions)

Site notes

  • Preference.onDetached() seems to equivalent to Fragment.onPause() for the purpose of saving the preference and recreating the notification.
  • From the doc on the 4 constructors of Preference, only modifying the one that i did should be enogh (I think).

Due diligence

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Thank you! I didn't know this could be done, but it certainly simplifies code ;-)

@TobiGr TobiGr requested a review from Stypox October 30, 2020 22:09
@XiangRongLin
Copy link
Collaborator Author

Github does have a squash and merge feature right? Otherwise I'm going to squash it manually.

@Stypox
Copy link
Member

Stypox commented Oct 31, 2020

@XiangRongLin thank you! @TobiGr merge, I already added it in release draft

@TobiGr TobiGr merged commit 008eb5b into TeamNewPipe:dev Oct 31, 2020
@XiangRongLin XiangRongLin deleted the notification_setting branch October 31, 2020 11:07
This was referenced Nov 10, 2020
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.

[Codebase] Non standard notification setting layout xml
3 participants