[HOLD for payment 2024-07-10][$250] 'You location' button is displayed on distance request thumbnail #43330

m-natarajan · 2024-06-08T13:46:04Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 1.4.80-16
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail: n/a
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

Action Performed:

Navigate to a workspace chat
Make a distance request
Open the IOU report

Expected Result:

There shouldn't be 'Your location' button on the thumbnail

Actual Result:

There is 'Your location' button on the thumbnail

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Bug6506112_1717844094622.Screen_Recording_2024-06-08_at_1.52.06_in_the_afternoon.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~015e9f59317ae04047
Upwork Job ID: 1800208798311565511
Last Price Increase: 2024-06-10
Automatic offers:

hungvu193 | Reviewer | 102701269

Issue Owner

Current Issue Owner: @lschurr

melvin-bot · 2024-06-08T13:46:17Z

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

m-natarajan · 2024-06-08T13:46:28Z

@lschurr FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

m-natarajan · 2024-06-08T13:46:31Z

We think that this bug might be related to #wave-collect - Release 1

Krishna2323 · 2024-06-08T14:08:39Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

'You location' button is displayed on distance request thumbnail

What is the root cause of that problem?

The button is always displayed.

App/src/components/MapView/MapView.tsx

Lines 241 to 257 in 4176e95

    
               <View style={[styles.pAbsolute, styles.p5, styles.t0, styles.r0, {zIndex: 1}]}> 
        
                   <PressableWithoutFeedback 
        
                       accessibilityRole={CONST.ROLE.BUTTON} 
        
                       onPress={centerMap} 
        
                       accessibilityLabel={translate('common.center')} 
        
                   > 
        
                       <View style={styles.primaryMediumIcon}> 
        
                           <Icon 
        
                               width={variables.iconSizeNormal} 
        
                               height={variables.iconSizeNormal} 
        
                               src={Expensicons.Crosshair} 
        
                               fill={theme.icon} 
        
                           /> 
        
                       </View> 
        
                   </PressableWithoutFeedback> 
        
               </View> 
        
           </View>

App/src/components/MapView/MapView.website.tsx

Lines 255 to 271 in 4176e95

    
               <View style={[styles.pAbsolute, styles.p5, styles.t0, styles.r0, {zIndex: 1}]}> 
        
                   <PressableWithoutFeedback 
        
                       accessibilityRole={CONST.ROLE.BUTTON} 
        
                       onPress={centerMap} 
        
                       accessibilityLabel={translate('common.center')} 
        
                   > 
        
                       <View style={styles.primaryMediumIcon}> 
        
                           <Icon 
        
                               width={variables.iconSizeNormal} 
        
                               height={variables.iconSizeNormal} 
        
                               src={Expensicons.Crosshair} 
        
                               fill={theme.icon} 
        
                           /> 
        
                       </View> 
        
                   </PressableWithoutFeedback> 
        
               </View> 
        
           </View>

What changes do you think we should make in order to solve the problem?

We can use the interactive prop to hide the button for both web and native.

Bug 2: User location (blue dot) is also shown in the thumbnail (ConfirmedRoute). If we also want to hide the user location, we can use interactive prop or create a new prop for hiding that.

App/src/components/MapView/MapView.website.tsx

Lines 231 to 237 in 6044241

    
           <Marker 
        
               key="Current-position" 
        
               longitude={currentPosition?.longitude ?? 0} 
        
               latitude={currentPosition?.latitude ?? 0} 
        
           > 
        
               <View style={styles.currentPositionDot} /> 
        
           </Marker>

Bug 3: On confirmation page (also in money request preview), the map focuses on the users current location first and then animates to the waypoints. This happens because currentPosition initial state is set to cachedUserLocation. We set it to the waypoints but until then the map is already loaded. To resolve this, we should use shouldPanMapToCurrentPosition variable to set the initial state of currentPosition accordingly.

App/src/components/MapView/MapView.website.tsx

Line 60 in 6044241

    
           const [currentPosition, setCurrentPosition] = useState(cachedUserLocation ?? initialLocation);

App/src/components/MapView/MapView.website.tsx

