[Payment due @truph01] Add keyboard shortcuts for searching expense reports and for jumping to a report's policy - #92064
Conversation
…to a report's policy
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index 98cca264500..92292c975be 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -7891,7 +7891,7 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc
copy: 'Kommentar kopieren',
openDebug: 'Testeinstellungen öffnen',
expenseReportSearch: 'Spesenberichte suchen',
- goToWorkspace: 'Zum Arbeitsbereich des aktuellen Berichts wechseln',
+ goToWorkspace: 'Zum Workspace des aktuellen Berichts gehen',
},
},
guides: {
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 8093d20fa64..2de699be44d 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -2814,9 +2814,9 @@ ${amount} pour ${merchant} - ${date}`,
title: 'Modifier l’agent',
agentName: 'Nom de l’agent',
instructions: 'Écrire des instructions personnalisées',
- chatWithAgent: 'Discuter avec l\u2019agent',
+ chatWithAgent: 'Discuter avec l’agent',
copilotIntoAccount: 'Copilote dans le compte',
- deleteAgent: 'Supprimer l\u2019agent',
+ deleteAgent: 'Supprimer l’agent',
deleteAgentTitle: 'Supprimer l’agent ?',
deleteAgentMessage: 'Voulez-vous vraiment supprimer cet agent ? Cette action est irréversible.',
},
@@ -2979,7 +2979,7 @@ ${amount} pour ${merchant} - ${date}`,
phoneOrEmail: 'Téléphone ou e-mail',
error: {
agentSignInBlocked:
- 'Les comptes d\u2019agent ne permettent pas de se connecter directement. Pour utiliser un agent, connectez-vous avec votre propre compte et accédez-y via Copilot.',
+ 'Les comptes d’agent ne permettent pas de se connecter directement. Pour utiliser un agent, connectez-vous avec votre propre compte et accédez-y via Copilot.',
invalidFormatEmailLogin: 'L’adresse e-mail saisie est invalide. Veuillez corriger le format et réessayer.',
},
cannotGetAccountDetails: 'Impossible de récupérer les détails du compte. Veuillez essayer de vous reconnecter.',
@@ -7920,8 +7920,8 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e
newChat: 'Nouvel écran de discussion',
copy: 'Copier le commentaire',
openDebug: 'Ouvrir la boîte de dialogue des préférences de test',
- expenseReportSearch: 'Rechercher des rapports de dépenses',
- goToWorkspace: "Accéder à l'espace de travail du rapport actuel",
+ expenseReportSearch: 'Rechercher des notes de frais',
+ goToWorkspace: "Aller à l'espace de travail de la note de frais actuelle",
},
},
guides: {
diff --git a/src/languages/it.ts b/src/languages/it.ts
index e8d10fd8400..d3db750a398 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -2802,8 +2802,8 @@ ${amount} per ${merchant} - ${date}`,
title: 'Modifica agente',
agentName: 'Nome agente',
instructions: 'Scrivi istruzioni personalizzate',
- chatWithAgent: 'Chatta con l\u2019agente',
- copilotIntoAccount: 'Copilot nell\u2019account',
+ chatWithAgent: 'Chatta con l’agente',
+ copilotIntoAccount: 'Copilot nell’account',
deleteAgent: 'Elimina agente',
deleteAgentTitle: 'Eliminare agente?',
deleteAgentMessage: 'Sei sicuro di voler eliminare questo agente? Questa azione non può essere annullata.',
@@ -7878,8 +7878,8 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`,
newChat: 'Nuova schermata chat',
copy: 'Copia commento',
openDebug: 'Apri la finestra delle preferenze di test',
- expenseReportSearch: 'Cerca report spese',
- goToWorkspace: "Vai all'area di lavoro del report corrente",
+ expenseReportSearch: 'Cerca i report spese',
+ goToWorkspace: 'Vai allo spazio di lavoro del report corrente',
},
},
guides: {
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index a10d8e18326..4bf4925200a 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -7787,7 +7787,7 @@ ${reportName}
copy: 'コメントをコピー',
openDebug: 'テスト設定ダイアログを開く',
expenseReportSearch: '経費レポートを検索',
- goToWorkspace: '現在のレポートのワークスペースへ移動',
+ goToWorkspace: '現在のレポートのワークスペースに移動',
},
},
guides: {
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 07911cca35e..992da0207f6 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -7848,7 +7848,7 @@ er bestedingsregels toe om de kasstroom van het bedrijf te beschermen.`,
newChat: 'Nieuw chatscherm',
copy: 'Opmerking kopiëren',
openDebug: 'Dialoogvenster testvoorkeuren openen',
- expenseReportSearch: 'Onkostenrapporten zoeken',
+ expenseReportSearch: 'Onkostendeclaraties zoeken',
goToWorkspace: 'Ga naar de werkruimte van het huidige rapport',
},
},
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 3e0a62a4e5b..d73488cbc75 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -7838,8 +7838,8 @@ Dodaj więcej zasad wydatków, żeby chronić płynność finansową firmy.`,
newChat: 'Nowy ekran czatu',
copy: 'Skopiuj komentarz',
openDebug: 'Otwórz okno preferencji testowania',
- expenseReportSearch: 'Wyszukaj raporty wydatków',
- goToWorkspace: 'Przejdź do obszaru roboczego bieżącego raportu',
+ expenseReportSearch: 'Szukaj raportów wydatków',
+ goToWorkspace: 'Przejdź do przestrzeni roboczej bieżącego raportu',
},
},
guides: {
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 4e6c09f1d12..619820eb1db 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -7840,8 +7840,8 @@ Adicione mais regras de gasto para proteger o fluxo de caixa da empresa.`,
newChat: 'Nova tela de chat',
copy: 'Copiar comentário',
openDebug: 'Abrir caixa de diálogo de preferências de teste',
- expenseReportSearch: 'Pesquisar relatórios de despesas',
- goToWorkspace: 'Ir para o espaço de trabalho do relatório atual',
+ expenseReportSearch: 'Buscar relatórios de despesas',
+ goToWorkspace: 'Ir para o workspace do relatório atual',
},
},
guides: {
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 0059816e537..fbe1e28f549 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -7640,8 +7640,8 @@ ${reportName}
newChat: '新聊天界面',
copy: '复制评论',
openDebug: '打开测试首选项对话框',
- expenseReportSearch: '搜索费用报告',
- goToWorkspace: '前往当前报告的工作区',
+ expenseReportSearch: '搜索报销报告',
+ goToWorkspace: '前往当前报表的工作区',
},
},
guides: {
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 331d49bfe3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| [PLATFORM_OS_MACOS]: {input: 'u', modifierFlags: keyModifierCommand}, | ||
| [PLATFORM_IOS]: {input: 'u', modifierFlags: keyModifierCommand}, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
These two new shortcuts open another UI or navigate to a workspace, but unlike SEARCH/NEW_CHAT they are not tagged with type: CONST.KEYBOARD_SHORTCUTS_TYPES.NAVIGATION_SHORTCUT. The Plaid connection flows explicitly block only shortcuts with that type (see the shortcutsToBlock filter in AddPlaidBankAccount and the Plaid connection steps), so during a Plaid flow Ctrl/Cmd+U can open search and Ctrl/Cmd+B can navigate away instead of being suppressed, interrupting the bank/card linking flow.
Useful? React with 👍 / 👎.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
| const {openSearchRouter} = useSearchRouterActions(); | ||
| const shouldShowRequire2FAPage = useShouldShowRequire2FAPage(); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
The effect subscribes once with [] deps but the callback captures shouldShowRequire2FAPage from the initial render. If this value changes to true after mount, the keyboard shortcut handler still uses the stale false value and would navigate during the 2FA requirement page.
We can consider using this:
const shouldShowRequire2FAPageRef = useRef(shouldShowRequire2FAPage);
useEffect(() => {
shouldShowRequire2FAPageRef.current = shouldShowRequire2FAPage;
}, [shouldShowRequire2FAPage]);
| if (!reportID) { | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Do we need
if (Navigation.isOnboardingFlow() || shouldShowRequire2FAPage) {
return;
}
here?
There was a problem hiding this comment.
I think not because we will never have a reportID if we are in the onboarding flow or require 2FA page, so we would exist above already in those cases
|
@truph01 please re-review |
|
|
||
| const report = getReportOrDraftReport(reportID); | ||
| const policyID = report?.policyID ?? (report?.parentReportID ? getReportOrDraftReport(report.parentReportID)?.policyID : undefined); | ||
| if (!policyID) { |
There was a problem hiding this comment.
When the user is in the concierge report, the policyID returns _FAKE_, it should be returned early here as well. If not, it will open the not found screen workspaces/_FAKE_/overview
There was a problem hiding this comment.
Also, I think we should check whether the user can access that policy before navigating.
There was a problem hiding this comment.
When the user is in the concierge report, the policyID returns FAKE, it should be returned early here as well. If not, it will open the not found screen workspaces/FAKE/overview
True, updated
Also, I think we should check whether the user can access that policy before navigating.
Meh
There was a problem hiding this comment.
Also, I think we should check whether the user can access that policy before navigating.
What do you think about this?
In this video, the user is viewing a public room. When they press Cmd + B, the app attempts to open the corresponding policy. However, since the user doesn't have access to that policy, they are redirected to the Not Found page.
Screen.Recording.2026-06-04.at.13.19.57.mov
There was a problem hiding this comment.
Yeah, that's fine for me. You are trying to go to a page you don't have access, so we show you that page.
| const shortcutConfig = CONST.KEYBOARD_SHORTCUTS.GO_TO_WORKSPACE; | ||
| const unsubscribe = KeyboardShortcut.subscribe( | ||
| shortcutConfig.shortcutKey, | ||
| () => { |
There was a problem hiding this comment.
I think we should wrap the function in callFunctionIfActionIsAllowed as well
There was a problem hiding this comment.
When an anonymous user is accessing a public room, pressing the shortcut should navigate them to the login screen. It is what we did in ExpenseReportSearchHandler.tsx
| shortcutConfig.shortcutKey, | ||
| () => { | ||
| callFunctionIfActionIsAllowed(() => { | ||
| if (Navigation.isOnboardingFlow() || shouldShowRequire2FAPage) { |
There was a problem hiding this comment.
Should we return early when the search router is already being opened?
There was a problem hiding this comment.
No? I tried pressing CMD+U many times and I don't see any problems with it...
There was a problem hiding this comment.
Could you check this video?
When the Search router is already open and I press Cmd + U, the Search route closes and then immediately reopens.
I think that when the Search route is already open, pressing Cmd + U should do nothing.
Screen.Recording.2026-06-04.at.13.16.22.mov
There was a problem hiding this comment.
I rather have the shortcut work than do nothing... also I don't think this matters much
|
@iwiznia I have two new comments |
melvin-bot
Bot
changed the title
|
🎯 @truph01, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
cead22
left a comment
cead22
left a comment
There was a problem hiding this comment.
QA test should be the same as Tests right?
Yes, it already says that: |
|
🚧 @iwiznia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.4.0-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review — no changes required I reviewed this PR against the help site articles under What this PR does: Adds two in-app keyboard shortcuts — Why no docs change is needed:
Adding a brand-new shortcuts reference page for just these two additions — while leaving the existing in-app shortcuts undocumented — would be inconsistent and is out of scope for this change. If the team later decides to publish a full keyboard-shortcuts reference, that would be a separate, dedicated effort. Since no changes are required, I did not create a draft help site PR. |
|
Deploy Blocker #92810 was identified to be related to this PR. |
|
Deploy Blocker #92813 was identified to be related to this PR. |
|
Deploy Blocker #92815 was identified to be related to this PR. |
|
Deploy Blocker #92825 was identified to be related to this PR. |
|
Deploy Blocker #92830 was identified to be related to this PR. |
|
Deploy Blocker ##92945 was identified to be related to this PR |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.0-7 🚀
|
|
🤖 Payment issue created: #92992 |
10 participants
Explanation of Change
Context https://expensify.slack.com/archives/C03U7DCU4/p1780048986882969
OldDot had SHIFT+P and SHIFT+G, but since we are a chat, in almost every page you are typing that needs to type in the chat input, so using SHIFT for shortcuts is no good (all shortcuts we have use CMD except for SHIFT+ESC because that does not type anything under normal circumstances).
I picked CMD+B and CMD+U because they do not trigger any default browser shortcut.
$ #92247
Tests
type:expense-report report-id:Offline tests
No
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-05-29.at.12.58.37.PM.mov