React-native-google-places-autocomplete: Support dark mode for poweredBy image

Created on 19 Nov 2020  Â·  13Comments  Â·  Source: FaridSafi/react-native-google-places-autocomplete

Thanks for this amazing library.

It would be awesome to support dark mode. The current poweredBy image only has black text. Thanks!

enhancement help wanted

Most helpful comment

I view it more as a style customization than necessarily a dark mode feature itself. You might style the drop down to be dark, even if the rest of your app isn't in dark mode. Because of this, I think it makes more sense as a prop that the user has control over.

Maybe it makes more sense to just let the poweredByText get styled directly in styles, following the API of other styles, since you proposed using Text alongside the logo anyway.

All 13 comments

Because poweredBy is an image and not text + image (for the logo), It doesn't support dark mode.

This is a feature request that I would like to see implemented.

Open to any PRs.

@bell-steven I think this could be done simply by adding a darkMode prop to use for toggling the image in _renderPoweredLogo. That doesn't get into the fussiness of selecting the best option from _all_ the images Google provides for this, but it would at least satisfy this request. I'm willing to submit a PR, if this sounds like a good solution.

@aidanarnold I don't want to add a prop for this. It should happen automatically.
Text + an image for the Google logo is a pretty clean solution.
If we add a prop, how are we detecting if the app is in dark mode? I don't want to have logic built in, it should be as straightforward as possible.

I actually think a prop could make sense. You could use react-native-appearance if you don't mind adding a dependency, and also have a prop override it.

This way, custom themes can be controlled by the consumer of the API, but it also happens automatically.

My app is permanently dark mode, so I would prefer to set this as a prop.

Adding a dependency is exactly what I'm trying to avoid here.

I agree. What's the problem with a darkMode prop then? I don't see a way to detect this automatically without a dependency.

My thinking with a prop is that it puts control into the consuming app to determine whether or not dark mode is detected, i.e. if the consuming app has a toggle for light/dark, that toggle would also control thedarkMode prop. Aside from the powered by logo, I'm not sure what detecting dark mode would meaningfully do from the perspective of this module. The default styling is light, so the consuming app would already be providing overrides to match with a "dark" mode.

Now that React Native (and the native OSes) handle dark mode, I don't want to add any logic to the app that is already handled by React Native or the OS.

I view it more as a style customization than necessarily a dark mode feature itself. You might style the drop down to be dark, even if the rest of your app isn't in dark mode. Because of this, I think it makes more sense as a prop that the user has control over.

Maybe it makes more sense to just let the poweredByText get styled directly in styles, following the API of other styles, since you proposed using Text alongside the logo anyway.

I can see the case for switching to use text and a logo, enabling override via styles for the text, but... I will point out that the image recommended by Google (e.g. here) for a dark background has "Google" itself in white. That's what got me thinking this could be addressed by toggling the image with a prop. ¯\ _(ツ)_/¯

So, is the consensus to replace the existing image with text ("Powered By") and a Google logo, and add a poweredByText style to allow customizing the text?

Thank you @nandorojo and @aidanarnold for sharing your perspectives. Because we already have poweredContainer and powered style objects, I think adding a poweredText style is the logical next step. As an aside, not that renaming powered to poweredImage is a breaking change.

To @aidanarnold's point of adding a custom image, I think that allowing the entire poweredByContainer to be replaced by a custom component fits best with the the mission of the library, which is to provide an extremely _customizable_ Google Places autocomplete complete with sane defaults out of the box. This should probably be a separate PR.

@bell-steven So, I'm trying to make sense of how adding the text (and poweredText style) would work. The image that is currently provided isn't just the Google logo, but the "Powered By" text and Google logo. Assuming you add a poweredText style, and add the "Powered By" text if it is provided, you would end up with "Powered By [Powered By Google]," so I would think that if you are providing that style and want the text, you'd also want to provide an image (logo)... right? I guess you could probably override the image using the powered style, but that seems to make adding the text (with a style) kind of brittle -- because it now sort of requires that you specify an image, unless you're okay with seeing "Powered By [Powered By Google]." Even noting that a dark background will probably hide the text in the image, there would still be a big space between the text and logo (where the that text is), so it'd still look weird...

I think your second point, about allowing the poweredByContainer to be overridden entirely with a custom component, might be a better solution to this, overall. It goes along well with the ability to customize other elements, e.g. rowData, and still doesn't require adding other props.

FWIW, I ended up addressing this issue by overriding the poweredByContainer to have a white background so the provided image would show up correctly. It might not be the best look, but was a reasonable workaround.

The image would definitely have to be changed out to exclude the "powered by" text either way.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tezahzulueta picture tezahzulueta  Â·  3Comments

frankfaustino picture frankfaustino  Â·  4Comments

lukBakTsh picture lukBakTsh  Â·  3Comments

sohel-tech picture sohel-tech  Â·  3Comments

tgreco picture tgreco  Â·  3Comments