Skip to content

Add 'Mark all as read' context menu to the Unreads tab - #98950

Merged
tgolen merged 13 commits into
mainfrom
claude-markAllAsReadUnreadsTab
Aug 24, 2026
Merged

tgolen merged 13 commits into
mainfrom
claude-markAllAsReadUnreadsTab

Conversation

@MelvinBot

@MelvinBot MelvinBot commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

Implements Part 1 of the approved proposal in #98057: a fast, discoverable way to clear an unread backlog from the LHN.

Right-clicking (or long-pressing) the Unread tab in the inbox tab row now opens a context menu with a single Mark all as read option. Selecting it shows a confirmation modal ("Are you sure you want to mark all chats as read?"); confirming marks every unread chat as read.

This is a UI-wiring change on top of the existing bulk primitive — it reuses the same markAllMessagesAsRead() action that already backs the Shift+Escape keyboard shortcut, so there is no new Onyx/API/state work. Implementation notes:

  • Wired the previously-unused onLongTabPress hook on TabSelectorBase into InboxTabSelector, gated to only the UNREAD tab.
  • The menu is a standard PopoverMenu anchored to the tab row via usePopoverPosition; the confirmation uses the (non-deprecated) useConfirmModal global-modal hook.
  • Added inboxTabs.markAllAsRead and inboxTabs.markAllAsReadConfirmationPrompt copy across all locales.

Currently the action clears all unread reports globally (matching the keyboard shortcut). Scoping it to only the visible/filtered Unread list would be a follow-up if desired.

Fixed Issues

$ #98057
PROPOSAL: #98057 (comment)

Tests

  1. Sign in to an account that has at least one unread chat (an unread badge shows on the Unread tab in the LHN).
  2. Right-click (desktop/web) or long-press (mobile) the Unread tab.
  3. Verify a context menu appears with a Mark all as read option.
  4. Select it and verify a confirmation modal appears asking "Are you sure you want to mark all chats as read?".
  5. Confirm and verify all unread chats become read and the Unread badge count clears.
  6. Repeat and press Cancel on the confirmation; verify nothing changes.
  • Verify that no errors appear in the JS console

Offline tests

Same as Tests

QA Steps

Same as Tests

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
Screen.Recording.2026-08-19.at.16.50.02.mov
Android: mWeb Chrome
Screen.Recording.2026-08-19.at.16.51.34.mov
iOS: Native
Screen.Recording.2026-08-19.at.16.54.08.mov
iOS: mWeb Safari
Screen.Recording.2026-08-19.at.16.58.12.mov
MacOS: Chrome / Safari
Screen.Recording.2026-08-19.at.16.50.02.mov

Co-authored-by: Hans Vu <hungvu193@users.noreply.github.com>
@OSBotify

Copy link
Copy Markdown
Contributor

🦜 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 diff
diff --git a/src/languages/de.ts b/src/languages/de.ts
index 5bfd1118..18148d96 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -3068,7 +3068,13 @@ ${amount} für ${merchant} – ${date}`,
         prompt: (priorityModePageUrl: string) =>
             `Behalten Sie den Überblick, indem Sie nur ungelesene Chats oder Chats sehen, die Ihre Aufmerksamkeit erfordern. Keine Sorge, Sie können dies jederzeit in den <a href="${priorityModePageUrl}">Einstellungen</a> ändern.`,
     },
-    inboxTabs: {all: 'Alle', todo: 'Aufgaben', unread: 'Ungelesen', markAllAsRead: 'Alle als gelesen markieren', markAllAsReadConfirmationPrompt: 'Möchtest du wirklich alle Chats als gelesen markieren?'},
+    inboxTabs: {
+        all: 'Alle',
+        todo: 'Aufgaben',
+        unread: 'Ungelesen',
+        markAllAsRead: 'Alle als gelesen markieren',
+        markAllAsReadConfirmationPrompt: 'Sind Sie sicher, dass Sie alle Chats als gelesen markieren möchten?',
+    },
     reportDetailsPage: {
         inWorkspace: (policyName: string) => `in ${policyName}`,
         generatingPDF: 'PDF erstellen',
diff --git a/src/languages/el.ts b/src/languages/el.ts
index 14be9d0f..5994f221 100644
--- a/src/languages/el.ts
+++ b/src/languages/el.ts
@@ -3128,7 +3128,7 @@ ${amount} για ${merchant} - ${date}`,
         todo: 'Εκκρεμότητες',
         unread: 'Μη αναγνωσμένα',
         markAllAsRead: 'Επισήμανση όλων ως αναγνωσμένων',