Lines 95 to 97 in 6044241

    
           if (!shouldPanMapToCurrentPosition()) { 
        
               setCurrentPositionToInitialState(); 
        
               return;

const shouldPanMapToCurrentPosition = useCallback(() => !userInteractedWithMap && (!waypoints || waypoints.length === 0), [userInteractedWithMap, waypoints]);
const [currentPosition, setCurrentPosition] = useState(shouldPanMapToCurrentPosition() && cachedUserLocation ? cachedUserLocation : initialLocation);

What alternative solutions did you explore? (Optional)

We can also introduce a new prop for hiding the button and pass that prop MoneyRequestView to hide the button, similar to interactive prop.
We can disable the press event on the button and style the cursor according to the prop.

melvin-bot · 2024-06-10T16:50:00Z

Job added to Upwork: https://www.upwork.com/jobs/~015e9f59317ae04047

melvin-bot · 2024-06-10T16:50:05Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @hungvu193 (External)

truph01 · 2024-06-10T23:32:03Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

'You location' button is displayed on distance request thumbnail

What is the root cause of that problem?

We always display 'Your location' button:

App/src/components/MapView/MapView.website.tsx

Line 256 in e66ae37

<PressableWithoutFeedback
And we have a similar logic in MapView.native.tsx as well.

What changes do you think we should make in order to solve the problem?

In this logic:

App/src/components/ReportActionItem/ReportActionItemImage.tsx

Lines 80 to 83 in e66ae37

    
           if (showMapAsImage) { 
        
               return ( 
        
                   <View style={[styles.w100, styles.h100]}> 
        
                       <ConfirmedRoute

We integrate a map view component to preview the distance map while awaiting the backend to provide the map preview in image format for the IOU report. The ConfirmedRoute component is intended to match the output that the backend will eventually deliver. However, in the staging environment, there are significant discrepancies between the behavior of the ConfirmedRoute component and the backend's expected output:

The ConfirmedRoute display the 'Your location' button in the top right (main bug).
The blue dot indicates that it is the current user location.
When the ConfirmedRoute loaded, it focus on the current user location and then, it animates to the routes. But the expected is, ConfirmedRoute always display the routes instead of the current user location initially.

Based on the above, I propose a solution following by the below steps:

Create a new param in MapView (shouldDisplayUserLocation with default value is false for example). We can consider using the existed param, interactive.
Then, we just display this component:

App/src/components/MapView/MapView.website.tsx

Line 255 in e66ae37

<View style={[styles.pAbsolute, styles.p5, styles.t0, styles.r0, {zIndex: 1}]}>

if shouldDisplayUserLocation is true.
Then, just display this component:

App/src/components/MapView/MapView.website.tsx

Line 231 in e66ae37

<Marker

if shouldDisplayUserLocation is true.
Then, an additional condition && shouldDisplayUserLocation to this condition:

App/src/components/MapView/MapView.website.tsx

Line 240 in e66ae37

if (utils.areSameCoordinate([coordinate[0], coordinate[1]], [currentPosition?.longitude ?? 0, currentPosition?.latitude ?? 0])) {
Then, add a new variable in MapView.website.tsx:

 const initialViewState = useMemo(() => {
            if(!shouldDisplayUserLocation){
                if(!waypoints){
                    return undefined
                }
                const {northEast, southWest} = utils.getBounds(
                    waypoints.map((waypoint) => waypoint.coordinate),
                    directionCoordinates,
                );
                return {
                    zoom: initialState.zoom,
                    bounds: [northEast, southWest]
                }
            }
            else {
                return {
                    longitude: currentPosition?.longitude,
                    latitude: currentPosition?.latitude,
                    zoom: initialState.zoom,
                };
            }
        }, [waypoints, directionCoordinates, shouldDisplayUserLocation]);

and use it in this logic instead:

App/src/components/MapView/MapView.website.tsx

Line 222 in e66ae37

initialViewState={{

Finally, use shouldDisplayUserLocation={true} in where want to display the 'Your location' button.

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

Krishna2323 · 2024-06-11T01:04:14Z

Proposal updated

Added solutions for extra bugs mentioned here.

hungvu193 · 2024-06-11T02:22:30Z

Reviewing shortly

hungvu193 · 2024-06-11T02:37:35Z

3. Then, just display this component:

App/src/components/MapView/MapView.website.tsx

Line 231 in e66ae37

<Marker
if `shouldDisplayUserLocation` is `true`.

@truph01 Can you explain why we should only show markers (routes) when shouldDisplayUserLocation is true?
I believe we still want to show routes on the thumbnail.

truph01 · 2024-06-11T02:41:23Z

@hungvu193 That marker is to display the blue dot, not the routes:

App/src/components/MapView/MapView.website.tsx

Lines 231 to 237 in e66ae37

    
           <Marker 
        
               key="Current-position" 
        
               longitude={currentPosition?.longitude ?? 0} 
        
               latitude={currentPosition?.latitude ?? 0} 
        
           > 
        
               <View style={styles.currentPositionDot} /> 
        
           </Marker>

So I mean that we just display the blue dot if shouldDisplayUserLocation is true. The waypoints is still as it is.

hungvu193 · 2024-06-11T02:46:54Z

That marker is to display the blue dot, not the routes:

Oops, my bad I was chasing wrong mentioned code 🥶 .

Thanks for proposals everyone. I believe we want to hide that blue dot along with You location button. (cc @lschurr for confirming this one)
In this case, I'd go with @truph01 's proposal here.
🎀 👀 🎀 C+ reviewed

melvin-bot · 2024-06-11T02:47:06Z

Triggered auto assignment to @mountiny, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

Krishna2323 · 2024-06-11T02:50:26Z

@hungvu193, what do you think about my proposal? My proposal covers all three issues and the other issues are not part of original issues, so I don't think the proposal should be selected based on that. Hiding the blue dot will be same as hiding the your location button so I don't think the selected proposal is different from mine.

hungvu193 · 2024-06-11T03:04:47Z

Hey @Krishna2323, I chose @truph01 proposal because it has more detail about RCA and solution.
I don't think

We can use the interactive prop to hide the button for both web and native.

or

We can also introduce a new prop for hiding the button and pass that prop MoneyRequestView to hide the button, similar to interactive prop.

is detail enough. I know that contributors are always under pressure when posting proposals, but please keep them clear and more understandable 🙏

Krishna2323 · 2024-06-11T03:08:49Z

@hungvu193, I don't see any detailed RCA(same as mine) in the selected proposal and the solution is straight forward so I don't think I have to mention each and every component. I already mentioned we can create a new prop and can be passed similarly to interactive prop. What more details are you looking for?

Krishna2323 · 2024-06-11T03:16:46Z

@hungvu193, If we address the second issue, why not also tackle the third one? I apologize for the argument 🙏🏻, but I believe it is very unfair to assign someone else’s proposal, which uses a similar approach to mine, especially when it is based on something that wasn't part of the original issue. I humbly request a re-evaluation.

cc: @mountiny

hungvu193 · 2024-06-11T03:28:36Z

Sorry to let you down, but if you read selected proposal, you can see the different right?

We integrate a map view component to preview the distance map while awaiting the backend to provide the map preview in image format for the IOU report. The ConfirmedRoute component is intended to match the output that the backend will eventually deliver. However, in the staging environment, there are significant discrepancies between the behavior of the ConfirmedRoute component and the backend's expected output:

This's what I'm looking for.

And I don't think it's straight forward enough to put in that short explanation, as I mentioned here.

Anyway, I'll let @mountiny decide.

Krishna2323 · 2024-06-11T03:41:54Z

@hungvu193, I believe the original issue cares is not showing the center button and my proposal has enough RCA for that, the other proposal tried to point out different issues thats why they included these points in solutions section. I also mentioned about the interactive prop which is used for same purpose (receipt thumbnail). Also, can you pls explain me why aren't we solving the third bug if we are solving the second bug?

Thanks for your responses, I'll wait for @mountiny's views here.

hungvu193 · 2024-06-12T07:00:21Z

I think my above answer is clear enough. However, that's just my thoughts. Since then I raised an internal discussion here: https://app.slack.com/client/E047TPA624F/C02NK2DQWUX
I'll let you know ASAP when we have final decision. Thanks for your patient 🙏

melvin-bot · 2024-06-12T09:35:55Z

📣 @truph01 You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

mountiny · 2024-06-12T09:36:42Z

I agree @truph01 proposal is a bit more detailed and such we are going to proceed with theirs

Thanks everyone for your proposals, let's move on and focus on other bugs/features

@truph01 please raise the pr

truph01 · 2024-06-13T04:51:34Z

@hungvu193 PR #43630 is ready for review.

melvin-bot · 2024-06-25T19:38:40Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2024-06-25T19:38:44Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.1-19 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Fix: Your location button button is displayed on distance request thumbnail #43630

If no regressions arise, payment will be issued on 2024-07-02. 🎊

For reference, here are some details about the assignees on this issue:

@hungvu193 requires payment automatic offer (Reviewer)
@truph01 requires payment (Needs manual offer from BZ)

melvin-bot · 2024-06-25T19:38:45Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@hungvu193] The PR that introduced the bug has been identified. Link to the PR:
[@hungvu193] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@hungvu193] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@hungvu193] Determine if we should create a regression test for this bug.
[@hungvu193] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

lschurr · 2024-07-02T18:09:53Z

Payment summary:

Contributor: @truph01 $250
Contributor+: @hungvu193 $250 (paid in Upwork)

lschurr · 2024-07-02T18:10:08Z

@truph01 - Are you paid via NewDot or is the automation on the comment incorrect?

truph01 · 2024-07-03T04:56:41Z

@lschurr The automation looks incorrect, I usually get paid via Upwork.

My Upwork profile link is https://www.upwork.com/freelancers/~01c880ca813fdd85ec

Can you send me the offer?

lschurr · 2024-07-03T16:25:58Z

Yep! Just sent the offer @truph01

melvin-bot · 2024-07-03T19:29:12Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.3-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Fix: Your location button button is displayed on distance request thumbnail #43630

If no regressions arise, payment will be issued on 2024-07-10. 🎊

For reference, here are some details about the assignees on this issue:

@hungvu193 requires payment automatic offer (Reviewer)
@truph01 requires payment (Needs manual offer from BZ)

melvin-bot · 2024-07-03T19:29:14Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@hungvu193] The PR that introduced the bug has been identified. Link to the PR:
[@hungvu193] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@hungvu193] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@hungvu193] Determine if we should create a regression test for this bug.
[@hungvu193] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

