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.

Sign up for GitHub

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

fix: Distance - Start and Stop labels do not change when dragging one over another. #44748

Merged
merged 7 commits into from
Jul 12, 2024
Merged

fix: Distance - Start and Stop labels do not change when dragging one over another. #44748

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
00d5b26
fix: Distance - Start and Stop labels do not change when dragging one…
Krishna2323 Jul 2, 2024
717e2ec
minor fix.
Krishna2323 Jul 2, 2024
0f16a6c
Merge branch 'Expensify:main' into krishna2323/issue/44153
Krishna2323 Jul 9, 2024
82fc097
minor update.
Krishna2323 Jul 9, 2024
33a04fc
Merge branch 'Expensify:main' into krishna2323/issue/44153
Krishna2323 Jul 10, 2024
e166e19
added isWaypointEmpty to check if a waypoint is empty or not.
Krishna2323 Jul 12, 2024
826e52f
minor fix.
Krishna2323 Jul 12, 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
4 changes: 2 additions & 2 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,8 @@ function initMoneyRequest(reportID: string, policy: OnyxEntry<OnyxTypes.Policy>,
// Add initial empty waypoints when starting a distance expense
if (iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE) {
comment.waypoints = {
waypoint0: {},
waypoint1: {},
waypoint0: {keyForList: 'start_waypoint'},
waypoint1: {keyForList: 'stop_waypoint'},
};
if (!isFromGlobalCreate) {
const customUnitRateID = DistanceRequestUtils.getCustomUnitRateID(reportID);
Expand Down
23 changes: 20 additions & 3 deletions src/pages/iou/request/step/IOURequestStepDistance.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,15 @@ function IOURequestStepDistance({
const {translate} = useLocalize();

const [optimisticWaypoints, setOptimisticWaypoints] = useState<WaypointCollection | null>(null);
const waypoints = useMemo(() => optimisticWaypoints ?? transaction?.comment?.waypoints ?? {waypoint0: {}, waypoint1: {}}, [optimisticWaypoints, transaction]);
const waypoints = useMemo(
() =>
optimisticWaypoints ??
transaction?.comment?.waypoints ?? {
waypoint0: {keyForList: 'start_waypoint'},
waypoint1: {keyForList: 'stop_waypoint'},
},
[optimisticWaypoints, transaction],
);
const waypointsList = Object.keys(waypoints);
const previousWaypoints = usePrevious(waypoints);
const numberOfWaypoints = Object.keys(waypoints).length;
Expand All @@ -92,7 +100,14 @@ function IOURequestStepDistance({
const isRouteAbsentWithoutErrors = !hasRoute && !hasRouteError;
const shouldFetchRoute = (isRouteAbsentWithoutErrors || haveValidatedWaypointsChanged) && !isLoadingRoute && Object.keys(validatedWaypoints).length > 1;
const [shouldShowAtLeastTwoDifferentWaypointsError, setShouldShowAtLeastTwoDifferentWaypointsError] = useState(false);
const nonEmptyWaypointsCount = useMemo(() => Object.keys(waypoints).filter((key) => !isEmpty(waypoints[key])).length, [waypoints]);
const nonEmptyWaypointsCount = useMemo(
() =>
Object.keys(waypoints).filter((key) => {
const {keyForList, ...waypointWithoutKey} = waypoints[key];
return !isEmpty(waypointWithoutKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can split it into a shareable method, that can be used in the below changes as well. A method like isEmptyWaypoint? (we should handle null/undefined inside that method as well)

const nonEmptyWaypointsCount = useMemo(() => Object.keys(waypoints).filter((key) => !isEmptyWaypoint(waypoints[key])).length, [waypoints]);

And

if (isEmptyWaypoint(newWaypoints[`waypoint${index}`]) && !isEmptyWaypoint(waypoints[`waypoint${index}`] ?? {})) {
  emptyWaypointIndex = index;
}

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will check this in few hours.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hoangzinh, suggestion looks good to me, code updated.

}).length,
[waypoints],
);
const duplicateWaypointsError = useMemo(
() => nonEmptyWaypointsCount >= 2 && Object.keys(validatedWaypoints).length !== nonEmptyWaypointsCount,
[nonEmptyWaypointsCount, validatedWaypoints],
Expand Down Expand Up @@ -346,7 +361,9 @@ function IOURequestStepDistance({
data.forEach((waypoint, index) => {
newWaypoints[`waypoint${index}`] = waypoints[waypoint] ?? {};
// Find waypoint that BECOMES empty after dragging
if (isEmpty(newWaypoints[`waypoint${index}`]) && !isEmpty(waypoints[`waypoint${index}`] ?? {})) {
const {keyForList, ...newWaypointsWithoutKey} = newWaypoints[`waypoint${index}`];
const {keyForList: waypointKey, ...waypointWithoutKey} = waypoints[`waypoint${index}`];
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible if waypoints[waypoint${index}] empty here? I mean if there is any case that raise error here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry for delay, I will confirm this today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hoangzinh, I think there won't be any case when waypoints[waypoint${index}] is empty but I have added a if block to make sure that it won't throw any error if there is edge case.

if (isEmpty(newWaypointsWithoutKey) && !isEmpty(waypointWithoutKey ?? {})) {
emptyWaypointIndex = index;
}
});
Expand Down
Loading