-        markAllAsReadConfirmationPrompt: 'Είστε βέβαιοι ότι θέλετε να επισημάνετε όλες τις συνομιλίες ως αναγνωσμένες;',
+        markAllAsReadConfirmationPrompt: 'Είστε βέβαιοι ότι θέλετε να σημειώσετε όλες τις συνομιλίες ως αναγνωσμένες;',
     },
     reportDetailsPage: {
         goToRoom: 'Μετάβαση στο δωμάτιο',
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 89f83840..61e42289 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -2974,7 +2974,13 @@ ${amount} para ${merchant} - ${date}`,
         prompt: (priorityModePageUrl: string) =>
             `Mantente al tanto de todo viendo solo los chats no leídos o los chats que necesitan tu atención. No te preocupes, puedes cambiarlo en cualquier momento en los <a href="${priorityModePageUrl}">ajustes</a>.`,
     },
-    inboxTabs: {all: 'Todo', todo: 'Pendientes', unread: 'No leído', markAllAsRead: 'Marcar todo como leído', markAllAsReadConfirmationPrompt: '¿Seguro que quieres marcar todos los chats como leídos?'},
+    inboxTabs: {
+        all: 'Todo',
+        todo: 'Pendientes',
+        unread: 'No leído',
+        markAllAsRead: 'Marcar todo como leído',
+        markAllAsReadConfirmationPrompt: '¿Seguro que quieres marcar todos los chats como leídos?',
+    },
     reportDetailsPage: {
         goToRoom: 'Ir a la sala',
         inWorkspace: (policyName) => `en ${policyName}`,
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index ac60bee9..adc705b6 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -3074,7 +3074,13 @@ ${amount} pour ${merchant} - ${date}`,
         prompt: (priorityModePageUrl: string) =>
             `Gardez le contrôle en n’affichant que les discussions non lues ou celles qui nécessitent votre attention. Ne vous inquiétez pas, vous pouvez modifier ce réglage à tout moment dans les <a href="${priorityModePageUrl}">paramètres</a>.`,
     },
-    inboxTabs: {all: 'Tout', todo: 'Tâches', unread: 'Non lu', markAllAsRead: 'Tout marquer comme lu', markAllAsReadConfirmationPrompt: 'Voulez-vous vraiment marquer toutes les discussions comme lues ?'},
+    inboxTabs: {
+        all: 'Tout',
+        todo: 'Tâches',
+        unread: 'Non lu',
+        markAllAsRead: 'Tout marquer comme lu',
+        markAllAsReadConfirmationPrompt: 'Voulez-vous vraiment marquer toutes les discussions comme lues ?',
+    },
     reportDetailsPage: {
         inWorkspace: (policyName: string) => `dans ${policyName}`,
         generatingPDF: 'Générer le PDF',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 951802ca..f0b41cbd 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -3059,7 +3059,13 @@ ${amount} per ${merchant} - ${date}`,
         prompt: (priorityModePageUrl: string) =>
             `Tieniti al passo vedendo solo le chat non lette o quelle che richiedono la tua attenzione. Non preoccuparti, puoi cambiare questa impostazione in qualsiasi momento nelle <a href="${priorityModePageUrl}">impostazioni</a>.`,
     },
-    inboxTabs: {all: 'Tutti', todo: 'Attività da fare', unread: 'Non letti', markAllAsRead: 'Segna tutto come letto', markAllAsReadConfirmationPrompt: 'Vuoi davvero segnare tutte le chat come lette?'},
+    inboxTabs: {
+        all: 'Tutti',
+        todo: 'Attività da fare',
+        unread: 'Non letti',
+        markAllAsRead: 'Segna tutto come letto',
+        markAllAsReadConfirmationPrompt: 'Sei sicuro di voler contrassegnare tutte le chat come lette?',
+    },
     reportDetailsPage: {
         inWorkspace: (policyName: string) => `in ${policyName}`,
         generatingPDF: 'Genera PDF',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 87362e29..fa2682f2 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -3060,7 +3060,13 @@ ${amount} voor ${merchant} - ${date}`,
         prompt: (priorityModePageUrl: string) =>
             `Blijf op de hoogte door alleen ongelezen chats of chats te zien die je aandacht nodig hebben. Geen zorgen, je kunt dit op elk moment wijzigen in de <a href="${priorityModePageUrl}">instellingen</a>.`,
     },
