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 settings to match device's theme (dark & black) #3632

Merged
merged 4 commits into from
Mar 18, 2021

Conversation

B0pol
Copy link
Member

@B0pol B0pol commented May 22, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Added two device theme settings:

  • Device theme (Dark)
  • Device theme (Black)

Device theme (Dark) and Device theme (Black) will set the theme to light if device's theme is light or unknown, otherwise it will set dark or black.

Available for Android 10+ only, LineageOS 16+ and other ROMs providing dark theme.

Here is a GIF:
newpipe_device_theme_gif

Fixes the following issue(s)

Testing apk

NewPipe_theme.zip

Agreement

@B0pol B0pol added feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface labels May 22, 2020
@B0pol B0pol added this to the 0.20.0 milestone May 22, 2020
@B0pol B0pol changed the title Add settings for device theme (dark & black) Add settings to match device's theme (dark & black) May 22, 2020
@B0pol B0pol modified the milestones: 0.20.0, 0.19.5 May 28, 2020
@B0pol
Copy link
Member Author

B0pol commented May 31, 2020

I've rebased and fixed a bug around isLightThemeSelected.

Copy link
Contributor

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Nice 👍

app/src/main/java/org/schabi/newpipe/util/ThemeHelper.java Outdated Show resolved Hide resolved
app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
app/src/main/java/org/schabi/newpipe/util/ThemeHelper.java Outdated Show resolved Hide resolved
@wb9688
Copy link
Contributor

wb9688 commented Jun 23, 2020

Imho we should hide the follow system theme options when the user's Android version doesn't support that.

@B0pol
Copy link
Member Author

B0pol commented Jun 23, 2020

Imho we should hide the follow system theme options when the user's Android version doesn't support that.

I won't hide it for android 9 because of LineageOS supporting themes anyway. And SAF settings was doing this for a long time

Copy link
Contributor

@wb9688 wb9688 left a comment

Choose a reason for hiding this comment

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

Other than this small thing, I would merge this.

app/src/main/java/org/schabi/newpipe/util/ThemeHelper.java Outdated Show resolved Hide resolved
@Stypox
Copy link
Member

Stypox commented Jul 3, 2020

From @B0pol accidentally posted in #3471 (comment)
I rebased and used a switch preference instead of "follow … (theme 1)" "follow … (theme 2)"

@wb9688
Copy link
Contributor

wb9688 commented Jul 5, 2020

@B0pol: Your switch preference is flawed, because then it won't actually follow device theme when you've selected the light theme.

@B0pol
Copy link
Member Author

B0pol commented Jul 5, 2020

@B0pol: Your switch preference is flawed, because then it won't actually follow device theme when you've selected the light theme.

that's intended. With what would you switch? Dark or black? If we had only dark and light I'd have make a simple third theme: auto. The point is to select between dark and black here

@wb9688
Copy link
Contributor

wb9688 commented Jul 5, 2020

@B0pol: Yes, but why is it showing the light option when you could only choose between dark and black for the follow device theme functionality? I still prefer your previous UI though.

@TobiGr
Copy link
Contributor

TobiGr commented Jul 5, 2020

I agree with @wb9688. What about replacing the switch with a list

Follow device theme
May not work on Android 9 and below. Device theme ignored.
May not work on Android 9 and below. Use (dark|black) theme on device's night mode.

@opusforlife2
Copy link
Collaborator

When the follow system theme toggle is switched on, the light theme should be greyed out so only the dark and black options are selectable. That's what I had suggested previously.

@wb9688 wb9688 modified the milestones: 0.19.6, 0.20.0 Jul 14, 2020
@B0pol
Copy link
Member Author

B0pol commented Jul 24, 2020

I have another idea:

  • light
  • dark
  • black
  • [maybe? Terminal theme]
  • automatic (system)
  • [in the future] automatic (battery saver)
  • [in the future] automatic (day / night)

And after that two other lists
[In the future, if it exists)

  • light 1
  • light and cool 2

The second list:

  • dark
  • black
  • [terminal?]

And if automatic is set, we switch between light and the selected night theme (default dark).

@wb9688 wb9688 modified the milestones: 0.20.0, 0.20.1 Jul 29, 2020
@opusforlife2 opusforlife2 mentioned this pull request Aug 17, 2020
@opusforlife2
Copy link
Collaborator

@B0pol See #4148 (comment)

@Stypox Stypox force-pushed the device_theme branch 2 times, most recently from 14648d1 to 8568acf Compare March 18, 2021 10:16
fix bugs related to isLightThemeSelected not handling device themes
such as license having dark background when it should be white
@Stypox Stypox dismissed their stale review March 18, 2021 10:26

Outdated

@Stypox Stypox removed request for TobiGr and wb9688 March 18, 2021 10:26
Also remove unused strings
@Stypox
Copy link
Member

Stypox commented Mar 18, 2021

I rebased, squashed some commits, added 731c65c which refactors ThemeHelper and tested thorougly on API 19 (emulated, where there is no device theme system setting) and on API 29 (Fairphone 3). Everything works fine and smoothly. @TobiGr This has also been tested before many times and I think it should be merged. @B0pol could you review my changes?

@Stypox Stypox self-assigned this Mar 18, 2021
@Redirion
Copy link
Member

Tested on Pixel 4a (API 30). LGTM!

@Redirion Redirion merged commit 08b960c into TeamNewPipe:dev Mar 18, 2021
This was referenced Mar 21, 2021
tossj pushed a commit to tossj/NewPipe-legacy that referenced this pull request Apr 22, 2021
Add settings to match device's theme (dark & black)
@B0pol B0pol deleted the device_theme branch August 20, 2022 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schedule dark mode Add "System default" theme option. Add full black theme.
10 participants