[HOLD for payment 2024-11-29] [$250] Update second `Allow location access` modal on web #50601

jamesdeanexpensify · 2024-10-10T21:35:50Z

On web only, for the second Allow location access modal only (the one with the Got it button):

Fix the bug where you can't submit after clicking Got it (see video below)
Fix the bug where, after clicking the Got it button, the same button quickly flickers to say Continue (same video below)
Update the copy on the modal slightly to:

Allow location access
Location access helps us keep your timezone and currency accurate wherever you go. Please allow location access from your device's permission settings.

Remove the Not now button entirely because it serves no purpose in this case

2024-10-10_09-11-41.mp4

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021847227311681373401
Upwork Job ID: 1847227311681373401
Last Price Increase: 2024-10-25
Automatic offers:

alitoshmatov | Reviewer | 104624114
truph01 | Contributor | 104624115

Issue Owner

Current Issue Owner: @alitoshmatov

The text was updated successfully, but these errors were encountered:

melvin-bot · 2024-10-10T21:36:01Z

Triggered auto assignment to @Christinadobrzyn (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.

truph01 · 2024-10-11T01:20:12Z

Edited by proposal-police: This proposal was edited at 2024-10-17 18:30:45 UTC.

Proposal

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

Remove "Allow location access" modal on web

What is the root cause of that problem?

When pressing on submit button, we check whether we should start a permission flow:

App/src/pages/iou/request/step/IOURequestStepConfirmation.tsx

Line 592 in 7bec576

if (shouldStartLocationPermissionFlow) {

if so, the startPermissionFlow is set to true then this useEffect is called:

App/src/components/LocationPermissionModal/index.tsx

Lines 22 to 36 in 4c90d62

    
           useEffect(() => { 
        
               if (!startPermissionFlow) { 
        
                   return; 
        
               } 
        
               getLocationPermission().then((status) => { 
        
                   if (status === RESULTS.GRANTED || status === RESULTS.LIMITED) { 
        
                       return onGrant(); 
        
                   } 
        
                   setShowModal(true); 
        
                   setHasError(status === RESULTS.BLOCKED); 
        
               }); 
        
               // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps -- We only want to run this effect when startPermissionFlow changes 
        
           }, [startPermissionFlow]);

First, it checks the current permission. If it is RESULTS.GRANTED or RESULTS.LIMITED, it don't show any modal. Otherwise, the modal is shown.
This modal is to:

pre-prompt the user for their location access instead of using the native prompt from Apple/Google that would penalize you if you asked for location access too many times. This way, we can prompt users with our own modal on mobile to get a response before the native prompt, avoiding any penalties if the answer is "no" but still allowing us to ask as many times as we want.

When user chooses "Confirm" button, then the native prompt is shown. In this issue, we are going to remove pre-prompt.

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

As I mentioned above, In this issue, we are going to remove pre-prompt. So the permission flow will be: Checking existing permission > displaying native prompt > sending expense instead of Checking existing permission > display custom pre-prompt > displaying native prompt > sending expense.
To match the new flow, we can create a new index.website.ts, its logic is the same as index.ts, just need to update the useEffect:

App/src/components/LocationPermissionModal/index.tsx

Lines 22 to 36 in 4c90d62

    
           useEffect(() => { 
        
               if (!startPermissionFlow) { 
        
                   return; 
        
               } 
        
               getLocationPermission().then((status) => { 
        
                   if (status === RESULTS.GRANTED || status === RESULTS.LIMITED) { 
        
                       return onGrant(); 
        
                   } 
        
                   setShowModal(true); 
        
                   setHasError(status === RESULTS.BLOCKED); 
        
               }); 
        
               // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps -- We only want to run this effect when startPermissionFlow changes 
        
           }, [startPermissionFlow]);

useEffect(() => {
    if (!startPermissionFlow) {
        return;
    }

    getLocationPermission().then((status) => {
        if (status === RESULTS.GRANTED || status === RESULTS.LIMITED) {
            return onGrant();
        }
        
        // If the user has previously denied the permission, we still need to show a modal
        // to inform them that they must go to the settings page to enable location permission.
        if (status === RESULTS.BLOCKED) {
            setShowModal(true);
            setHasError(status === RESULTS.BLOCKED);
        } else {
            // If the native permission prompt hasn't been shown before, show it now.
            grantLocationPermission();
        }
    });
    
    // eslint-disable-next-line react-hooks/exhaustive-deps -- We only want this effect to run when startPermissionFlow changes.
}, [startPermissionFlow]);

What alternative solutions did you explore? (Optional)

Solution for new design:

Update:

App/src/components/LocationPermissionModal/index.tsx

Line 85 in 4c90d62

<ConfirmModal

        <ConfirmModal
            shouldShowCancelButton={!(isWeb && hasError)}
            onModalHide={() => {
                setHasError(false);
                resetPermissionFlow();
            }}

Update:

App/src/components/LocationPermissionModal/index.tsx

Lines 38 to 49 in 4c90d62

    
           const handledBlockedPermission = (cb: () => void) => () => { 
        
               if (hasError) { 
        
                   if (Linking.openSettings) { 
        
                       Linking.openSettings(); 
        
                   } 
        
                   setShowModal(false); 
        
                   setHasError(false); 
        
                   resetPermissionFlow(); 
        
                   return; 
        
               } 
        
               cb(); 
        
           };

    const handledBlockedPermission = (cb: () => void) => () => {
        if (hasError) {
            if (Linking.openSettings) {
                Linking.openSettings();
            } else {
                onDeny?.();
            }
            setShowModal(false);
            return;
        }
        cb();
    };

Update the title:

App/src/components/LocationPermissionModal/index.tsx

Line 94 in 4c90d62

    
           title={translate(hasError ? 'receipt.locationErrorTitle' : 'receipt.locationAccessTitle')}

and prompt:

App/src/components/LocationPermissionModal/index.tsx

Line 92 in 4c90d62

    
           prompt={translate(hasError ? 'receipt.locationErrorMessage' : 'receipt.locationAccessMessage')}

to match the new design:

Allow location access
Location access helps us keep your timezone and currency accurate wherever you go. Please allow location access from your device's permission settings.

It is very straightforward:

            prompt={translate(hasError ? (isWeb ? {new_prompt} : 'receipt.locationErrorMessage') : 'receipt.locationAccessMessage')}
            title={translate(hasError ? (isWeb ? {new_title} : 'receipt.locationErrorTitle') : 'receipt.locationAccessTitle')}

truph01 · 2024-10-11T01:27:48Z

@jamesdeanexpensify We don't want to display the pre-prompt but still display the native prompt on web, no?

jamesdeanexpensify · 2024-10-11T17:28:46Z

Yes, we would still show the native prompt on web. Thanks!

twilight2294 · 2024-10-11T19:29:58Z

Edited by proposal-police: This proposal was edited at 2024-10-15 11:18:52 UTC.

Proposal

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

Remove "Allow location access" modal on web

What is the root cause of that problem?

Feature request

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

New Solution according to expected results:

Call skipLocationPermission when we click got it on web:
Introduce shouldShowCancelButton prop to ConfirmModal:

And update the locationErrorMessage message to Location access helps us keep your timezone and currency accurate wherever you go. Please allow location access from your device's permission settings., if this doesn't fit screen then minor style changes would also be needed. But

        <ConfirmModal
            onConfirm={(hasError && isWeb) ? skipLocationPermission : grantLocationPermission}
            shouldShowCancelButton={!(hasError && isWeb)}

These changes are enough to match the expected results in this case

truph01 · 2024-10-11T23:02:01Z

@jamesdeanexpensify @Julesssss In my opinion, I don’t think we can fully hide the modal

Previously, when the user clicked the submit button and the native prompt hadn’t been shown yet, we displayed the pre-prompt modal:

If the user chose 'Continue,' but location permission wasn’t granted, the native prompt wouldn’t appear in the future. That's why, the next time, we display a modal to inform them that they need to manually enable location permission:

So, the 2nd modal should not be hidden.

melvin-bot · 2024-10-14T18:42:44Z

@Julesssss, @Christinadobrzyn Whoops! This issue is 2 days overdue. Let's get this updated quick!

Christinadobrzyn · 2024-10-15T03:02:47Z

Thanks for the feedback @truph01!

Sorry, just catching up here, can you confirm the difference you are proposing? I think it's that we only show the "Not Now"/"Got it" prompt on the web and not the pre-prompt? Is that right?

truph01 · 2024-10-15T03:07:47Z

I think it's that we only show the "Not Now"/"Got it" prompt on the web and not the pre-prompt

Yes

Christinadobrzyn · 2024-10-16T06:07:33Z

I think this makes sense - do you @Julesssss @jamesdeanexpensify? Do you think we need a consensus from the team about this?

Julesssss · 2024-10-16T12:06:14Z

That seems to be what we agreed, but lets check in with James first.

jamesdeanexpensify · 2024-10-16T19:42:10Z

We're discussing this one internally and may recommend an alternative course of action. Stay tuned!

jamesdeanexpensify · 2024-10-16T20:15:12Z

Just for alignment purposes - by "native prompt" on web, are we all talking about the second screenshot here?

twilight2294 · 2024-10-16T20:17:59Z

Just for alignment purposes - by "native prompt" on web, are we all talking about the second screenshot here?

Yes

twilight2294 · 2024-10-16T20:41:13Z

The video you attached is not being played, can you add another

twilight2294 · 2024-10-16T20:43:24Z

I actually think on web we can probably leave these prompts entirely in place, but (again, on web only):

@jamesdeanexpensify this means that we only show the prompt on native devices right?

jamesdeanexpensify · 2024-10-16T20:54:47Z

Hmmm...that video is working for me. Here it is again:

2024-10-10_09-11-41.mp4

And sorry, what do you mean by "only show the prompt on native devices"?

truph01 · 2024-10-16T23:43:13Z

Just for alignment purposes - by "native prompt" on web, are we all talking about the second screenshot #50601 (comment)?

When users sign-in on a new device and are not asked about the location permission before. If they create a scan expense, the modal below is displayed, it is "pre-prompt":

If the user chooses "Continue", the modal below is displayed (in top left screen), it is "native prompt":

If the user chooses "Block", and then creates an other expense, the modal below is displayed:

If the user chooses "Got it", the modal will close.

jamesdeanexpensify · 2024-10-17T00:24:31Z

Ah, that's super helpful! Bringing this back to the team to chat about next steps again. Thanks!

jamesdeanexpensify · 2024-10-17T17:53:35Z

OK, here is where we landed:

On web only, for the second Allow location access modal only (the one with the Got it button):

Fix the bug where you can't submit after clicking Got it (see video below)
Fix the bug where, after clicking the Got it button, the same button quickly flickers to say Continue (same video below)
Update the copy on the modal slightly to:

Allow location access
Location access helps us keep your timezone and currency accurate wherever you go. Please allow location access from your device's permission settings.

Remove the Not now button entirely because it serves no purpose in this case

2024-10-10_09-11-41.mp4

twilight2294 · 2024-10-17T18:22:50Z

@jamesdeanexpensify so by only web we mean that there should be no change to the behaviour on native platforms right ?

melvin-bot · 2024-11-22T19:47:51Z

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

melvin-bot · 2024-11-22T19:47:55Z

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

fix: Update second Allow location access modal on web #51709

If no regressions arise, payment will be issued on 2024-11-29. 🎊

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

@alitoshmatov requires payment automatic offer (Reviewer)
@truph01 requires payment automatic offer (Contributor)

melvin-bot · 2024-11-22T19:47:56Z

@alitoshmatov @Christinadobrzyn @alitoshmatov The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

melvin-bot · 2024-12-02T09:10:56Z

@Julesssss, @Christinadobrzyn, @alitoshmatov, @truph01 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

alitoshmatov · 2024-12-02T13:54:50Z

BugZero Checklist:

[Contributor] Classify the bug:

Bug classification

Source of bug:

1a. Result of the original design (eg. a case wasn't considered)
1b. Mistake during implementation
1c. Backend bug
1z. Other: Not bug, feature extension

Where bug was reported:

2a. Reported on production
2b. Reported on staging (deploy blocker)
2c. Reported on both staging and production
2d. Reported on a PR
2z. Other:

Who reported the bug:

[Contributor] 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: -
[Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: No need
[Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal Template

[BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

Link to issue: https://github.com/Expensify/Expensify/issues/451657

Regression Test Proposal

NOTE: This test steps are from PR, we have 7 days cooldown for asking location permission in PR we disabled this via code to test, not sure how this condition should be handled here

Precondition:

Clear all the ND's permission in chrome://settings/content/location if has.
Freshly signed in user
We have Forward time in settings step in several places this means you should go to device's settings and change time to future at least 7 days from now

Test:

Test 1: For [Chrome]

FAB > Submit expense > Scan > Upload receipt.
In confirmation page, click "Submit expense". Verify the below modal is displayed:

Choose "Continue". Verify the below modal is displayed:

Choose "Block".
Forward time in settings
FAB > Submit expense > Scan > Upload receipt.
In confirmation page, click "Submit expense". Verify the below modal is displayed:

Choose "Got it". Verify the expense is submitted.

Test 2: For [Safari]

Login in ND in new tab.
FAB > Submit expense > Scan > Upload receipt.
In confirmation page, click "Submit expense". Verify the below modal is displayed:

Choose "Continue". Verify the below modal is displayed:

Choose "Don't allow".
Forward time in settings
FAB > Submit expense > Scan > Upload receipt.
In confirmation page, click "Submit expense". Verify the below modal is displayed:

Choose "Continue". Verify the expense is submitted.

Test 3: For Android/IOS app

FAB > Submit expense > Scan > Upload receipt.
In confirmation page, click "Submit expense". Verify the below modal is displayed:

Choose "Continue". Verify the below modal is displayed:

Choose "Don't Allow".
Forward time in settings
FAB > Submit expense > Scan > Upload receipt.
In confirmation page, click "Submit expense". Verify the below modal is displayed:

Choose "Settings". Verify app redirect to setting screen.

Do we agree 👍 or 👎

Christinadobrzyn · 2024-12-02T19:36:02Z

sorry @alitoshmatov can you update the images above so they are visible? I can't seem to access them.

alitoshmatov · 2024-12-03T17:26:31Z

@Christinadobrzyn Can you recheck it

Christinadobrzyn · 2024-12-03T17:27:24Z

Thanks @alitoshmatov can you just check this - I tested again and the popup isn't showing at all.

Log into New Expensify on Mac Chrome (in a new account)
Click Fab > Submit Expense
Click Distance expense
Click Start > Use Current Location
Click on link in the “It looks like you’ve denied access to your location. Please allow location access in settings and try again.” message
Notice you’re redirected to https://help.expensify.com/, not a specific article.
There's no popup.

2024-12-03_10-10-35.mp4

Christinadobrzyn · 2024-12-04T15:34:16Z

Just checking @alitoshmatov - can you review ^. The Allow Location popup is no longer showing, is that expected?

alitoshmatov · 2024-12-04T16:14:02Z

@Christinadobrzyn You have to upload a receipt not create a distance expanse

alitoshmatov · 2024-12-04T16:14:38Z

Also there is this point to consider:

NOTE: This test steps are from PR, we have 7 days cooldown for asking location permission in PR we disabled this via code to test, not sure how this condition should be handled here

Christinadobrzyn · 2024-12-05T20:32:55Z

@alitoshmatov sorry can you provide more context on that point, as in, do you think we should discuss changing something with the team related to that cooldown period or are you just thinking how that should be documented in the regression test?

alitoshmatov · 2024-12-06T21:30:38Z

@Christinadobrzyn I think you can change computer/phone's time manually on the settings to bypass 7 day cooldown. Just checked it and it works.

Updated QA to show when they should change device's time with Forward time in settings step.
Feel free to update testing steps to be more explanatory

JmillsExpensify · 2024-12-09T10:28:02Z

Awaiting payment summary

Christinadobrzyn · 2024-12-09T15:43:43Z

Thanks @alitoshmatov! Created a regression test with this information.

It look like payment is through Upwork according to this payment summary.

Contributor: @truph01 paid $250 via Upwork (paid here - https://www.upwork.com/nx/wm/workroom/38828652/overview)
Contributor+: @alitoshmatov owed $250 via Upwork (Paid here - https://www.upwork.com/nx/wm/workroom/38831119/overview)

Closing out as complete!

jamesdeanexpensify added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 10, 2024

melvin-bot bot assigned Christinadobrzyn Oct 10, 2024

Julesssss self-assigned this Oct 11, 2024

Christinadobrzyn added this to #expensify-bugs Oct 14, 2024

melvin-bot bot added the Overdue label Oct 14, 2024

melvin-bot bot removed the Overdue label Oct 15, 2024

jamesdeanexpensify changed the title ~~Remove "Allow location access" modal on web~~ Updated second Allow location access modal on web Oct 17, 2024

jamesdeanexpensify changed the title ~~Updated second Allow location access modal on web~~ Update second Allow location access modal on web Oct 17, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production labels Nov 22, 2024

melvin-bot bot changed the title ~~[$250] Update second Allow location access modal on web~~ [HOLD for payment 2024-11-29] [$250] Update second Allow location access modal on web Nov 22, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Nov 22, 2024

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Nov 29, 2024

melvin-bot bot removed the Overdue label Dec 2, 2024

garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in #expensify-bugs Dec 3, 2024

melvin-bot bot added the Overdue label Dec 6, 2024

melvin-bot bot removed the Overdue label Dec 6, 2024

melvin-bot bot added the Overdue label Dec 9, 2024

Christinadobrzyn closed this as completed Dec 9, 2024

melvin-bot bot removed the Overdue label Dec 9, 2024

github-project-automation bot moved this from Hold for Payment to Done in #expensify-bugs Dec 9, 2024

[HOLD for payment 2024-11-29] [$250] Update second Allow location access modal on web #50601

[HOLD for payment 2024-11-29] [$250] Update second Allow location access modal on web #50601

Comments

jamesdeanexpensify commented Oct 10, 2024 • edited by Christinadobrzyn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

melvin-bot bot commented Oct 10, 2024

Uh oh!

truph01 commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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)

Uh oh!

truph01 commented Oct 11, 2024

Uh oh!

jamesdeanexpensify commented Oct 11, 2024

Uh oh!

twilight2294 commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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?

Uh oh!

truph01 commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melvin-bot bot commented Oct 14, 2024

Uh oh!

Christinadobrzyn commented Oct 15, 2024

Uh oh!

truph01 commented Oct 15, 2024

Uh oh!

Christinadobrzyn commented Oct 16, 2024

Uh oh!

Julesssss commented Oct 16, 2024

Uh oh!

jamesdeanexpensify commented Oct 16, 2024

Uh oh!

jamesdeanexpensify commented Oct 16, 2024

Uh oh!

twilight2294 commented Oct 16, 2024

Uh oh!

twilight2294 commented Oct 16, 2024

Uh oh!

twilight2294 commented Oct 16, 2024

Uh oh!

jamesdeanexpensify commented Oct 16, 2024

Uh oh!

truph01 commented Oct 16, 2024

Uh oh!

jamesdeanexpensify commented Oct 17, 2024

Uh oh!

jamesdeanexpensify commented Oct 17, 2024

Uh oh!

twilight2294 commented Oct 17, 2024

Uh oh!

melvin-bot bot commented Nov 22, 2024

Uh oh!

melvin-bot bot commented Nov 22, 2024

Uh oh!

melvin-bot bot commented Nov 22, 2024

Uh oh!

melvin-bot bot commented Dec 2, 2024

Uh oh!

alitoshmatov commented Dec 2, 2024 • edited by Christinadobrzyn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

BugZero Checklist:

Regression Test Proposal

Precondition:

Test:

Uh oh!

Christinadobrzyn commented Dec 2, 2024

Uh oh!

alitoshmatov commented Dec 3, 2024

Uh oh!

Christinadobrzyn commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

[HOLD for payment 2024-11-29] [$250] Update second `Allow location access` modal on web #50601

[HOLD for payment 2024-11-29] [$250] Update second `Allow location access` modal on web #50601

jamesdeanexpensify commented Oct 10, 2024 •

edited by Christinadobrzyn

Loading

truph01 commented Oct 11, 2024 •

edited

Loading

twilight2294 commented Oct 11, 2024 •

edited

Loading

truph01 commented Oct 11, 2024 •

edited

Loading

alitoshmatov commented Dec 2, 2024 •

edited by Christinadobrzyn

Loading

Christinadobrzyn commented Dec 3, 2024 •

edited

Loading