-    inboxTabs: {all: 'Alles', todo: 'Te doen', unread: 'Ongelezen', markAllAsRead: 'Alles als gelezen markeren', markAllAsReadConfirmationPrompt: 'Weet je zeker dat je alle chats als gelezen wilt markeren?'},
+    inboxTabs: {
+        all: 'Alles',
+        todo: 'Te doen',
+        unread: 'Ongelezen',
+        markAllAsRead: 'Alles als gelezen markeren',
+        markAllAsReadConfirmationPrompt: 'Weet je zeker dat je alle chats als gelezen wilt markeren?',
+    },
     reportDetailsPage: {
         inWorkspace: (policyName: string) => `in ${policyName}`,
         generatingPDF: 'PDF genereren',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 5b016300..57d2a692 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -3047,7 +3047,13 @@ ${amount} dla ${merchant} - ${date}`,
         prompt: (priorityModePageUrl: string) =>
             `Bądź na bieżąco, widząc tylko nieprzeczytane czaty lub czaty wymagające twojej uwagi. Spokojnie, możesz to zmienić w dowolnym momencie w <a href="${priorityModePageUrl}">ustawieniach</a>.`,
     },
-    inboxTabs: {all: 'Wszystko', todo: 'Zadania do wykonania', unread: 'Nieprzeczytane', markAllAsRead: 'Oznacz wszystkie jako przeczytane', markAllAsReadConfirmationPrompt: 'Czy na pewno chcesz oznaczyć wszystkie czaty jako przeczytane?'},
+    inboxTabs: {
+        all: 'Wszystko',
+        todo: 'Zadania do wykonania',
+        unread: 'Nieprzeczytane',
+        markAllAsRead: 'Oznacz wszystkie jako przeczytane',
+        markAllAsReadConfirmationPrompt: 'Na pewno chcesz oznaczyć wszystkie czaty jako przeczytane?',
+    },
     reportDetailsPage: {
         inWorkspace: (policyName: string) => `w ${policyName}`,
         generatingPDF: 'Wygeneruj PDF',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 3927e917..245c766b 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -3050,7 +3050,13 @@ ${amount} para ${merchant} - ${date}`,
         prompt: (priorityModePageUrl: string) =>
             `Fique no controle vendo apenas chats não lidos ou que precisam da sua atenção. Não se preocupe, você pode mudar isso a qualquer momento em <a href="${priorityModePageUrl}">configurações</a>.`,
     },
