search

SimonHalvdansson / Harmonic-HN

Modern Android client for Hacker News
https://play.google.com/store/apps/details?id=com.simon.harmonichackernews
Apache License 2.0
611 stars 40 forks source link

Fix android:isLightTheme for light themes #149

Open hsfzxjy opened 7 months ago

hsfzxjy commented 7 months ago

The android:isLightTheme is mistakenly set to false. This leads to webview's theme being always dark even if 'Match WebView dark mode to theme' was turned on.

SimonHalvdansson commented 7 months ago

The WebView color matching logic is quite deep (on the Android side) with different behavior for different Android versions. This was set to false to enable dark mode on light themes on Android 13+ if I recall correctly.

On my Pixel 8 with Android 14, my WebView does not have dark mode when I set match WebView dark mode to theme to on and use a light theme. Most likely your fix should only be applied to a specific set of Android versions - what version are you testing on?

hsfzxjy commented 7 months ago

I am on Android 11. Before the fix I cannot set webview to light mode no matter how I toggle the options.

hsfzxjy commented 7 months ago

This was set to false to enable dark mode on light themes on Android 13+ if I recall correctly.

Why would one enable dark mode on light themes? If one turns on "match webview dark mode to themes", the expected behavior should be light webview on light theme.

SimonHalvdansson commented 7 months ago

Okay it not even being toggleable is quite bad; there definitely needs to be a change here - I just want to make sure that nothing breaks on newer versions of Android.

Why would one enable dark mode on light themes? If one turns on "match webview dark mode to themes", the expected behavior should be light webview on light theme.

Indeed that is the correct behavior, the use is if the user manually toggles it with the button in the bottom right in the WebView.

Since everything is working for newer versions right now, the first step to debug this should be to change only the values-29 and see if that breaks something else, keeping values-31 unchanged.

hsfzxjy commented 7 months ago

Okay, I will see if only changing values-v29 would suffice.

hsfzxjy commented 7 months ago

Okay, I will see if only changing values-v29 would suffice.

Good news, it does fix my problem. I've force-pushed the commit to only change v29.

SimonHalvdansson commented 4 months ago

This is weird, I tried on the emulator now and everyhing is fine for switching WebView themes on both both API 29 and API 30. Can you reproduce the issue in the emulator?

hsfzxjy commented 4 months ago

This is weird, I tried on the emulator now and everyhing is fine for switching WebView themes on both both API 29 and API 30. Can you reproduce the issue in the emulator?

I can't reproduce in emulator either, but such misbehavior indeed happens on my phone. Anyway, I still believe setting android:isLightTheme to false for a light theme is not appropriate, and fixing it shouldn't incur any adverse effect.