Skip to content
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

Remove lingering border from AddressSearch component #6115

Merged
merged 18 commits into from
Nov 9, 2021
112 changes: 63 additions & 49 deletions src/components/AddressSearch.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _ from 'underscore';
import React, {useEffect, useRef} from 'react';
import React, {useEffect, useState, useRef} from 'react';
import PropTypes from 'prop-types';
import {LogBox} from 'react-native';
import {LogBox, View} from 'react-native';
import {GooglePlacesAutocomplete} from 'react-native-google-places-autocomplete';
import CONFIG from '../CONFIG';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
Expand Down Expand Up @@ -35,8 +35,11 @@ const defaultProps = {
containerStyles: null,
};

// Do not convert to class component! setAddressText only works in functional components!
// Reference: https://github.com/FaridSafi/react-native-google-places-autocomplete/issues/609#issuecomment-886133839
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
const AddressSearch = (props) => {
const googlePlacesRef = useRef();
const [displayListViewBorder, setDisplayListViewBorder] = useState(false);
useEffect(() => {
if (!googlePlacesRef.current) {
return;
Expand Down Expand Up @@ -79,55 +82,66 @@ const AddressSearch = (props) => {
};

return (
<GooglePlacesAutocomplete
ref={googlePlacesRef}
fetchDetails
suppressDefaultStyles
enablePoweredByContainer={false}
onPress={(data, details) => saveLocationDetails(details)}
query={{
key: 'AIzaSyC4axhhXtpiS-WozJEsmlL3Kg3kXucbZus',
language: props.preferredLocale,
types: 'address',
components: 'country:us',
}}
requestUrl={{
useOnPlatform: 'web',
url: `${CONFIG.EXPENSIFY.URL_EXPENSIFY_COM}api?command=Proxy_GooglePlaces&proxyUrl=`,
}}
textInputProps={{
InputComp: ExpensiTextInput,
label: props.label,
containerStyles: props.containerStyles,
errorText: props.errorText,
onChangeText: (text) => {
const isTextValid = !_.isEmpty(text) && _.isEqual(text, props.value);

// Ensure whether an address is selected already or has address value initialized.
if (!_.isEmpty(googlePlacesRef.current.getAddressText()) && !isTextValid) {
saveLocationDetails({});
}
},
// We use the View height to determine if we should hide the border and margin of the listView dropdown
// to prevent a lingering border when there are no address suggestions.
// The height of the input + wrapped error message is 112 pixels
<View
onLayout={(event) => {
const {height} = event.nativeEvent.layout;
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
return height > 112 ? setDisplayListViewBorder(true) : setDisplayListViewBorder(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the answer to this question but it seems worth asking. Can we use a more dynamic way to measure the height? We have a ref to the <GooglePlacesAutoComplete> component so maybe we can use the measure function (mentioned here).

Copy link
Contributor Author

@luacmartins luacmartins Oct 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm we can definitely get the input's height dynamically. I'm not sure if it's worth it though, because we still have to manually compensate for the top margin and border width, so the conditional becomes something like this height > inputHeight + 6. Also, the solution is far from elegant 🤣

I'll spend a bit more time and see if I can come up with something better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually managed to move the onLayout prop to the GooglePlacesAutocomplete component and now the solution is independent of the text input height. Thanks for the tip @Luke9389!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll review again

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh rad, nice work @luacmartins! I really appreciate you following through on that. 🙇

}}
styles={{
textInputContainer: [styles.flexColumn],
listView: [
styles.borderTopRounded,
styles.borderBottomRounded,
styles.mt1,
styles.overflowAuto,
styles.borderLeft,
styles.borderRight,
],
row: [
styles.pv4,
styles.ph3,
styles.overflowAuto,
],
description: [styles.googleSearchText],
separator: [styles.googleSearchSeparator],
}}
/>
>
<GooglePlacesAutocomplete
ref={googlePlacesRef}
fetchDetails
suppressDefaultStyles
enablePoweredByContainer={false}
onPress={(data, details) => saveLocationDetails(details)}
query={{
key: 'AIzaSyC4axhhXtpiS-WozJEsmlL3Kg3kXucbZus',
language: props.preferredLocale,
types: 'address',
components: 'country:us',
}}
requestUrl={{
useOnPlatform: 'web',
url: `${CONFIG.EXPENSIFY.URL_EXPENSIFY_COM}api?command=Proxy_GooglePlaces&proxyUrl=`,
}}
textInputProps={{
InputComp: ExpensiTextInput,
label: props.label,
containerStyles: props.containerStyles,
errorText: props.errorText,
onChangeText: (text) => {
const isTextValid = !_.isEmpty(text) && _.isEqual(text, props.value);

// Ensure whether an address is selected already or has address value initialized.
if (!_.isEmpty(googlePlacesRef.current.getAddressText()) && !isTextValid) {
saveLocationDetails({});
}
},
}}
styles={{
textInputContainer: [styles.flexColumn],
listView: [
displayListViewBorder && styles.borderTopRounded,
displayListViewBorder && styles.borderBottomRounded,
displayListViewBorder && styles.mt1,
styles.overflowAuto,
styles.borderLeft,
styles.borderRight,
],
row: [
styles.pv4,
styles.ph3,
styles.overflowAuto,
],
description: [styles.googleSearchText],
separator: [styles.googleSearchSeparator],
}}
/>
</View>
);
};

Expand Down