-    inboxTabs: {all: 'Todos', todo: 'Pendências', unread: 'Não lidas', markAllAsRead: 'Marcar tudo como lido', markAllAsReadConfirmationPrompt: 'Tem certeza de que deseja marcar todos os chats como lidos?'},
+    inboxTabs: {
+        all: 'Todos',
+        todo: 'Pendências',
+        unread: 'Não lidas',
+        markAllAsRead: 'Marcar tudo como lido',
+        markAllAsReadConfirmationPrompt: 'Tem certeza de que quer marcar todos os chats como lidos?',
+    },
     reportDetailsPage: {
         inWorkspace: (policyName: string) => `em ${policyName}`,
         generatingPDF: 'Gerar PDF',

Note

You can apply these changes to your branch by copying the patch to your clipboard, then running pbpaste | git apply 😉

View workflow run

@codecov

codecov Bot commented Aug 19, 2026 •

Copy link
Copy Markdown

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.

Files with missing lines Coverage Δ
src/components/TabSelector/TabSelectorBase.tsx 89.74% <100.00%> (ø)
src/components/TabSelector/TabSelectorItem.tsx 100.00% <100.00%> (ø)
...dShortcutsHandler/MarkAllMessagesAsReadHandler.tsx 90.90% <100.00%> (ø)
src/libs/actions/Report/MarkAllMessageAsRead.tsx 90.32% <100.00%> (ø)
src/pages/Search/SearchTypeMenuNarrow.tsx 86.51% <ø> (ø)
src/selectors/ReportNameValuePairs.ts 83.87% <50.00%> (-8.13%) ⬇️
src/pages/inbox/sidebar/InboxTabSelector.tsx 83.72% <74.07%> (-6.76%) ⬇️
... and 373 files with indirect coverage changes

Co-authored-by: Hans Vu <hungvu193@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

Fixed the failing Oxfmt check — pushed commit 119b819.

Cause: the PR added markAllAsRead and markAllAsReadConfirmationPrompt to the single-line inboxTabs object in the 7 non-English language files, pushing those lines past the formatter's width limit. oxfmt wants that object wrapped across multiple lines, so the CI's "no Oxfmt diff" check failed.

Fix: ran npm run fmt, which reformatted only the inboxTabs object in de, es, fr, it, nl, pl, and pt-BR. Pure whitespace/wrapping — no translation text or quote characters were changed.

The only other failing check, Check independent approval, is a process gate (the PR still needs an independent reviewer approval) and isn't something a code change can resolve.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

✅ Android test — all steps passed

Ran the testing steps for this PR on standalone Android NewDot. Every verification passed and no console errors appeared.

# Check Result
1 Long-press Unread tab shows context menu with Mark all as read ✅ Pass
2 Selecting it shows confirmation modal "Are you sure you want to mark all chats as read?" ✅ Pass
3 Cancel leaves unread chats unchanged (badge still shows 1) ✅ Pass
4 Confirm marks all chats read and clears the Unread badge ✅ Pass
5 Unread tab shows "No unread chats" empty state afterward ✅ Pass
Evidence (screenshots)

1. Context menu with "Mark all as read" after long-pressing the Unread tab
Context menu with Mark all as read option after long-pressing Unread tab

2. Confirmation modal
Confirmation modal asking to mark all chats as read

3. Cancel — inbox unchanged, Unread badge still 1
Inbox unchanged after pressing Cancel, Unread badge still shows 1

4. Confirm — Unread badge cleared, chats read
Inbox after confirming Mark all as read, Unread badge cleared

5. Unread tab empty state
Unread tab showing No unread chats empty state

A recording of the run is attached by CI.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

⚠️ Web test — could not complete (tooling limitation) + a possible desktop-web bug

I wasn't able to run the web testing steps: my sanctioned web automation toolkit has no supported way to trigger a right-click / long-press on desktop web, which is exactly the gesture this feature depends on. Every gesture primitive returned UNSUPPORTED_OPERATION for the web platform (click --button secondary, longpress, press --hold-ms, gesture pan). A plain left-click only switches tabs, so the context menu could never be opened to test steps 1–5.

I did drive the session to Home and confirm the All / Unread / To-dos tab row renders and that clicking Unread shows the "No unread chats" empty state — but that's it.

🐛 Worth double-checking before merge: right-click may not open the menu on desktop web

While looking into why I couldn't trigger the menu, I noticed a code-level concern (not runtime-verified — I couldn't fire a real contextmenu/right-click event):

