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

Display Route #25434

Merged
merged 33 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
41a3590
Add GetRoute API command
thienlnam Aug 17, 2023
a0f270c
Add handling to fetch route
thienlnam Aug 17, 2023
e5c55cd
get display showing
thienlnam Aug 18, 2023
92e4489
Add styling / update version
thienlnam Aug 18, 2023
3a2f1a0
Add errorFields
thienlnam Aug 18, 2023
c3abd93
Prevent fetching while offline
thienlnam Aug 18, 2023
e03cd93
Fix proptype
thienlnam Aug 18, 2023
4201756
Add error dot indicator and screen wrapper
thienlnam Aug 18, 2023
1b9056c
Handle clearing errors when updating input
thienlnam Aug 18, 2023
02529d3
Merge branch 'main' into jack-add-map-display-route
thienlnam Aug 21, 2023
2c5a9f0
Update react-native-x-maps to 1.0.8
thienlnam Aug 21, 2023
a8fb1c7
Update DistanceRequest.js
thienlnam Aug 21, 2023
0105c37
prevent looping of getRoute
thienlnam Aug 21, 2023
d67bba4
add waypoint validation
thienlnam Aug 22, 2023
c928bfd
update styling
thienlnam Aug 22, 2023
1cc24c9
Prettier
thienlnam Aug 22, 2023
b33c36d
Update styles.js
thienlnam Aug 22, 2023
2f3be36
Merge branch 'main' into jack-add-map-display-route
thienlnam Aug 22, 2023
d8cf419
Merge branch 'main' into jack-add-map-display-route
thienlnam Aug 22, 2023
679872e
Update DistanceRequest.js
thienlnam Aug 22, 2023
55b5a38
clear out existing routes
thienlnam Aug 22, 2023
f4a8fce
Update Transaction.js
thienlnam Aug 22, 2023
d6d9d74
Merge branch 'main' into jack-add-map-display-route
thienlnam Aug 22, 2023
944241e
use previous ref
thienlnam Aug 22, 2023
90b941e
lint
thienlnam Aug 22, 2023
090ba17
Clear route on new waypoint
thienlnam Aug 22, 2023
b542085
style actually
thienlnam Aug 22, 2023
9f4d3ab
Merge branch 'main' into jack-add-map-display-route
thienlnam Aug 22, 2023
e32aeb1
Merge branch 'main' into jack-add-map-display-route
thienlnam Aug 22, 2023
0eeb47b
Move shouldFetchRoute to a constant
thienlnam Aug 22, 2023
844ee72
Update DistanceRequest.js
thienlnam Aug 22, 2023
2fe1326
update logic lol
thienlnam Aug 22, 2023
4708446
adjust the check
thienlnam Aug 22, 2023
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
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@
"react-native-web-linear-gradient": "^1.1.2",
"react-native-web-lottie": "^1.4.4",
"react-native-webview": "^11.17.2",
"react-native-x-maps": "^1.0.9",
"react-native-x-maps": "1.0.10",
"react-pdf": "^6.2.2",
"react-plaid-link": "3.3.2",
"react-web-config": "^1.0.0",
Expand Down
37 changes: 34 additions & 3 deletions src/components/DistanceRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {withOnyx} from 'react-native-onyx';
import MapView from 'react-native-x-maps';
import ONYXKEYS from '../ONYXKEYS';
import * as Transaction from '../libs/actions/Transaction';
import * as TransactionUtils from '../libs/TransactionUtils';
import MenuItemWithTopDescription from './MenuItemWithTopDescription';
import * as Expensicons from './Icon/Expensicons';
import theme from '../styles/themes/default';
Expand All @@ -21,6 +22,10 @@ import useNetwork from '../hooks/useNetwork';
import useLocalize from '../hooks/useLocalize';
import Navigation from '../libs/Navigation/Navigation';
import ROUTES from '../ROUTES';
import ScreenWrapper from './ScreenWrapper';
import DotIndicatorMessage from './DotIndicatorMessage';
import * as ErrorUtils from '../libs/ErrorUtils';
import usePrevious from '../hooks/usePrevious';

