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

Reapply - Add offline search functionality for addresses #43011

Merged
merged 27 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8ffcb6b
Reapply "Merge pull request #35045 from callstack-internal/issues/30123"
pac-guerreiro Jun 3, 2024
e81675e
fix: unreachable form inputs
pac-guerreiro Jun 3, 2024
d039eb0
chore: disable typescript rule for constant
pac-guerreiro Jun 3, 2024
89492f6
Merge branch 'main' into issues/30123
pac-guerreiro Jun 11, 2024
cab7692
Merge branch 'main' into issues/30123
pac-guerreiro Jun 11, 2024
7257198
Merge branch 'main' into issues/30123
pac-guerreiro Jun 11, 2024
838218d
fix: hiding submit button on address search screen
pac-guerreiro Jun 11, 2024
6b11184
refactor: remove unnecessary code and comments
pac-guerreiro Jun 12, 2024
ea52a61
Merge branch 'main' into issues/30123
pac-guerreiro Jun 12, 2024
fbb2d4a
refactor: apply suggestion
pac-guerreiro Jun 13, 2024
bb1dff8
Merge branch 'main' into issues/30123
pac-guerreiro Jun 13, 2024
1b3a5ef
Merge branch 'main' into issues/30123
pac-guerreiro Jun 19, 2024
6f1994e
refactor: remove unnecessary styles
pac-guerreiro Jun 19, 2024
875bf49
Merge branch 'main' into issues/30123
pac-guerreiro Jun 25, 2024
0a17153
fix: address suggestions list not scrollable on web
pac-guerreiro Jun 25, 2024
e07f641
Merge branch 'main' into issues/30123
pac-guerreiro Jun 26, 2024
a5d2f9b
refactor: fix linter issues
pac-guerreiro Jun 26, 2024
0333496
Merge branch 'main' into issues/30123
pac-guerreiro Jun 26, 2024
f1c0d53
refactor: fix prettier issues
pac-guerreiro Jun 26, 2024
a56370d
fix: suggestions not wrapping inside row
pac-guerreiro Jun 27, 2024
13eb028
Merge branch 'main' into issues/30123
pac-guerreiro Jun 28, 2024
fdadd5f
fix: scrolling issues
pac-guerreiro Jun 28, 2024
27c9233
Merge branch 'main' into issues/30123
pac-guerreiro Jul 2, 2024
785a45f
fix: predefined places being filtered in offline mode
pac-guerreiro Jul 2, 2024
4920a65
Merge branch 'main' into issues/30123
pac-guerreiro Jul 4, 2024
0b5bbe3
Revert "fix: predefined places being filtered in offline mode"
pac-guerreiro Jul 4, 2024
363ab6f
fix: address search not showing recent waypoints while offline
pac-guerreiro Jul 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const onboardingChoices = {
type OnboardingPurposeType = ValueOf<typeof onboardingChoices>;

const CONST = {
RECENT_WAYPOINTS_NUMBER: 20,
DEFAULT_POLICY_ROOM_CHAT_TYPES: [chatTypes.POLICY_ADMINS, chatTypes.POLICY_ANNOUNCE, chatTypes.DOMAIN_ALL],

// Note: Group and Self-DM excluded as these are not tied to a Workspace
Expand Down
62 changes: 49 additions & 13 deletions src/components/AddressSearch/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,25 @@ import CONST from '@src/CONST';
import type {Address} from '@src/types/onyx/PrivatePersonalDetails';
import CurrentLocationButton from './CurrentLocationButton';
import isCurrentTargetInsideContainer from './isCurrentTargetInsideContainer';
import type {AddressSearchProps} from './types';
import listViewOverflow from './listViewOverflow';
import type {AddressSearchProps, PredefinedPlace} from './types';

/**
* Check if the place matches the search by the place name or description.
* @param search The search string for a place
* @param place The place to check for a match on the search
* @returns true if search is related to place, otherwise it returns false.
*/
function isPlaceMatchForSearch(search: string, place: PredefinedPlace): boolean {
if (!search) {
return true;
}
if (!place) {
return false;
}
const fullSearchSentence = `${place.name ?? ''} ${place.description}`;
return search.split(' ').every((searchTerm) => !searchTerm || (searchTerm && fullSearchSentence.toLocaleLowerCase().includes(searchTerm.toLocaleLowerCase())));
pac-guerreiro marked this conversation as resolved.
Show resolved Hide resolved
}

// The error that's being thrown below will be ignored until we fork the
// react-native-google-places-autocomplete repo and replace the
Expand All @@ -41,6 +59,7 @@ function AddressSearch(
isLimitedToUSA = false,
label,
maxInputLength,
onFocus,
onBlur,
onInputChange,
onPress,
Expand Down Expand Up @@ -298,10 +317,16 @@ function AddressSearch(
};
}, []);

const filteredPredefinedPlaces = useMemo(() => {
Copy link
Contributor

@ntdiary ntdiary Oct 16, 2024

Choose a reason for hiding this comment

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

Coming from #48643. If user is online, the filtered predefined places will briefly appear before the server returns the result. :)

if (!isOffline || !searchValue) {
return predefinedPlaces ?? [];
}
return predefinedPlaces?.filter((predefinedPlace) => isPlaceMatchForSearch(searchValue, predefinedPlace)) ?? [];
}, [isOffline, predefinedPlaces, searchValue]);

const listEmptyComponent = useCallback(
() =>
!!isOffline || !isTyping ? null : <Text style={[styles.textLabel, styles.colorMuted, styles.pv4, styles.ph3, styles.overflowAuto]}>{translate('common.noResultsFound')}</Text>,
[isOffline, isTyping, styles, translate],
() => (!isTyping ? null : <Text style={[styles.textLabel, styles.colorMuted, styles.pv4, styles.ph3, styles.overflowAuto]}>{translate('common.noResultsFound')}</Text>),
[isTyping, styles, translate],
);

const listLoader = useCallback(
Expand Down Expand Up @@ -338,11 +363,10 @@ function AddressSearch(
ref={containerRef}
>
<GooglePlacesAutocomplete
disableScroll
fetchDetails
suppressDefaultStyles
enablePoweredByContainer={false}
predefinedPlaces={predefinedPlaces ?? undefined}
predefinedPlaces={filteredPredefinedPlaces}
pac-guerreiro marked this conversation as resolved.
Show resolved Hide resolved
listEmptyComponent={listEmptyComponent}
listLoaderComponent={listLoader}
renderHeaderComponent={renderHeaderComponent}
Expand All @@ -351,7 +375,7 @@ function AddressSearch(
const subtitle = data.isPredefinedPlace ? data.description : data.structured_formatting.secondary_text;
return (
<View>
{!!title && <Text style={[styles.googleSearchText]}>{title}</Text>}
{!!title && <Text style={styles.googleSearchText}>{title}</Text>}
<Text style={[title ? styles.textLabelSupporting : styles.googleSearchText]}>{subtitle}</Text>
</View>
);
Expand Down Expand Up @@ -385,6 +409,7 @@ function AddressSearch(
shouldSaveDraft,
onFocus: () => {
setIsFocused(true);
onFocus?.();
},
onBlur: (event) => {
if (!isCurrentTargetInsideContainer(event, containerRef)) {
Expand Down Expand Up @@ -414,10 +439,18 @@ function AddressSearch(
}}
styles={{
textInputContainer: [styles.flexColumn],
listView: [StyleUtils.getGoogleListViewStyle(displayListViewBorder), styles.overflowAuto, styles.borderLeft, styles.borderRight, !isFocused && {height: 0}],
listView: [
StyleUtils.getGoogleListViewStyle(displayListViewBorder),
listViewOverflow,
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the recent changes, is this style still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eVoloshchak good point! I just removed this 😄 Thanks!

styles.borderLeft,
styles.borderRight,
styles.flexGrow0,
!isFocused && styles.h0,
],
row: [styles.pv4, styles.ph3, styles.overflowAuto],
description: [styles.googleSearchText],
separator: [styles.googleSearchSeparator],
container: [styles.mh100],
}}
numberOfLines={2}
isRowScrollable={false}
Expand All @@ -441,11 +474,14 @@ function AddressSearch(
)
}
placeholder=""
/>
<LocationErrorMessage
onClose={() => setLocationErrorCode(null)}
locationErrorCode={locationErrorCode}
/>
listViewDisplayed
disableScroll
>
<LocationErrorMessage
onClose={() => setLocationErrorCode(null)}
locationErrorCode={locationErrorCode}
/>
</GooglePlacesAutocomplete>
</View>
</ScrollView>
{isFetchingCurrentLocation && <FullScreenLoadingIndicator />}
Expand Down
4 changes: 4 additions & 0 deletions src/components/AddressSearch/listViewOverflow/index.native.ts
pac-guerreiro marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// eslint-disable-next-line no-restricted-imports
import {defaultStyles} from '@styles/index';

export default defaultStyles.overflowHidden;
4 changes: 4 additions & 0 deletions src/components/AddressSearch/listViewOverflow/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// eslint-disable-next-line no-restricted-imports
import {defaultStyles} from '@styles/index';

export default defaultStyles.overflowAuto;
11 changes: 9 additions & 2 deletions src/components/AddressSearch/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,20 @@ type StreetValue = {
street: string;
};

type PredefinedPlace = Place & {
name?: string;
};

type AddressSearchProps = {
/** The ID used to uniquely identify the input in a Form */
inputID?: string;

/** Saves a draft of the input value when used in a form */
shouldSaveDraft?: boolean;

/** Callback that is called when the text input is focused */
onFocus?: () => void;

/** Callback that is called when the text input is blurred */
onBlur?: () => void;

Expand Down Expand Up @@ -65,7 +72,7 @@ type AddressSearchProps = {
canUseCurrentLocation?: boolean;

/** A list of predefined places that can be shown when the user isn't searching for something */
predefinedPlaces?: Place[] | null;
predefinedPlaces?: PredefinedPlace[] | null;

/** A map of inputID key names */
renamedInputKeys?: Address;
Expand All @@ -85,4 +92,4 @@ type AddressSearchProps = {

type IsCurrentTargetInsideContainerType = (event: FocusEvent | NativeSyntheticEvent<TextInputFocusEventData>, containerRef: RefObject<View | HTMLElement>) => boolean;

export type {CurrentLocationButtonProps, AddressSearchProps, IsCurrentTargetInsideContainerType, StreetValue};
export type {CurrentLocationButtonProps, AddressSearchProps, IsCurrentTargetInsideContainerType, StreetValue, PredefinedPlace};
2 changes: 1 addition & 1 deletion src/libs/actions/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ function saveWaypoint(transactionID: string, index: string, waypoint: RecentWayp
if (!recentWaypointAlreadyExists && waypoint !== null) {
const clonedWaypoints = lodashClone(recentWaypoints);
clonedWaypoints.unshift(waypoint);
Onyx.merge(ONYXKEYS.NVP_RECENT_WAYPOINTS, clonedWaypoints.slice(0, 5));
Onyx.merge(ONYXKEYS.NVP_RECENT_WAYPOINTS, clonedWaypoints.slice(0, CONST.RECENT_WAYPOINTS_NUMBER));
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/pages/iou/request/step/IOURequestStepWaypoint.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ function IOURequestStepWaypoint({
onEntryTransitionEnd={() => textInput.current?.focus()}
shouldEnableMaxHeight
testID={IOURequestStepWaypoint.displayName}
style={styles.overflowHidden}
pac-guerreiro marked this conversation as resolved.
Show resolved Hide resolved
>
<FullPageNotFoundView shouldShow={shouldDisableEditor}>
<HeaderWithBackButton
Expand Down Expand Up @@ -252,10 +253,10 @@ export default withWritableReportOrNotFound(
recentWaypoints: {
key: ONYXKEYS.NVP_RECENT_WAYPOINTS,

// Only grab the most recent 5 waypoints because that's all that is shown in the UI. This also puts them into the format of data
// Only grab the most recent 20 waypoints because that's all that is shown in the UI. This also puts them into the format of data
// that the google autocomplete component expects for it's "predefined places" feature.
selector: (waypoints) =>
(waypoints ? waypoints.slice(0, 5) : []).map((waypoint) => ({
(waypoints ? waypoints.slice(0, CONST.RECENT_WAYPOINTS_NUMBER as number) : []).map((waypoint) => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

@neil-marcellini @arosiclair @eVoloshchak, this has caused a regression. Can you pls confirm whats the expected fix for that? I think we can slice the predefinedPlaces array to only show 5 items at a time. It will be still able to search and find option from all the 20 options, we will just slice the array for the UI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a bug, it's working as desired as explained in that regression issue here

name: waypoint.name,
description: waypoint.address ?? '',
geometry: {
Expand Down
3 changes: 3 additions & 0 deletions src/styles/utils/sizing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import type {ViewStyle} from 'react-native';
* https://getbootstrap.com/docs/5.0/utilities/sizing/
*/
export default {
h0: {
height: 0,
},
h100: {
height: '100%',
},
Expand Down
Loading