The popover is wired only to onLongPress via InboxTabSelector.tsx:118 → GenericPressable's onLongPress. In react-native-web, PressResponder only handles onLongPress for touch-originated pointers; a real desktop mouse right-click (contextmenu) does not invoke onLongPress. Other context menus in the app additionally wire onContextMenu / onSecondaryInteraction for desktop right-click parity (e.g. SidebarLinks.tsx, ReportActionItem.tsx), but InboxTabSelector does not.

If that holds, the PR's test step "Right-click (desktop/web) the Unread tab" may not actually open the menu on desktop — only a mouse press-and-hold would. Please verify manually in a real desktop browser.

Evidence & what I confirmed

Session reached Inbox with the tab row visible:
Inbox tab row (All/Unread/To-dos) with one Concierge chat

Plain click on Unread switches tab (shows empty state) — no context menu:
Unread tab clicked normally, shows No unread chats empty state

Toolkit rejections (web platform):

  • click --button secondary → supported only on macOS and Linux
  • longpress → not supported on this device
  • press --hold-ms → longPress is not supported on web
  • gesture pan → not supported on web

Note: the same feature passed end-to-end on Android Native (long-press works there), so the concern is specific to desktop-web right-click parity.

@hungvu193

Copy link
Copy Markdown
Contributor

@MelvinBot The right click on Tab Selector still doesn't work, let fix it with secondaryInteraction

…eraction

Co-authored-by: Hans Vu <hungvu193@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

Fixed in dc57aa7. Right-click now opens the menu.

The tab was using PressableWithFeedback with only onLongPress, which fires on touch long-press but never on a web right-click — so the menu couldn't be opened with a mouse. Switched TabSelectorItem to PressableWithSecondaryInteraction and wired onSecondaryInteraction, which handles both long-press (native) and right-click / contextmenu (web). The nav TabSelector is unaffected — it never passes onLongPress, so onSecondaryInteraction is undefined there and behavior is unchanged.

Static checks all pass: ESLint, typecheck, and React Compiler compliance.

⚠️ I couldn't get an automated web run to confirm behavior — the prestarted web session's browser client crashed on a blank page (a transient dev-server build error fired while the file was mid-write; the server recovered but the client didn't, and the tester can't reload without risking the session). Please re-run the Melvin-Test-Web check on a fresh session, or verify manually that right-clicking the Unread tab opens the Mark all as read menu (and the browser's native context menu is suppressed).

@hungvu193

Copy link
Copy Markdown
Contributor

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dc57aa76ac

ℹ️ 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".

]}
wrapperStyle={equalWidth ? styles.flex1 : styles.flexGrow1}
onLongPress={onLongPress}
onSecondaryInteraction={onLongPress}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep secondary interactions disabled for disabled tabs

On narrow web layouts while offline, SearchTypeMenuNarrow retains saved-search tabs with a pending delete and marks them isDisabled. The previous onLongPress was suppressed by the disabled pressable, but PressableWithSecondaryInteraction handles browser contextmenu events outside the pressable and does not check disabled, so this new handler still opens the rename/share/delete menu for an item already being deleted. Gate the secondary interaction when the tab is disabled, matching the existing press behavior.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MelvinBot Let's fix this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 94b2901408.

PressableWithSecondaryInteraction wires the web contextmenu (right-click) listener directly and calls onSecondaryInteraction without checking the pressable's disabled prop (PressableWithSecondaryInteraction/index.tsx:83), so a right-click on a disabled tab (e.g. a saved-search tab with a pending delete in SearchTypeMenuNarrow) would still open the menu.

