-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Display Route #25434
Changes from 32 commits
41a3590
a0f270c
e5c55cd
92e4489
3a2f1a0
c3abd93
e03cd93
4201756
1b9056c
02529d3
2c5a9f0
a8fb1c7
0105c37
d67bba4
c928bfd
1cc24c9
b33c36d
2f3be36
d8cf419
679872e
55b5a38
f4a8fce
d6d9d74
944241e
90b941e
090ba17
b542085
9f4d3ab
e32aeb1
0eeb47b
844ee72
2fe1326
4708446
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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; | ||
|
@@ -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 */ | ||
|
@@ -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) => { | ||
|
@@ -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> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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))} | ||
|
@@ -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 | ||
|
@@ -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} | ||
|
@@ -219,7 +251,7 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) { | |
</View> | ||
)} | ||
</View> | ||
</> | ||
</ScreenWrapper> | ||
); | ||
} | ||
|
||
|
@@ -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, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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({ | ||
|
@@ -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, | ||
}, | ||
}, | ||
}, | ||
}); | ||
} | ||
|
||
|
@@ -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, | ||
}, | ||
}, | ||
}, | ||
}); | ||
} | ||
|
||
|
@@ -96,8 +119,72 @@ function removeWaypoint(transactionID, currentIndex) { | |
...transaction.comment, | ||
waypoints: reIndexedWaypoints, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
// 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}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong again @thienlnam. Lets keep this as it was and just change the variable name to
shouldIgnoreFetching
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed it - we don't use the inverse as much so I just adjusted the check (more of a preference thing though)