fix: avoid location prompt cooldown for blocked permission#90345
fix: avoid location prompt cooldown for blocked permission#90345KJ21-ENG wants to merge 7 commits into
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
2d40848 to
8696ab2
Compare
|
@truph01 @flaviadefaria kind ping here |
1 similar comment
|
@truph01 @flaviadefaria kind ping here |
|
@KJ21-ENG Please fix jest unit tests and reassure perf |
|
@truph01 I checked the current CI. All 8 Jest shard jobs are green. The
I do not see a code/test change needed for this PR. Can we rerun the failed Storybook and Reassure jobs? |
|
@truph01 @flaviadefaria kind ping here. Thanks |
1 similar comment
|
@truph01 @flaviadefaria kind ping here. Thanks |
|
@truph01 @flaviadefaria Kind ping here. Thanks |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@truph01 @flaviadefaria Kind ping here. Thanks |
|
@KJ21-ENG Please resolve:
|
…-scan-location-modal
df2edf8 to
b5b13e0
Compare
@truph01 Done. |
Explanation of Change
This PR updates
LocationPermissionModalso its deny callback tells consumers whether the denial came from an explicit app-level user skip or from the browser/OS permission layer refusing the request.When the user explicitly chooses
Not now, consumers still writeNVP_LAST_LOCATION_PERMISSION_PROMPTand preserve the intended 7-day cooldown. When the user tries to continue but browser/OS location permission is blocked or denied, consumers continue without location but do not write the cooldown, so the next scan flow can prompt again.This PR is intentionally still in draft while I complete final manual review before requesting review.
Fixed Issues
$ #89668
PROPOSAL: #89668 (comment)
Tests
Allow location accessmodal appears.Got It/continue while browser location permission remains blocked.Allow location accessmodal appears again instead of being suppressed by the 7-day cooldown.Not nowon the modal.Not nowskip.Automated regression coverage:
npx jest tests/ui/LocationPermissionModalTest.tsx --runInBandNative app recordings are N/A for the original repro because native blocked-permission UI is intentionally different: native shows
Settings+Not now, while the reported browser/mWeb repro uses the blocked geolocationGot itpath. TappingNot nowon native is an explicit skip and should still apply the cooldown.Offline tests
Not applicable. This change only affects client-side location permission prompt state and the local prompt cooldown timestamp. There is no network-dependent behavior change.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionSettings+Not now.Settings+Not now.toggleReportand notonIconClick)src/languages/*files and using the translation method. N/A: no product copy was added or changed.tests/ui/LocationPermissionModalTest.tsx; existing platform-specific files still follow the repo naming convention.STYLE.md) were followed. N/A: no JSDocs were added or changed.Avatar, I verified the components usingAvatarare working as expected)tests/ui/LocationPermissionModalTest.tsxis a focused regression test file and the test names/assertions describe the behavior being covered.StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)). N/A: no CSS styles were added.npm run compress-svg). N/A: no assets were added or modified.Avataris modified, I verified thatAvataris working as expected in all cases). The changed callback contract was updated in all LocationPermissionModal consumers.Designlabel and/or tagged@Expensify/designso the design team can review the changes. N/A: no visual design change.ScrollViewcomponent to make it scrollable when more elements are added to the page. N/A: no page was added.tests/ui/LocationPermissionModalTest.tsx, including explicitNot nowskips vs browser/OS permission denials for the shared and Android modal implementations.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps. N/A: main has not been merged into this PR after review.Screenshots/Videos
Android: Native
N/A. Native blocked-permission state intentionally shows
Settings+Not now; the reported repro is the browser/mWeb blocked geolocationGot itpath.Android: mWeb Chrome
Android_Mweb_1778601841089889.mp4
iOS: Native
N/A. Native blocked-permission state intentionally shows
Settings+Not now; tappingNot nowis an explicit skip and should preserve the cooldown.iOS: mWeb Safari
ios_mweb_1778601961415969.mp4
MacOS: Chrome / Safari
Macos.chrome_1778598041282857.mp4