Gated it to match the primary press: onSecondaryInteraction={isPressableDisabled ? undefined : onLongPress}, where isPressableDisabled = !disabledAction && isDisabled is the same expression already used for the disabled prop (TabSelectorItem.tsx:76). When onSecondaryInteraction is undefined, the component skips registering the contextmenu listener entirely, so disabled tabs no longer open a menu on right-click or long-press.

Comment on lines +79 to +81
// Wait for the popover to fully hide before showing the confirmation modal so they don't overlap.
shouldCallAfterModalHide: true,
onSelected: confirmMarkAllAsRead,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve modal sequencing in Safari

On Safari, PopoverMenu intentionally ignores shouldCallAfterModalHide unless its shouldAvoidSafariException prop is enabled. Because this instance leaves that prop at its default, selecting the action hides the popover via onItemSelected and invokes showConfirmModal immediately, so the confirmation appears while the popover and its focus trap are still dismissing rather than after it has hidden as intended. Enable the Safari override here or launch the confirmation from the popover's hide callback.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MelvinBot Same here, let's investigate and fix it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 94b2901408.

Confirmed: PopoverMenu only honors shouldCallAfterModalHide when !isSafari() || shouldAvoidSafariException (PopoverMenu/index.tsx:421). Since this instance left shouldAvoidSafariException at its default (false), on Safari the selection fell through to the immediate branch — onSelected (and thus showConfirmModal) fired while the popover and its focus trap were still dismissing.

Added shouldAvoidSafariException to this PopoverMenu (InboxTabSelector.tsx) so the after-hide sequencing the shouldCallAfterModalHide comment intends holds on Safari too — the confirmation modal now opens only after the popover has fully hidden.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is already addressed in the latest commit 94b2901 on this branch.

The PopoverMenu in InboxTabSelector.tsx:131 now passes shouldAvoidSafariException, so shouldCallAfterModalHide is honored on Safari and the confirmation modal opens only after the popover has fully hidden — fixing the sequencing issue codex flagged.

No further change needed here.

@truph01

truph01 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

@hungvu193 Please let me know once this PR is ready for review. Thanks!

@hungvu193

Copy link
Copy Markdown
Contributor

@hungvu193 Please let me know once this PR is ready for review. Thanks!

Sure

…al sequencing

Co-authored-by: Hans Vu <hungvu193@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

No code-actionable failures remain. The Oxfmt check that was failing is now green after my earlier fix (commit 119b819, which ran npm run fmt on the language files).

The two checks still showing red are both non-actionable process gates, not code problems:

  • Check independent approval — waiting on an independent reviewer's approval.
  • checklist (PR Reviewer Checklist) — completed by the reviewer, not by a code change.

baseline-perf-tests is still pending, not failed. Nothing here requires a code fix from me.

@hungvu193

Copy link
Copy Markdown
Contributor

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 94b2901408

ℹ️ 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".

}
setActiveTab(key);
}}
onLongTabPress={openMarkAllAsReadMenu}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Avoid intercepting right-clicks on tabs without an action

On desktop web, passing openMarkAllAsReadMenu as the selector-wide handler installs a secondary-interaction callback on All and To-dos as well as Unread. PressableWithSecondaryInteraction prevents the browser context menu before this callback returns for those unsupported keys, so right-clicking All or To-dos now silently does nothing instead of retaining its prior browser behavior. Attach the secondary interaction only to the Unread item, rather than filtering after the event has already been consumed.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already early return with other tab, this won't be an issue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it will be considered as regression?
In staging, right clicking on other tabs opens browser context menu:

Screen.Recording.2026-08-20.at.09.37.53.mov

In this PR, right clicking on other tabs opens nothing:

Screen.Recording.2026-08-20.at.09.38.07.mov

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's definitely regression 👀

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hungvu193 This issue was reintroduced when there are no unread chats in the Unread tab.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@truph01 Let's revert it while waiting for Design team decision then?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hungvu193 Yes, then we can address it as follow-up PR if needed

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No code fix needed — the only actionable failure (Oxfmt check) was already resolved by the earlier npm run fmt commit and is now passing.