truph01 · 2024-07-09T07:41:37Z

@lschurr Offer accepted TY!

lschurr · 2024-07-10T15:26:38Z

This is complete :)

m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 8, 2024

melvin-bot bot assigned lschurr Jun 8, 2024

lschurr added the External Added to denote the issue can be worked on by a contributor label Jun 10, 2024

melvin-bot bot changed the title ~~'You location' button is displayed on distance request thumbnail~~ [$250] 'You location' button is displayed on distance request thumbnail Jun 10, 2024

melvin-bot bot added Overdue Help Wanted Apply this label when an issue is open to proposals by contributors labels Jun 10, 2024

melvin-bot bot assigned hungvu193 Jun 10, 2024

melvin-bot bot removed the Overdue label Jun 10, 2024

melvin-bot bot assigned mountiny Jun 11, 2024

truph01 mentioned this issue Jun 13, 2024

Fix: Your location button button is displayed on distance request thumbnail #43630

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jun 13, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jun 25, 2024

melvin-bot bot changed the title ~~[$250] 'You location' button is displayed on distance request thumbnail~~ [HOLD for payment 2024-07-02] [$250] 'You location' button is displayed on distance request thumbnail Jun 25, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Jun 25, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 2, 2024

melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jul 3, 2024

melvin-bot bot changed the title ~~[HOLD for payment 2024-07-02] [$250] 'You location' button is displayed on distance request thumbnail~~ [HOLD for payment 2024-07-10] [HOLD for payment 2024-07-02] [$250] 'You location' button is displayed on distance request thumbnail Jul 3, 2024

