-
-
Notifications
You must be signed in to change notification settings - Fork 128
Fix Android KeyboardToolbar dark mode detection with extensible theme hook #1116
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
base: main
Are you sure you want to change the base?
Fix Android KeyboardToolbar dark mode detection with extensible theme hook #1116
Conversation
Hey @JustinHaut Thank you for looking into this and than you for your PR! I appreciate that! ❤️ I like the idea of the fix, but I strongly believe that it should be handled in: Lines 42 to 48 in a57fa4b
It seems like this method returns
So maybe you can see how P. S. it looks like /** Utility object providing static methods for working with UI mode properties from Context. */
internal object UiModeUtils {
/**
* Determines whether the current UI mode is dark mode
*
* @param context The context to check the UI mode from
* @return true if the current UI mode is dark mode, false otherwise
*/
@JvmStatic
fun isDarkMode(context: Context): Boolean =
context.resources.configuration.uiMode and Configuration.UI_MODE_NIGHT_MASK ==
Configuration.UI_MODE_NIGHT_YES
} Worth to check the result of my implementation and this function 👀 Also I'm not sure if my implementation is sensitive to Also - are you sure that you use dark theme across the whole OS? Maybe you just customized keyboard appearance and OS itself is running in light theme? And in app you force dark theme via |
Thank you for the awesome library and for your guidance on the native fix! I've confirmed this works on system theme changes. fun Context.isSystemDarkMode(): Boolean =
resources.configuration.uiMode and Configuration.UI_MODE_NIGHT_MASK == Configuration.UI_MODE_NIGHT_YES -You're correct, implementation is not sensitive to Appearance.setColorScheme() changes. Looks like neither iOS nor Android follow the setColorScheme() So the native fix now correctly handles system detection, but none of the components reflect app-level overrides from Appearance.setColorScheme(). In the meantime, would you like me to adjust the PR to reflect the Android native fix only? ...thinking about it - if users prefer dark mode, most probably keep their system set on it at the system level anyway 🤔 |
Again, from what I remember using
Yes, please do 🙂 The bug that you describe is Android only anyway, so fix for Android only (in native code) looks good for me 😊 Once you do it I'll run e2e tests and will test PR manually too to confirm that it doesn't break anything! Thank you for your PR again 🙌 |
- Use Configuration.UI_MODE_NIGHT_MASK for reliable system dark mode detection - Remove JavaScript fallback approach for cleaner native-only solution - Fix issue where toolbar showed light theme when system was in dark mode Tested on Galaxy S22 (API 35) - now correctly detects dark mode
Thanks for the feedback! Updated the PR to use the native-only approach. The JavaScript fallback has been removed and now it's just the |
Thanks! I'll test implementation over weekend and will merge if don't find any new bugs 🙂 Thank you again for your contribution! |
📊 Package size report
|
Hey @JustinHaut I tested this implementation more, and as I was suspecting before - the new implementation is sensitive to ![]() In this example (tested on API 35 emulator) project I used non-dark theme on OS level, but used |
Just noticed that YouTube has a light keyboard with system light and app dark too, so by that standard this seems reasonable 🙃 Thank you!! 🙂 |
@JustinHaut so what should we do here? Will you close the PR (since it'll bring new bugs too)? I'll try to use Samsung device and play around dark theme to confirm that Samsung devices really return wrong dark theme detection 🤔 |
Seems like this is where the javascript tweak would need to come in to achieve true parity. I personally don't think the dark bar when respecting system mode but not in-app theme looks bad. The white bar on dark background is more jarring IMO. But it's totally up to you. I can close it if you'd prefer revisit this at another time. |
In your JS code yo wrote: if (keyboardColorScheme === "light" && systemColorScheme === "dark") {
return "dark";
} Which is literally: I think we need to understand why system dark mode is not recognized by my current and how we can make it recognizable 🤔 This is really sad that Android doesn't have an API to query the keyboard theme and we need to use transitive dependencies to "guess" the color 😔 This PR aims to solve one problem, but adds new bugs, so I can not merge it as is. I'll try to run Samsung device to see why dark theme can not be detected by my native code! |
📜 Description
Fixed Android KeyboardToolbar dark mode detection by creating a new
useKeyboardToolbarTheme
hook that implements a fallback strategy when keyboard appearance detection is unreliable. The hook falls back to system appearance when keyboard reports "light" but the system is actually in dark mode.💡 Motivation and Context
On Android devices, the KeyboardToolbar components (toolbar, buttons, arrows) incorrectly display in light mode even when the system is in dark mode. This happens because keyboard appearance detection from native events can be unreliable across different Android devices and keyboard implementations.
Evidence from Galaxy S22 (API 35):
🔍 Debug - Keyboard appearance: light // ❌ Incorrect
🔍 Debug - System appearance: dark // ✅ Correct
This issue affects modern Android devices due to OEM customizations and keyboard implementation differences, not just older API versions.
📢 Changelog
JS
useKeyboardToolbarTheme
hook with system appearance fallback logiciOS
Android
🤔 How Has This Been Tested?
yarn prepare
yarn typescript
yarn lint
useKeyboardState/
,useWindowDimensions/
)Testing Environment:
Results:
📸 Screenshots:
Before:
After:
📝 Checklist