search

cooperka / react-native-snackbar

:candy: Material Design "Snackbar" component for Android and iOS.
Other
823 stars 152 forks source link

Enable title coloring #101

Closed thomasw closed 5 years ago

thomasw commented 5 years ago

Related #47

This enables changing the snackbar title color on Android, which is likely a requirement if your app's theme defines a dark value for textColor (doing so overrides the default white color of the title and sets its color to textColor).

This pull request does not yet include the equivalent changes for iOS, which I'm unable to work on and verify at the moment. I'm opening this for visibility so someone can take it the rest of the way if they come across it before I have the chance.

cooperka commented 5 years ago

This looks great @thomasw, thank you. If I have time to make equivalent changes to iOS this week I will, otherwise I'll merge as-is and note that it's Android-only in the readme. Cheers.

thomasw commented 5 years ago

Sweet 👍 As you can see from those test failures, some of the changes which I claimed didn't result in behavior changes actually did result in at least a subtle one. On Android, the implementation behaves the same when given a config containing keys with their values set to undefined vs. a config with undefined keys. I'm assuming the same would be true for the iOS side, but it's definitely something we'll want to verify.

thomasw commented 5 years ago

On Android, the implementation behaves the same when given a config containing keys with their values set to undefined...

This might not actually be true. These changes cause an exception when a snackbar is invoked with an undefined action. https://github.com/cooperka/react-native-snackbar/blob/2b19f0ce39e26982b345a464b1c90dcc7320729a/android/src/main/java/com/azendoo/reactnativesnackbar/SnackbarModule.java#L104 must be evaluating to true when it shouldn't be.

I'll dig a little deeper as soon as I have time to come back to this.

pozdena commented 5 years ago

Not sure if this is the right way to do this, but I made a PR to merge iOS title color support into thomas' fork. I tested it; works well in my app. I can't speak to the test failure situation, but hopefully you guys can incorporate these changes. Thanks!

https://github.com/thomasw/react-native-snackbar/pull/1

cooperka commented 5 years ago

Thank you both! I merged this for Android and added @pozdena's iOS change separately in 3ccb4630f2e6762cfc0b8a81b202032177a46396. Both platforms tested and verified.

thomasw commented 5 years ago

Thanks, everyone! 🎆