const MAX_WAYPOINTS = 25;
const MAX_WAYPOINTS_TO_DISPLAY = 4;
Expand Down Expand Up @@ -51,6 +56,9 @@ const propTypes = {
address: PropTypes.string,
}),
}),

/** Server side errors keyed by microtime */
errorFields: PropTypes.objectOf(PropTypes.objectOf(PropTypes.string)),
}),

/** Data about Mapbox token for calling Mapbox API */
Expand Down Expand Up @@ -78,7 +86,13 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) {

const waypoints = lodashGet(transaction, 'comment.waypoints', {});
const numberOfWaypoints = _.size(waypoints);

const lastWaypointIndex = numberOfWaypoints - 1;
const isLoadingRoute = lodashGet(transaction, 'comment.isLoading', false);
const hasRouteError = Boolean(lodashGet(transaction, 'errorFields.route'));
const previousWaypoints = usePrevious(waypoints);
const haveWaypointsChanged = !_.isEqual(previousWaypoints, waypoints);
const shouldFetchRoute = haveWaypointsChanged && !isOffline && !isLoadingRoute && TransactionUtils.validateWaypoints(waypoints);

const waypointMarkers = _.filter(
_.map(waypoints, (waypoint, key) => {
Expand Down Expand Up @@ -133,10 +147,19 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) {
setShouldShowGradient(visibleAreaEnd < scrollContentHeight);
};

// Handle fetching the route when there are at least 2 waypoints
useEffect(() => {
if (!shouldFetchRoute) {
return;
}

Transaction.getRoute(transactionID, waypoints);
}, [shouldFetchRoute, transactionID, waypoints]);

useEffect(updateGradientVisibility, [scrollContainerHeight, scrollContentHeight]);

return (
<>
<ScreenWrapper shouldEnableMaxHeight>
Copy link
Member

Choose a reason for hiding this comment

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

Nested ScreenWrapper can cause extra padding. There should only be one ScreenWrapper component per page. This caused #26008.

<View
style={styles.distanceRequestContainer(scrollContainerMaxHeight)}
onLayout={(event = {}) => setScrollContainerHeight(lodashGet(event, 'nativeEvent.layout.height', 0))}
Expand Down Expand Up @@ -183,6 +206,13 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) {
colors={[theme.transparent, theme.modalBackground]}
/>
)}
{hasRouteError && (
<DotIndicatorMessage
style={[styles.mh5, styles.mv3]}
messages={ErrorUtils.getLatestErrorField(transaction, 'route')}
type="error"
/>
)}
</View>
<View style={[styles.flexRow, styles.justifyContentCenter, styles.pt1]}>
<Button
Expand All @@ -204,6 +234,8 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) {
location: CONST.SF_COORDINATES,
zoom: DEFAULT_ZOOM_LEVEL,
}}
directionCoordinates={lodashGet(transaction, 'routes.route0.geometry.coordinates', [])}
directionStyle={styles.mapDirection}
style={styles.mapView}
waypoints={waypointMarkers}
styleURL={CONST.MAPBOX_STYLE_URL}
Expand All @@ -219,7 +251,7 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) {
</View>
)}
</View>
</>
</ScreenWrapper>
);
}