lschurr changed the title ~~[HOLD for payment 2024-07-10] [HOLD for payment 2024-07-02] [$250] 'You location' button is displayed on distance request thumbnail~~ [HOLD for payment 2024-07-10][$250] 'You location' button is displayed on distance request thumbnail Jul 9, 2024

lschurr closed this as completed Jul 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-07-10][$250] 'You location' button is displayed on distance request thumbnail #43330

[HOLD for payment 2024-07-10][$250] 'You location' button is displayed on distance request thumbnail #43330

m-natarajan commented Jun 8, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jun 8, 2024

m-natarajan commented Jun 8, 2024

m-natarajan commented Jun 8, 2024

Krishna2323 commented Jun 8, 2024 •

edited

Loading

melvin-bot bot commented Jun 10, 2024

melvin-bot bot commented Jun 10, 2024

truph01 commented Jun 10, 2024

Krishna2323 commented Jun 11, 2024

hungvu193 commented Jun 11, 2024

hungvu193 commented Jun 11, 2024 •

edited

Loading

truph01 commented Jun 11, 2024

hungvu193 commented Jun 11, 2024

melvin-bot bot commented Jun 11, 2024

Krishna2323 commented Jun 11, 2024 •

edited

Loading

hungvu193 commented Jun 11, 2024

