Skip to content

Commit 9b917f2

Browse files
mananjadhavmananjadhav
committed
Merge branch 'main' of github.com:mananjadhav/App into mj-78130-hold-modal
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # src/components/MoneyRequestHeaderSecondaryActions.tsx # src/hooks/useHoldRejectActions.ts
2 parents e2b06df + 763218c commit 9b917f2

867 files changed

Lines changed: 25596 additions & 9395 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/skills/coding-standards/SKILL.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ Coding standards for the Expensify App. Each standard is a standalone file in `r
3636
- [PERF-15](rules/perf-15-cleanup-async-effects.md) — Clean up async Effects
3737
- [PERF-16](rules/perf-16-guard-double-init.md) — Guard double initialization
3838
- [PERF-17](rules/perf-17-pass-raw-index-on-demand.md) — Pass raw source, index on demand (no pre-built digest)
39+
- [PERF-18](rules/perf-18-use-pre-mount-destination.md) — Use usePreMountDestination for RHP pre-mounting
3940

4041
### Consistency
4142
- [CONSISTENCY-1](rules/consistency-1-no-platform-checks.md) — No platform-specific checks in components

.claude/skills/coding-standards/rules/perf-17-avoid-intermediate-lookup.md renamed to .claude/skills/coding-standards/rules/perf-17-pass-raw-index-on-demand.md

File renamed without changes.

‎.claude/skills/coding-standards/rules/perf-18-use-pre-mount-destination.md‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
---
2+
ruleId: PERF-18
3+
title: Use usePreMountDestination for RHP-to-fullscreen pre-mounting
4+
---
5+
6+
## [PERF-18] Use usePreMountDestination for RHP-to-fullscreen pre-mounting
7+
8+
### Reasoning
9+
10+
Modal-to-destination flows need the destination mounted before the RHP dismisses. Otherwise users see a gap on narrow layout or a flash of the previous page on wide layout.
11+
12+
`usePreMountDestination` centralizes this lifecycle:
13+
14+
- Idle-priority pre-insert on narrow layout, with a fallback timer so the work is not starved
15+
- Reveal-before-dismiss fallback on wide layout or if narrow pre-insert has not finished
16+
- Automatic cleanup for back-out and unmount paths
17+
18+
When reviewing these flows, focus on whether the navigation lifecycle is correct for the user path:
19+
20+
- The destination route is stable at mount time
21+
- `reveal()` is called only after validation, synchronous writes, and target-route selection are complete
22+
- The caller handles flow-specific no-op cases before calling `reveal()`
23+
- Cleanup or preservation is handled on back-out and unmount
24+
25+
### Incorrect
26+
27+
```tsx
28+
const destinationRoute = buildDestinationRoute(itemID);
29+
const {reveal} = usePreMountDestination(destinationRoute);
30+
31+
const handleSubmit = () => {
32+
reveal(() => {
33+
saveDataRequiredByDestination();
34+
});
35+
};
36+
```
37+
38+
### Correct
39+
40+
```tsx
41+
const destinationRoute = buildDestinationRoute(itemID);
42+
const {reveal} = usePreMountDestination(destinationRoute);
43+
44+
const handleSubmit = () => {
45+
saveDataRequiredByDestination();
46+
reveal();
47+
};
48+
```
49+
50+
---
51+
52+
### Hook API
53+
54+
**Reveal methods:**
55+
56+
- `reveal(afterTransition?)`: if the hook owns a pre-inserted narrow route, clears the pre-insert flag and dismisses the RHP over that route. Otherwise, inserts the destination under the RHP and then dismisses it.
57+
- `cleanupPreMount()`: removes the owned pre-inserted destination before a back-out path closes the RHP without revealing the destination.
58+
59+
**Caller responsibilities:**
60+
61+
- Keep flow-specific checks and synchronous work outside the hook.
62+
- Handle no-op cases before calling `reveal()`, such as when the destination route is already the active fullscreen route behind the modal.
63+
- Pass `reveal(afterTransition)` only for work that must run after the dismiss/reveal transition.
64+
- Call `cleanupPreMount()` on every back-out path that closes the RHP without calling `reveal()`.
65+
66+
**Scheduling:**
67+
68+
- Mount-time pre-insert always waits for the RHP open transition before scheduling idle pre-insert work.
69+
- If no upcoming transition starts within 500ms, the hook proceeds with idle pre-insert scheduling. This only prevents missing the RHP open transition; broader transition timing changes should be discussed separately.
70+
71+
### Review Focus
72+
73+
Prioritize judgment-based issues:
74+
75+
- Whether the flow is actually an RHP/modal-to-different-fullscreen reveal
76+
- Whether the destination is stable and known when the hook mounts
77+
- Whether the caller keeps flow-specific checks and synchronous writes outside `reveal()`
78+
- Whether the caller handles no-op cases where the destination is already active
79+
- Whether a back-out path calls `cleanupPreMount()`
80+
- Whether an unmount-before-submit flow needs `shouldPreservePreInsertedRouteOnUnmount`
81+
82+
### When to use
83+
84+
Use `usePreMountDestination` when **all** of these are true:
85+
86+
- The flow dismisses an RHP/modal to reveal a **different** fullscreen destination
87+
- The destination route is **known at mount time**
88+
- The user spends enough time on the confirmation screen for pre-insert to complete before dismiss (narrow layout)
89+
90+
### When NOT to use
91+
92+
- Destination is not known in advance
93+
- There is no modal/RHP to dismiss
94+
- The destination is already the screen behind the modal
95+
- The transition is already fast enough. Profile first, do not add complexity speculatively
96+
- Flow-specific dismiss strategies that do not use pre-insert/reveal. Keep those helpers
97+
98+
### Review Metadata
99+
100+
Flag when:
101+
102+
- `usePreMountDestination` is used in a flow that does not dismiss an RHP/modal to reveal a different fullscreen destination
103+
- `usePreMountDestination` is used with a destination that is not stable/known at mount time
104+
- A caller relies on `reveal(afterTransition)` for work that must happen before navigation, such as validation, target-route selection, or a synchronous write needed before the destination is revealed
105+
- A back-out path closes the RHP without calling `cleanupPreMount()` when the component owns a pre-inserted route
106+
- A submit path unmounts the component before `reveal()` runs but does not preserve the pre-inserted route with `shouldPreservePreInsertedRouteOnUnmount`
107+
- New code reimplements pre-insert timing, back-out cleanup, or reveal-before-dismiss orchestration inline instead of using the hook, even if it avoids a direct `preInsertFullscreenUnderRHP` call
108+
109+
**DO NOT flag if:**
110+
111+
- The code uses `usePreMountDestination` correctly with the matching reveal method for the flow
112+
- The call site is an approved exception (`IOURequestStepConfirmation`, `useSkipConfirmationPreInsert` until migrated)
113+
- The flow uses specialized dismiss helpers that intentionally bypass pre-insert/reveal
114+
115+
**Search patterns:**
116+
117+
- `preInsertFullscreenUnderRHP`
118+
- `usePreMountDestination`
119+
- `revealRouteBeforeDismissingModal`
120+
- `PRE_INSERT_FULLSCREEN_DELAY`

‎.github/scripts/createDocsRoutes.ts‎

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ type Article = {
1212
type Section = {
1313
href: string;
1414
title: string;
15+
order?: number;
1516
articles?: Article[];
1617
sections?: Section[];
1718
};
@@ -23,6 +24,7 @@ type Hub = {
2324
icon: string;
2425
articles?: Article[];
2526
sections?: Section[];
27+
flatSections?: Section[];
2628
};
2729

2830
type Platform = {
@@ -36,6 +38,18 @@ type DocsRoutes = {
3638

3739
type HubEntriesKey = 'sections' | 'articles';
3840

41+
function isRecord(value: unknown): value is Record<string, unknown> {
42+
return typeof value === 'object' && value !== null && !Array.isArray(value);
43+
}
44+
45+
function getOptionalString(value: unknown): string | undefined {
46+
return typeof value === 'string' ? value : undefined;
47+
}
48+
49+
function getOptionalNumber(value: unknown): number | undefined {
50+
return typeof value === 'number' ? value : undefined;
51+
}
52+
3953
const warnMessage = (platform: string): string => `Number of hubs in _routes.yml does not match number of hubs in docs/${platform}/articles. Please update _routes.yml with hub info.`;
4054
const disclaimer = '# This file is auto-generated. Do not edit it directly. Use npm run createDocsRoutes instead.\n';
4155
const docsDir = `${process.cwd()}/docs`;
@@ -109,24 +123,55 @@ function getOrderFromArticleFrontMatter(path: string): number | undefined {
109123
if (!frontmatter) {
110124
return undefined;
111125
}
112-
const frontmatterObject = yaml.load(frontmatter) as Record<string, unknown>;
113-
return frontmatterObject.order as number | undefined;
126+
const frontmatterObject = yaml.load(frontmatter);
127+
if (!isRecord(frontmatterObject)) {
128+
return undefined;
129+
}
130+
return getOptionalNumber(frontmatterObject.order);
114131
} catch {
115132
return undefined;
116133
}
117134
}
118135

136+
function getSectionMeta(sectionPath: string): {title?: string; order?: number} {
137+
try {
138+
const metaPath = `${sectionPath}/_meta.yml`;
139+
if (!fs.existsSync(metaPath)) {
140+
return {};
141+
}
142+
const meta = yaml.load(fs.readFileSync(metaPath, 'utf8'));
143+
if (!isRecord(meta)) {
144+
return {};
145+
}
146+
return {
147+
title: getOptionalString(meta.title),
148+
order: getOptionalNumber(meta.order),
149+
};
150+
} catch {
151+
return {};
152+
}
153+
}
154+
155+
function sortSectionsByOrder(sections: Section[]) {
156+
sections.sort((a, b) => (a.order ?? Number.POSITIVE_INFINITY) - (b.order ?? Number.POSITIVE_INFINITY));
157+
}
158+
119159
/**
120160
* Build a section from a directory path, with optional parent path for nested href
121161
*/
122162
function buildSection(platformName: string, hub: string, sectionPath: string, parentHref: string): Section {
123163
const sectionName = sectionPath.split('/').pop() ?? sectionPath;
124164
const fullPath = `${docsDir}/articles/${platformName}/${hub}/${sectionPath}`;
165+
const meta = getSectionMeta(fullPath);
125166
const articles: Article[] = [];
126167
const childSections: Section[] = [];
127168
const href = parentHref ? `${parentHref}/${sectionName}` : sectionName;
128169

129170
for (const entry of fs.readdirSync(fullPath)) {
171+
if (entry === '_meta.yml') {
172+
continue;
173+
}
174+
130175
const entryPath = `${fullPath}/${entry}`;
131176
if (entry.endsWith('.md')) {
132177
const order = getOrderFromArticleFrontMatter(entryPath);
@@ -140,9 +185,12 @@ function buildSection(platformName: string, hub: string, sectionPath: string, pa
140185
// The sort is stable, so articles without an `order` keep their relative position and fall after ordered ones.
141186
articles.sort((a, b) => (a.order ?? Number.POSITIVE_INFINITY) - (b.order ?? Number.POSITIVE_INFINITY));
142187

188+
sortSectionsByOrder(childSections);
189+
143190
const section: Section = {
144191
href,
145-
title: toTitleCase(sectionName.replaceAll('-', ' ')),
192+
title: meta.title ?? toTitleCase(sectionName.replaceAll('-', ' ')),
193+
...(meta.order !== undefined && {order: meta.order}),
146194
...(articles.length > 0 && {articles}),
147195
...(childSections.length > 0 && {sections: childSections}),
148196
};
@@ -175,7 +223,9 @@ function createHubsWithArticles(hubs: string[], platformName: ValueOf<typeof pla
175223

176224
for (const fileOrFolder of fs.readdirSync(basePath)) {
177225
if (fileOrFolder.endsWith('.md')) {
178-
const articleObj = getArticleObj(fileOrFolder);
226+
const entryPath = `${basePath}/${fileOrFolder}`;
227+
const order = getOrderFromArticleFrontMatter(entryPath);
228+
const articleObj = getArticleObj(fileOrFolder, order);
179229
pushOrCreateEntry(routeHubs, hub, 'articles', articleObj);
180230
continue;
181231
}
@@ -188,7 +238,8 @@ function createHubsWithArticles(hubs: string[], platformName: ValueOf<typeof pla
188238
// Add flat section list for nested section page lookup
189239
const hubObj = routeHubs.find((obj) => obj.href === hub);
190240
if (hubObj?.sections?.length) {
191-
(hubObj as Hub & {flatSections?: Section[]}).flatSections = flattenSections(hubObj.sections);
241+
sortSectionsByOrder(hubObj.sections);
242+
hubObj.flatSections = flattenSections(hubObj.sections);
192243
}
193244
}
194245
}

‎.github/scripts/generateAllowedUrls.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import path from 'path';
55
* Script to extract all URLs from help articles and generate a whitelist.
66
* Run this at build time to update the allowed URLs list.
77
*
8-
* Usage: npx ts-node .github/scripts/generateAllowedUrls.ts
8+
* Usage: bun .github/scripts/generateAllowedUrls.ts
99
*/
1010

1111
const DOCS_DIR = path.join(__dirname, '..', '..', 'docs');

‎.github/workflows/createDeployChecklist.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,6 @@ jobs:
2222
uses: ./.github/actions/composite/setupNode
2323

2424
- name: Create or update deploy checklist
25-
run: npx ts-node scripts/createOrUpdateDeployChecklist.ts
25+
run: npx bun scripts/createOrUpdateDeployChecklist.ts
2626
env:
2727
GITHUB_TOKEN: ${{ secrets.OS_BOTIFY_TOKEN }}

‎.github/workflows/deploy.yml‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,26 @@ jobs:
358358
env:
359359
BROWSERSTACK: ${{ secrets.BROWSERSTACK }}
360360

361+
- name: Setup 1Password CLI
362+
uses: Expensify/GitHub-Actions/setup-certificate-1p@main
363+
with:
364+
OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}
365+
SHOULD_LOAD_SSL_CERTIFICATES: 'false'
366+
367+
- name: Load BrowserStack Automate credentials from 1Password
368+
id: load-browserstack-automate
369+
# v4.0.0
370+
uses: 1password/load-secrets-action@92467eb28f72e8255933372f1e0707c567ce2259
371+
with:
372+
export-env: false
373+
env:
374+
OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}
375+
BROWSERSTACK_AUTOMATE: op://${{ vars.OP_VAULT }}/BROWSERSTACK_AUTOMATE/notesPlain
376+
377+
- name: Upload Android build to BrowserStack Automate
378+
if: ${{ fromJSON(env.IS_APP_REPO) }}
379+
run: curl -u "${{ steps.load-browserstack-automate.outputs.BROWSERSTACK_AUTOMATE }}" -X POST "https://api-cloud.browserstack.com/app-automate/upload" -F "file=@${{ steps.find-apk.outputs.APK_PATH }}"
380+
361381
androidUploadApplause:
362382
name: Upload Android to Applause
363383
needs: [prep, androidBuild]
@@ -588,6 +608,26 @@ jobs:
588608
env:
589609
BROWSERSTACK: ${{ secrets.BROWSERSTACK }}
590610

611+
- name: Setup 1Password CLI
612+
uses: Expensify/GitHub-Actions/setup-certificate-1p@main
613+
with:
614+
OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}
615+
SHOULD_LOAD_SSL_CERTIFICATES: 'false'
616+
617+
- name: Load BrowserStack Automate credentials from 1Password
618+
id: load-browserstack-automate
619+
# v4.0.0
620+
uses: 1password/load-secrets-action@92467eb28f72e8255933372f1e0707c567ce2259
621+
with:
622+
export-env: false
623+
env:
624+
OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}
625+
BROWSERSTACK_AUTOMATE: op://${{ vars.OP_VAULT }}/BROWSERSTACK_AUTOMATE/notesPlain
626+
627+
- name: Upload iOS build to BrowserStack Automate
628+
if: ${{ fromJSON(env.IS_APP_REPO) }}
629+
run: curl -u "${{ steps.load-browserstack-automate.outputs.BROWSERSTACK_AUTOMATE }}" -X POST "https://api-cloud.browserstack.com/app-automate/upload" -F "file=@${{ steps.find-ipa.outputs.IPA_PATH }}"
630+
591631
iosUploadApplause:
592632
name: Upload iOS to Applause
593633
needs: [prep, iosBuild]
@@ -895,6 +935,35 @@ jobs:
895935
echo "IS_RELEASE_READY=$isReleaseReady" >> "$GITHUB_OUTPUT"
896936
echo "IS_RELEASE_READY is $isReleaseReady (VCR result: ${{ needs.victoryChartRendererBuild.result }})"
897937
938+
autoRetestRequestForCP:
939+
name: File retest request for cherry-picked deploy-blocker fixes
940+
runs-on: blacksmith-2vcpu-ubuntu-2404
941+
# Only for a cherry-pick to staging, once every platform is on staging.
942+
if: ${{ github.repository == 'Expensify/App' && needs.prep.outputs.DEPLOY_ENV == 'staging' && fromJSON(needs.prep.outputs.IS_CHERRY_PICK) && fromJSON(needs.checkDeploymentSuccess.outputs.IS_ALL_PLATFORMS_DEPLOYED) }}
943+
needs: [prep, checkDeploymentSuccess]
944+
steps:
945+
- name: Checkout
946+
uses: useblacksmith/checkout@c9796daa2a4bdebdab5bd16be2c09a70cd4e1121 # v1
947+
948+
- name: Setup Node
949+
uses: ./.github/actions/composite/setupNode
950+
951+
- name: Load retest webhook from 1Password
952+
id: loadWebhook
953+
# v4.0.0
954+
uses: 1password/load-secrets-action@92467eb28f72e8255933372f1e0707c567ce2259
955+
with:
956+
export-env: false
957+
env:
958+
OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}
959+
SLACK_RETEST_WEBHOOK: op://${{ vars.OP_VAULT }}/Repository-Secrets/SLACK_RETEST_WEBHOOK
960+
961+
- name: File retest request
962+
run: npx bun scripts/createRetestRequestForCP.ts --deploy-sha=${{ needs.prep.outputs.DEPLOY_SHA }} --deploy-tag=${{ needs.prep.outputs.TAG }}
963+
env:
964+
GITHUB_TOKEN: ${{ secrets.OS_BOTIFY_TOKEN }}
965+
SLACK_RETEST_WEBHOOK: ${{ steps.loadWebhook.outputs.SLACK_RETEST_WEBHOOK }}
966+
898967
createRelease:
899968
runs-on: blacksmith-2vcpu-ubuntu-2404
900969
if: ${{ always() && fromJSON(needs.checkDeploymentSuccess.outputs.IS_RELEASE_READY) }}

0 commit comments

Comments
 (0)