Expand All @@ -229,7 +261,6 @@ DistanceRequest.defaultProps = defaultProps;
export default withOnyx({
transaction: {
key: (props) => `${ONYXKEYS.COLLECTION.TRANSACTION}${props.transactionID}`,
selector: (transaction) => (transaction ? {transactionID: transaction.transactionID, comment: {waypoints: lodashGet(transaction, 'comment.waypoints')}} : null),
},
mapboxAccessToken: {
key: ONYXKEYS.MAPBOX_ACCESS_TOKEN,
Expand Down
32 changes: 32 additions & 0 deletions src/libs/TransactionUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,37 @@ function isReceiptBeingScanned(transaction) {
}

/**
* Verifies that the provided waypoints are valid
* @param {Object} waypoints
* @returns {Boolean}
*/
function validateWaypoints(waypoints) {
const waypointValues = _.values(waypoints);

// Ensure the number of waypoints is between 2 and 25
if (waypointValues.length < 2 || waypointValues.length > 25) {
return false;
}

for (let i = 0; i < waypointValues.length; i++) {
const currentWaypoint = waypointValues[i];
const previousWaypoint = waypointValues[i - 1];

// Check if the waypoint has a valid address
if (!currentWaypoint || !currentWaypoint.address || typeof currentWaypoint.address !== 'string' || currentWaypoint.address.trim() === '') {
return false;
}

// Check for adjacent waypoints with the same address
if (previousWaypoint && currentWaypoint.address === previousWaypoint.address) {
return false;
}
}

return true;
}

/*
* @param {Object} transaction
* @param {String} transaction.type
* @param {Object} [transaction.customUnit]
Expand All @@ -269,5 +300,6 @@ export {
getAllReportTransactions,
hasReceipt,
isReceiptBeingScanned,
validateWaypoints,
isDistanceRequest,
};
89 changes: 88 additions & 1 deletion src/libs/actions/Transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Onyx from 'react-native-onyx';
import lodashGet from 'lodash/get';
import ONYXKEYS from '../../ONYXKEYS';
import * as CollectionUtils from '../CollectionUtils';
import * as API from '../API';

const allTransactions = {};
Onyx.connect({
Expand Down Expand Up @@ -47,6 +48,15 @@ function addStop(transactionID) {
[`waypoint${newLastIndex}`]: {},
},
},

// Clear the existing route so that we don't show an old route
routes: {
route0: {
geometry: {
coordinates: null,
},
},
},
});
}

Expand All @@ -63,6 +73,19 @@ function saveWaypoint(transactionID, index, waypoint) {
[`waypoint${index}`]: waypoint,
},
},
// Empty out errors when we're saving a new waypoint as this indicates the user is updating their input
errorFields: {
route: null,
},

// Clear the existing route so that we don't show an old route
routes: {
route0: {
geometry: {
coordinates: null,
},
},
},
});
}

Expand Down Expand Up @@ -96,8 +119,72 @@ function removeWaypoint(transactionID, currentIndex) {
...transaction.comment,
waypoints: reIndexedWaypoints,
},
Copy link
Member

Choose a reason for hiding this comment

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

We should also clear the error when we are removing the waypoint as we do in saveWaypoint action . This caused #27538

// Clear the existing route so that we don't show an old route
routes: {
route0: {
geometry: {
coordinates: null,
},
},
},
};
Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, newTransaction);
}

export {addStop, createInitialWaypoints, saveWaypoint, removeWaypoint};
/**
* Gets the route for a set of waypoints
* Used so we can generate a map view of the provided waypoints
* @param {String} transactionID
* @param {Object} waypoints
*/
function getRoute(transactionID, waypoints) {
API.read(
'GetRoute',
{
transactionID,
waypoints: JSON.stringify(waypoints),
},
{
optimisticData: [
{
// Clears any potentially stale error messages from fetching the route
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {
comment: {
isLoading: true,
},
errorFields: {
route: null,
},
},
},
],
// The route and failure are sent back via pusher in the BE, we are just clearing the loading state here
successData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {
comment: {
isLoading: false,
},
},
},
],
failureData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {
comment: {
isLoading: false,
},
},
},
],
},
);
}

export {addStop, createInitialWaypoints, saveWaypoint, removeWaypoint, getRoute};
5 changes: 5 additions & 0 deletions src/styles/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -3835,6 +3835,11 @@ const styles = {
overflow: 'hidden',
},

mapDirection: {
width: 7,
color: Colors.green,
},

mapPendingView: {
backgroundColor: themeColors.highlightBG,
...flex.flex1,
Expand Down
Loading