The two checks still showing red are process/human gates, not something a code push can fix:

  • Check independent approval — the PR needs an independent reviewer's approval.
  • PR Reviewer Checklist / checklist — "No PR Reviewer Checklist was found"; it turns green once a reviewer completes the checklist on the PR.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No code fix needed — all code checks are green.

The Oxfmt fix I pushed earlier (119b819) resolved the only real failure. Oxfmt, ESLint, typecheck, and all test jobs now pass.

The two remaining ❌ checks are process gates, not code issues, so there's nothing for me to push:

  • Check independent approval — waiting on an independent reviewer's approval.
  • PR Reviewer Checklist / checklist — waiting on a reviewer to complete the checklist.

Both clear once the PR gets a review; no code change can resolve them.

@hungvu193

Copy link
Copy Markdown
Contributor

@MelvinBot Fill all the checkboxes, fill the QA and Offline section with "Same as Tests".

Fill the below screenshots link to Screenshots/Video section:

Screen.Recording.2026-08-19.at.16.50.02.mov
Screen.Recording.2026-08-19.at.16.51.34.mov
Screen.Recording.2026-08-19.at.16.54.08.mov
Screen.Recording.2026-08-19.at.16.58.12.mov

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Done ✅ — updated the PR description:

  • Checked all checkboxes in the PR Author Checklist (and the two "no JS console errors" boxes).
  • Offline tests → "Same as Tests"
  • QA Steps → "Same as Tests"
  • Moved the suggested test steps into the Tests section as the confirmed steps.
  • Added the four recordings to the Screenshots/Videos section.

⚠️ One thing to double-check: you provided 4 links but there are 5 platform sections. I placed them in the order given — Android: Native, Android: mWeb Chrome, iOS: Native, iOS: mWeb Safari — and reused the first recording for MacOS: Chrome / Safari. Please rearrange them if that mapping isn't right.

@hungvu193

Copy link
Copy Markdown
Contributor

@MelvinBot Mark this PR ready for review.

@MelvinBot
MelvinBot marked this pull request as ready for review August 19, 2026 10:18
…eOnyx selector

Read private_isArchived through reportNameValuePairsArchivedSelector in the
components and pass it into markAllMessagesAsRead, so the action stays a plain
function and consumers only re-render when the archived flags change.

Co-authored-by: Hans Vu <hungvu193@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

Done — reverted the Onyx.connectWithoutView read for report NVPs and moved back to useOnyx with a selector. Pushed in c8c05f9.

What changed

  • Added reportNameValuePairsArchivedSelector — it extracts only private_isArchived from each report's NVPs, so subscribers don't re-render when unrelated NVP fields (agent-zero indicators, calendly calls, etc.) change.
  • MarkAllMessageAsRead.tsx: removed the REPORT_NAME_VALUE_PAIRS connectWithoutView; markAllMessagesAsRead now takes the archived slice as a parameter.
  • InboxTabSelector.tsx and MarkAllMessagesAsReadHandler.tsx read via useOnyx(..., {selector}) and pass the result in. The handler keeps the value in a ref so the mount-time keyboard-shortcut callback reads it fresh at fire time.
  • Updated the markAllMessagesAsRead unit test call to pass the new argument.

The isArchivedReport type contract still holds — every ReportNameValuePairs field is optional, so the Pick<..., 'private_isArchived'> slice is assignable where the full type is expected.

One note: I couldn't run typecheck/lint/jest in this environment (the build tooling is sandboxed off here), so please rely on the PR's CI checks to confirm them green.


view run

@truph01

truph01 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

@hungvu193 I have a comment #98950 (comment)

@truph01

truph01 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

