fix: android platform colors

[ecklf]

Overview

I am using Android 11 and the colors didn't work for me. Did some research on PlatformColors and found that the current values got deprecated in Android API 28 (Pie).

With this move, Google seems to be encouraging the use of colors.xml.

Because of this, I added customization options with sensible defaults attached to the readme.

[mmazzarolo]

👋 @ecklf thanks for the PR! I'm curious, what's stopping you from naming your theme colors using the same convention that is currently being used (secondary_text_dark, hint_foreground_light, etc...)? I haven't tried it, but I assume it would work and would also allow us to support both pre/post API 28 seamlessly.

[ecklf]

Thanks for your feedback.

I'm curious, what's stopping you from naming your theme colors using the same convention that is currently being used (secondary_text_dark, hint_foreground_light, etc...)?

• Prefixed with dialog for giving the context of which element is affected
• hint_foreground_light is currently used to define the placeholder and underline color https://github.com/mmazzarolo/react-native-dialog/blob/2baf3f18f5a54c57991fd479e9dde3e54b74fe60/src/Input.tsx#L42-L53 I don't think that is descriptive. We could extract that into e.g. placeholder-text and underline-color
• I can add back in text for primary / secondary. Makes sense.

Let me know what you think and I'll make changes accordingly.

[mmazzarolo]

Yeah, these names are not descriptive at all 😞 — but they're the ones Google used, right? To clarify: as it is, this would be a breaking change. I think we should either just suggest using the names that were being used before API 28 (by pointing out how to set them in colors.xml like you did), or support both the old names and the more "descriptive" ones. Otherwise, this would be a breaking change. What do you think?

[ecklf]

They were used by Google, but you would theme native dialogs the following:

<style name="ThemeOverlay.App.MaterialAlertDialog" parent="ThemeOverlay.MaterialComponents.MaterialAlertDialog">
    <item name="colorPrimary">@color/shrine_pink_100</item>
    <item name="colorSecondary">@color/shrine_pink_100</item>
    <item name="colorSurface">@color/shrine_pink_light</item>
    <item name="colorOnSurface">@color/shrine_pink_900</item>
    <item name="alertDialogStyle">@style/MaterialAlertDialog.App</item>
    <item name="materialAlertDialogTitleTextStyle">@style/MaterialAlertDialog.App.Title.Text</item>
    <item name="buttonBarPositiveButtonStyle">@style/Widget.App.Button</item>
    <item name="buttonBarNeutralButtonStyle">@style/Widget.App.Button</item>
</style>

Since this library is using react-native components, it would make sense for me to specify what the colors are for.

I think we should either just suggest using the names that were being used before API 28 (by pointing out how to set them in colors.xml like you did), or support both the old names and the more "descriptive" ones. Otherwise, this would be a breaking change.

I haven't tested it, but I am sure you could do something like this to keep the old values.

<color name="dialog_primary_text_light">@android:color/primary_text_light</color>

Either way, it would indeed be a breaking change since it requires adding a colors.xml file with specific names. The alternative would be hard coding the color values (except the existing color prop on Button) to make this non-breaking.

If going the colors.xml route I would suggest:

<?xml version="1.0" encoding="utf-8"?>
<resources>
   <color name="dialog_surface_light">#FFFFFF</color>
   <color name="dialog_surface_dark">#212121</color>
   <color name="dialog_primary_text_light">#212121</color>
   <color name="dialog_primary_text_dark">#FAFAFA</color>
   <color name="dialog_secondary_text_light">#727272</color>
   <color name="dialog_secondary_text_dark">#C7C7C7</color>
   <color name="dialog_hint_text_light">#BF727272</color>
   <color name="dialog_hint_text_dark">#BFC7C7C7</color>
</resources>

This is based on https://material.io/develop/web/theming/theming-guide:

- Primary, used for most text.
- Secondary, used for text which is lower in the visual hierarchy.
- Hint, used for text hints (such as those in text fields and labels).
- Disabled, used for text in disabled components and content.
- Icon, used for icons.
- On-surface, used for text that is on top of a surface background.
- On-secondary, used for text that is on top of a secondary background.
- On-primary, used for text that is on top of a primary background.

[mmazzarolo]

Thanks, makes sense 👍

Just to clarify:

I haven't tested it, but I am sure you could do something like this to keep the old values.
Either way, it would indeed be a breaking change since it requires adding a colors.xml file with specific names. The alternative would be hard coding the color values (except the existing color prop on Button) to make this non-breaking.

What I mean here is more like:

PlatformColor(`@android:color/NEW_COLOR_NAME`) || PlatformColor(`@android:color/hint_foreground_dark)

Wouldn't this allow supporting both the current name + the new one, avoiding breaking changes?

[ecklf]

Thanks, makes sense 👍

Just to clarify:

I haven't tested it, but I am sure you could do something like this to keep the old values. Either way, it would indeed be a breaking change since it requires adding a colors.xml file with specific names. The alternative would be hard coding the color values (except the existing color prop on Button) to make this non-breaking.

What I mean here is more like:

PlatformColor(`@android:color/NEW_COLOR_NAME`) || PlatformColor(`@android:color/hint_foreground_dark)

Wouldn't this allow supporting both the current name + the new one, avoiding breaking changes?

Just tested that which resulted in a crash. Seems the color needs to exist when using PlatformColor. I didn't have any errors using the current state of the library btw, but I would have things like white text in dark mode.

[Calinteodor]

Hei! The problem persists. Will this be merged? Dark mode doesn't work ok on Android. Thanks!

[Nantris]

@Calinteodor what do you mean dark mode doesn't work on Android? In what sense? Can you share a screenshot?

[Calinteodor]

I think the problem is with PlatformColor not working properly on Android, as it is mentioned above. This is why I needed to apply this patch:

•        color: PlatformColor(`@android:color/${isDark ? "secondary_text_dark" : "secondary_text_light"}`),
  

•        color: isDark ? '#C7C7C7' : '#727272',
  

Here are some screenshots before the patch is applied, DARK MODE and LIGHT MODE:

Here are some screenshots after the patch is applied, DARK MODE and LIGHT MODE:

[statico]

+1 to this. Came here looking for a solution because our dialogs on Android are missing the title and the description is too faint.

My current fix is to add style={{ color: null }} to <Dialog.Title> and <Dialog.Description>. This breaks TypeScript types but dialogs now appear correct on iOS and Android in both dark and light modes.