Krishna2323 commented Jun 11, 2024

Krishna2323 commented Jun 11, 2024

hungvu193 commented Jun 11, 2024 •

edited

Loading

Krishna2323 commented Jun 11, 2024

hungvu193 commented Jun 12, 2024

melvin-bot bot commented Jun 12, 2024

mountiny commented Jun 12, 2024

truph01 commented Jun 13, 2024

melvin-bot bot commented Jun 25, 2024

melvin-bot bot commented Jun 25, 2024

melvin-bot bot commented Jun 25, 2024

lschurr commented Jul 2, 2024

lschurr commented Jul 2, 2024

truph01 commented Jul 3, 2024

lschurr commented Jul 3, 2024

melvin-bot bot commented Jul 3, 2024

melvin-bot bot commented Jul 3, 2024

truph01 commented Jul 9, 2024

lschurr commented Jul 10, 2024

[HOLD for payment 2024-07-10][$250] 'You location' button is displayed on distance request thumbnail #43330

[HOLD for payment 2024-07-10][$250] 'You location' button is displayed on distance request thumbnail #43330

Comments

m-natarajan commented Jun 8, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Jun 8, 2024

m-natarajan commented Jun 8, 2024

m-natarajan commented Jun 8, 2024

Krishna2323 commented Jun 8, 2024 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Jun 10, 2024

melvin-bot bot commented Jun 10, 2024

truph01 commented Jun 10, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

Krishna2323 commented Jun 11, 2024

Proposal updated

hungvu193 commented Jun 11, 2024

hungvu193 commented Jun 11, 2024 • edited Loading

truph01 commented Jun 11, 2024

hungvu193 commented Jun 11, 2024

melvin-bot bot commented Jun 11, 2024

Krishna2323 commented Jun 11, 2024 • edited Loading

hungvu193 commented Jun 11, 2024

Krishna2323 commented Jun 11, 2024

Krishna2323 commented Jun 11, 2024

hungvu193 commented Jun 11, 2024 • edited Loading

Krishna2323 commented Jun 11, 2024

hungvu193 commented Jun 12, 2024

melvin-bot bot commented Jun 12, 2024

mountiny commented Jun 12, 2024

truph01 commented Jun 13, 2024

melvin-bot bot commented Jun 25, 2024

melvin-bot bot commented Jun 25, 2024

melvin-bot bot commented Jun 25, 2024

lschurr commented Jul 2, 2024

lschurr commented Jul 2, 2024

truph01 commented Jul 3, 2024

lschurr commented Jul 3, 2024

melvin-bot bot commented Jul 3, 2024

melvin-bot bot commented Jul 3, 2024

truph01 commented Jul 9, 2024

lschurr commented Jul 10, 2024

m-natarajan commented Jun 8, 2024 •

edited by melvin-bot bot

Loading

Krishna2323 commented Jun 8, 2024 •

edited

Loading

hungvu193 commented Jun 11, 2024 •

edited

Loading

Krishna2323 commented Jun 11, 2024 •

edited

Loading

hungvu193 commented Jun 11, 2024 •

edited

Loading