@MelvinBot Revert 9b2cc1e

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🔄 Reverted commit 9b2cc1e ("Hide 'Mark all as read' menu when there are no unread chats") in efa2837 and pushed to claude-markAllAsReadUnreadsTab.

The menu-gating logic in src/pages/inbox/sidebar/InboxTabSelector.tsx is restored to only check the tab key (no inboxTabCounts[...] <= 0 condition) — matching the state before that commit. inboxTabCounts is still used elsewhere in the file, so nothing is left dangling.

Checks
  • npm run typecheck: no errors in src/. (The pre-existing errors it reports are all in .github/, scripts/, and tests/tooling/ and are unrelated to this change.)
  • eslint/prettier wrappers couldn't run in this environment (missing dev deps oxc-transform and @trivago/prettier-plugin-sort-imports). Since this is a pure inverse of a commit that already passed CI on this branch, CI will re-validate on push.

view run

@truph01

truph01 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: HybridApp
    • Android: mWeb Chrome
    • iOS: HybridApp
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained """"why"""" the code was doing something instead of only explaining """"what"""" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp
Screen.Recording.2026-08-22.at.17.10.35.mov
Android: mWeb Chrome
Screen.Recording.2026-08-22.at.16.57.29.mov
iOS: HybridApp
Screen.Recording.2026-08-22.at.17.05.38.mov
iOS: mWeb Safari
Screen.Recording.2026-08-22.at.17.09.35.mov
MacOS: Chrome / Safari
Screen.Recording.2026-08-22.at.16.53.02.mov

@dubielzyk-expensify

Copy link
Copy Markdown
Contributor

@hungvu193 Should we hide "Mark all as read" context menu if there is no unread message?

Part of me think it's worth keeping there just for the education, but it does seem a bit odd that it's an option. Curious what @shawnborton thinks.

@shawnborton

Copy link
Copy Markdown
Contributor

Hmm yeah tough one, I could see both sides but I kinda like just leaving it there as well.

@dubielzyk-expensify

Copy link
Copy Markdown
Contributor

Let's go with leaving it then 👍

@tgolen tgolen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve the Onyx.connectWithoutView() part of this review. Looks like there are no more references to it

@hungvu193

Copy link
Copy Markdown
Contributor

@tgolen Can you merge it? Ty

@tgolen
tgolen merged commit 17f4848 into main Aug 24, 2026
46 of 49 checks passed
@tgolen
tgolen deleted the claude-markAllAsReadUnreadsTab branch August 24, 2026 17:24
@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions

Copy link
Copy Markdown
Contributor

🚧 tgolen has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify OSBotify mentioned this pull request Aug 24, 2026
77 tasks done
@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/tgolen in version: 9.4.60-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Help site review — changes required ✅

This PR adds a user-facing Mark all as read action to the Unread tab (right-click on web / long-press on mobile → confirmation modal). That behavior is documentable and wasn't covered on the help site, so I've drafted an update.

Docs PR: #99367 — Docs updates for E/A#98950 Mark all as read from Unread tab

What changed: Added a task-based section "How to mark all chats as read from the Unread tab" to Navigating-New-Expensify.md — the article that already documents the Inbox filter tabs (All, Unread, To-dos) — plus a mark all as read keyword for search.

Verification notes
  • UI copy (Mark all as read and the prompt "Are you sure you want to mark all chats as read?") was taken verbatim from this PR's src/languages/en.ts.
  • The right-click (web) / long-press (mobile) interaction was confirmed from the PR implementation.
  • Live UI verification couldn't be completed this run — the dev web session was stuck on the splash screen (an environment/session issue unrelated to this feature) — so the wording relies on the merged code as the source of truth.

@MelvinBot, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review


view run

@mitarachim

Copy link
Copy Markdown

Deploy Blocker #99378 was identified to be related to this PR.

@mitarachim

Copy link
Copy Markdown

Deploy Blocker #99379 was identified to be related to this PR.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/AndrewGable in version: 9.4.60-2 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants