From c844374fa45db1dad8cf3b3f053cff6cab92734e Mon Sep 17 00:00:00 2001 From: MobileMage Date: Thu, 22 Jan 2026 20:57:45 +0100 Subject: [PATCH 1/4] fix: resolve keyboard reordering issues in DraggableList - Configure KeyboardSensor to use Space-only for start/end drag - Block Enter key during drag to prevent MenuItem navigation conflicts - Generate unique instance ID per mount to reset DndContext state - Dispatch Escape on unmount to cancel any active keyboard drag --- src/components/DraggableList/SortableItem.tsx | 12 ++++++++++- src/components/DraggableList/index.tsx | 21 ++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/components/DraggableList/SortableItem.tsx b/src/components/DraggableList/SortableItem.tsx index 7ed0e0b85a0f7..0f02eb013201e 100644 --- a/src/components/DraggableList/SortableItem.tsx +++ b/src/components/DraggableList/SortableItem.tsx @@ -4,7 +4,7 @@ import React from 'react'; import type {SortableItemProps} from './types'; function SortableItem({id, children, disabled = false}: SortableItemProps) { - const {attributes, listeners, setNodeRef, transform, transition} = useSortable({id, disabled}); + const {attributes, listeners, setNodeRef, transform, transition, isDragging} = useSortable({id, disabled}); const style = { touchAction: 'none', @@ -12,10 +12,20 @@ function SortableItem({id, children, disabled = false}: SortableItemProps) { transition, }; + // Prevent Enter key from reaching MenuItem when dragging to avoid navigation conflicts + const handleKeyDown = (e: React.KeyboardEvent) => { + if (isDragging && e.key === 'Enter') { + e.preventDefault(); + e.stopPropagation(); + } + }; + return (
({ }: DraggableListProps & {ref?: React.ForwardedRef}) { const styles = useThemeStyles(); + // Generate a unique ID per mount to ensure DndContext state resets when component remounts + const [instanceId] = useState(() => `${Date.now()}-${Math.random().toString(36).slice(2)}`); + + // Cancel any active keyboard drag when the component unmounts to prevent ghost drag state + useEffect(() => { + return () => { + if (typeof document === 'undefined') { + return; + } + document.dispatchEvent(new KeyboardEvent('keydown', {key: 'Escape', code: 'Escape', bubbles: true, cancelable: true})); + }; + }, []); + const items = data.map((item, index) => { return keyExtractor(item, index); }); @@ -77,6 +90,11 @@ function DraggableList({ }), useSensor(KeyboardSensor, { coordinateGetter: sortableKeyboardCoordinates, + keyboardCodes: { + start: ['Space'], + cancel: ['Escape'], + end: ['Space'], + }, }), ); @@ -90,6 +108,7 @@ function DraggableList({ >
Date: Thu, 22 Jan 2026 21:14:30 +0100 Subject: [PATCH 2/4] fix: address ESLint and only dispatch Escape when drag is active --- proposal-80150.md | 53 ++++++++++++ proposal-80157.md | 80 +++++++++++++++++++ proposal-80165.md | 77 ++++++++++++++++++ proposal-80233.md | 27 +++++++ src/components/DraggableList/SortableItem.tsx | 7 +- src/components/DraggableList/index.tsx | 18 ++++- 6 files changed, 257 insertions(+), 5 deletions(-) create mode 100644 proposal-80150.md create mode 100644 proposal-80157.md create mode 100644 proposal-80165.md create mode 100644 proposal-80233.md diff --git a/proposal-80150.md b/proposal-80150.md new file mode 100644 index 0000000000000..25e96a8d10cbd --- /dev/null +++ b/proposal-80150.md @@ -0,0 +1,53 @@ +## Proposal + +### Please re-state the problem that we are trying to solve in this issue. + +In the Left Hand Navigation (LHN), workspace expense rooms (policy expense chats) have noticeably less horizontal spacing than non-workspace rooms when in Focus/Compact mode. The text content appears 4px closer to the avatar than in standard chat rooms. + +### What is the root cause of that problem? + +The spacing inconsistency occurs due to different margin values applied to avatars based on their type: + +1. **Single avatars** in `OptionRowLHN.tsx` are given an explicit `styles.mr3` (12px margin-right) via `singleAvatarContainerStyle`: + https://github.com/Expensify/App/blob/e54cfd4428ac49ea5452c1e88c8938c0b1248d0a/src/components/LHNOptionsList/OptionRowLHN.tsx#L274 + +2. **Subscript avatars** (used for workspace expense rooms) use `StyleUtils.getContainerStyles(size)` which returns `styles.emptyAvatarMarginSmall` (8px margin-right) for `CONST.AVATAR_SIZE.SMALL`: + https://github.com/Expensify/App/blob/e54cfd4428ac49ea5452c1e88c8938c0b1248d0a/src/components/ReportActionAvatars/ReportActionAvatar.tsx#L194 + https://github.com/Expensify/App/blob/e54cfd4428ac49ea5452c1e88c8938c0b1248d0a/src/styles/index.ts#L2416-L2418 + +The 4px difference (`12px - 8px = 4px`) causes the visual inconsistency. In focus mode, avatar size is set to `SMALL` (line 268 of OptionRowLHN.tsx), which triggers this reduced margin for subscript avatars. + +### What changes do you think we should make in order to solve the problem? + +Add 4px margin-left (`styles.ml1`) to the content container specifically for policy expense chats in focus mode to compensate for the reduced avatar margin. This keeps the text alignment consistent across all room types. + +Modify line 127 of `/src/components/LHNOptionsList/OptionRowLHN.tsx`: + +**Current code:** +```typescript +const contentContainerStyles = isInFocusMode ? [styles.flex1, styles.flexRow, styles.overflowHidden, StyleUtils.getCompactContentContainerStyles()] : [styles.flex1]; +``` + +**Proposed code:** +```typescript +const contentContainerStyles = + isInFocusMode && optionItem?.isPolicyExpenseChat + ? [styles.flex1, styles.flexRow, styles.overflowHidden, StyleUtils.getCompactContentContainerStyles(), styles.ml1] + : isInFocusMode + ? [styles.flex1, styles.flexRow, styles.overflowHidden, StyleUtils.getCompactContentContainerStyles()] + : [styles.flex1]; +``` + +**Why this approach:** +- **Minimal change**: Only affects the text positioning for the specific case causing the issue +- **Targeted fix**: Doesn't modify global avatar styles that could affect other components +- **Uses existing patterns**: Similar conditional styling based on report type is already used elsewhere in the component (e.g., line 289-298 for `shouldUseFullTitle`) +- **Safe**: Only adds margin when both conditions are true (focus mode AND policy expense chat) + +### What alternative solutions did you explore? (Optional) + +1. **Modifying `emptyAvatarMarginSmall` to 12px**: This would affect all components using small subscript avatars globally, potentially causing unintended layout changes elsewhere. + +2. **Adding a new prop to ReportActionAvatars for subscript container styles**: More flexible but overly complex for this specific fix. + +3. **Using `shouldShowSubscript` instead of `isPolicyExpenseChat`**: While more comprehensive, this could affect other report types that may have intentionally different spacing. The issue specifically reports workspace expense rooms, so targeting `isPolicyExpenseChat` is appropriate. diff --git a/proposal-80157.md b/proposal-80157.md new file mode 100644 index 0000000000000..e65a17f9147fa --- /dev/null +++ b/proposal-80157.md @@ -0,0 +1,80 @@ +## Proposal + +### Please re-state the problem that we are trying to solve in this issue. + +When users are on the Search/Reports page with filters applied and select reports to export, a modal appears with the message "Export in progress - Concierge will send you the file shortly." This modal only has a confirm button. After confirming, users must manually navigate away from the Reports page to Concierge chat to download the exported file. This navigation causes users to lose their current filter context and workflow state. + +### What is the root cause of that problem? + +The export modal is displayed in `src/pages/Search/SearchPage.tsx` within the `beginExportWithTemplate` callback (lines 259-294): + +```typescript +const beginExportWithTemplate = useCallback( + async (templateName: string, templateType: string, policyID: string | undefined) => { + // ... export queuing logic ... + + const result = await showConfirmModal({ + title: translate('export.exportInProgress'), + prompt: translate('export.conciergeWillSend'), + confirmText: translate('common.buttonConfirm'), + shouldShowCancelButton: false, + }); + if (result.action !== ModalActions.CONFIRM) { + return; + } + clearSelectedTransactions(undefined, true); + }, + [queryJSON, selectedTransactionsKeys, areAllMatchingItemsSelected, selectedTransactionReportIDs, showConfirmModal, translate, clearSelectedTransactions], +); +``` + +After showing the confirmation modal telling users Concierge will send the file, the code simply clears the selected transactions and does nothing else. There's no way to access Concierge without manually navigating away from the Reports page. + +The app already has Side Panel functionality that can show Concierge without navigation. The Side Panel renders Concierge by default using the `conciergeReportID` from the `SidePanelContextProvider`. + +### What changes do you think we should make in order to solve the problem? + +After the export confirmation modal is dismissed, automatically open the Side Panel on desktop to show Concierge. Users can then receive and download the file without leaving the Reports page or losing their filters. + +In `src/pages/Search/SearchPage.tsx`, modify the `beginExportWithTemplate` callback: + +```typescript +// At the top of the file, add import: +import useSidePanel from '@hooks/useSidePanel'; + +// Inside the SearchPage component, get the openSidePanel function: +const {openSidePanel} = useSidePanel(); + +// Modify beginExportWithTemplate callback: +const beginExportWithTemplate = useCallback( + async (templateName: string, templateType: string, policyID: string | undefined) => { + // ... existing export queuing logic ... + + const result = await showConfirmModal({ + title: translate('export.exportInProgress'), + prompt: translate('export.conciergeWillSend'), + confirmText: translate('common.buttonConfirm'), + shouldShowCancelButton: false, + }); + if (result.action !== ModalActions.CONFIRM) { + return; + } + clearSelectedTransactions(undefined, true); + + // Open Concierge in Side Panel on desktop only + // shouldUseNarrowLayout is false on desktop + if (!shouldUseNarrowLayout) { + openSidePanel(); + } + }, + [queryJSON, selectedTransactionsKeys, areAllMatchingItemsSelected, selectedTransactionReportIDs, showConfirmModal, translate, clearSelectedTransactions, shouldUseNarrowLayout, openSidePanel], +); +``` + +The same pattern should be applied to `MoneyReportHeader.tsx` and `MoneyRequestReportActionsList.tsx` which also have `beginExportWithTemplate` implementations. + +To test, go to the Search page and apply filters like date range or status. Select some expenses/reports and click Export, then confirm the modal. On desktop, the Side Panel should open with Concierge while the filters remain intact. On mobile, nothing should change. Wait for Concierge to send the file and verify it can be downloaded from the Side Panel without the original filters being lost. + +### What alternative solutions did you explore? (Optional) + +Navigating to Concierge after the modal would defeat the purpose since users would still lose their context. Adding a button to the modal to go to Concierge is more intrusive and requires extra user action. Showing Concierge in a popup would require new modal infrastructure. Using the Side Panel works because it's already designed for this use case. diff --git a/proposal-80165.md b/proposal-80165.md new file mode 100644 index 0000000000000..46046bc9a2c35 --- /dev/null +++ b/proposal-80165.md @@ -0,0 +1,77 @@ +# Proposal for Issue #80165 + +## Please re-state the problem that we are trying to solve in this issue. + +When using keyboard navigation to reorder waypoints in the distance tracking feature, the "Please remove duplicate waypoints" error message persists after dropping an item with the **Enter** key. For example, when waypoints A > B > B (which shows the duplicate error) are reordered to B > A > B, the error remains visible until the user tabs away from the moved item. + +**Key observation:** The feature works correctly when pressing **Space** to finalize the reorder instead of Enter. The error properly clears in that case. + +This was introduced by PR #79793 which added keyboard reordering support to the DraggableList component. + +## What is the root cause of that problem? + +The root cause is related to how @dnd-kit handles the Enter vs Space key for finalizing a drag operation, and the focus/blur cycle that follows. + +Looking at `src/components/DraggableList/SortableItem.tsx`, the component spreads `listeners` from `useSortable` which includes keyboard event handlers. When: + +1. **Space is pressed**: The drag operation ends cleanly. The `onDragEnd` callback fires, waypoints state updates, and `duplicateWaypointsError` is recalculated correctly to `false`. + +2. **Enter is pressed**: The drag operation ends, but Enter key has additional behaviors in @dnd-kit. The Enter key can trigger a "click" or "activation" on the focused element after the drag ends. This causes the error recalculation to be interrupted or the component to be in a transitional focus state where the error doesn't update until focus changes. + +The evidence for this: +- Error clears immediately with Space +- Error persists with Enter but clears when tabbing away (focus change) +- The same behavior affects issue #80166 (waypoint cannot be dragged after Enter) + +In `src/pages/iou/request/step/IOURequestStepDistanceMap.tsx`, the `duplicateWaypointsError` is computed in a `useMemo` that depends on `waypoints`. When Enter is used, there appears to be a focus-related timing issue where React doesn't re-render with the updated error state until the focus changes. + +## What changes do you think we should make in order to solve the problem? + +In `src/components/DraggableList/SortableItem.tsx`, we should intercept the Enter key during an active drag to prevent any additional click/activation behavior after the drop completes: + +```typescript +function SortableItem({id, children, disabled = false}: SortableItemProps) { + const {attributes, listeners, setNodeRef, transform, transition, isDragging} = useSortable({id, disabled}); + const wasDraggingRef = useRef(false); + + // Track if we were just dragging to prevent Enter from triggering click + useEffect(() => { + if (isDragging) { + wasDraggingRef.current = true; + } + }, [isDragging]); + + const handleKeyDown = (e: React.KeyboardEvent) => { + // Call the original listener first + listeners?.onKeyDown?.(e); + + // If Enter was pressed while dragging or just after dropping, + // prevent additional activation behavior + if (e.key === 'Enter' && wasDraggingRef.current) { + e.preventDefault(); + e.stopPropagation(); + // Reset the flag after a brief delay to allow normal Enter behavior later + setTimeout(() => { + wasDraggingRef.current = false; + }, 0); + } + }; + + // ... rest of component +} +``` + +This ensures Enter behaves consistently with Space by preventing any post-drop activation that interferes with state updates. + +## What alternative solutions did you explore? (Optional) + +1. **Configure KeyboardSensor to only use Space for drop**: We could restrict the drop key to only Space via `keyboardCodes.end`. However, Enter is a standard and expected key for confirming actions, so this would reduce accessibility. + +2. **Force blur after keyboard drop**: We could programmatically blur the element after a keyboard drop, but this would be jarring for keyboard users who expect focus to remain on the item they just moved. + +3. **Debounce error display**: This would mask the symptom rather than fix the root cause. + +**Note:** Per the discussion in the issue thread, this is a UX improvement rather than a deploy blocker since: +- The feature works correctly with Space +- The error clears when tabbing away +- The core functionality is not broken diff --git a/proposal-80233.md b/proposal-80233.md new file mode 100644 index 0000000000000..17ab5b6b2b63d --- /dev/null +++ b/proposal-80233.md @@ -0,0 +1,27 @@ +## Proposal + +### Please re-state the problem that we are trying to solve in this issue. + +When a user types a markdown hyperlink with multiline text (e.g., `[line1\nline2\nline3](https://example.com)`), the text is not displayed as a hyperlink in the conversation. Instead, it appears as plain text. + +### What is the root cause of that problem? + +The root cause is in the `expensify-common` library's `ExpensiMark.js` file. The `MARKDOWN_LINK_REGEX` pattern at line 46 explicitly excludes newline characters (`\r` and `\n`) from the link text: + +```javascript +const MARKDOWN_LINK_REGEX = new RegExp(`\\[((?:[^\\[\\]\\r\\n]*(?:\\[[^\\[\\]\\r\\n]*][^\\[\\]\\r\\n]*)*))]\\(${UrlPatterns.MARKDOWN_URL_REGEX}\\)(?![^<]*(<\\/pre>|<\\/code>))`, 'gi'); +``` + +The character class `[^\\[\\]\\r\\n]*` matches any character **except** `[`, `]`, `\r`, and `\n`. This means when the link text contains a newline, the regex fails to match and the hyperlink is not parsed. + +### What changes do you think we should make in order to solve the problem? + +The fix needs to be made in the `expensify-common` repository. The `MARKDOWN_LINK_REGEX` should be updated to allow newline characters in the link text portion while still excluding brackets. + +The character class should be changed from `[^\\[\\]\\r\\n]*` to `[^\\[\\]]*` (or use a more permissive pattern with `[\s\S]` for the link text that still respects bracket boundaries). + +After the fix is deployed in `expensify-common`, the App's dependency should be updated to pull in the new version. + +### What alternative solutions did you explore? (Optional) + +N/A - The fix must be in the parsing regex itself since that's where the restriction is defined. There's no workaround at the App level without duplicating the markdown parsing logic. diff --git a/src/components/DraggableList/SortableItem.tsx b/src/components/DraggableList/SortableItem.tsx index 0f02eb013201e..b4c412bbdcbd1 100644 --- a/src/components/DraggableList/SortableItem.tsx +++ b/src/components/DraggableList/SortableItem.tsx @@ -14,10 +14,11 @@ function SortableItem({id, children, disabled = false}: SortableItemProps) { // Prevent Enter key from reaching MenuItem when dragging to avoid navigation conflicts const handleKeyDown = (e: React.KeyboardEvent) => { - if (isDragging && e.key === 'Enter') { - e.preventDefault(); - e.stopPropagation(); + if (!isDragging || e.key !== 'Enter') { + return; } + e.preventDefault(); + e.stopPropagation(); }; return ( diff --git a/src/components/DraggableList/index.tsx b/src/components/DraggableList/index.tsx index e560730aa6038..6b7d8aa5b0521 100644 --- a/src/components/DraggableList/index.tsx +++ b/src/components/DraggableList/index.tsx @@ -2,7 +2,7 @@ import type {DragEndEvent} from '@dnd-kit/core'; import {closestCenter, DndContext, KeyboardSensor, PointerSensor, useSensor, useSensors} from '@dnd-kit/core'; import {restrictToParentElement, restrictToVerticalAxis} from '@dnd-kit/modifiers'; import {arrayMove, SortableContext, sortableKeyboardCoordinates, verticalListSortingStrategy} from '@dnd-kit/sortable'; -import React, {Fragment, useEffect, useState} from 'react'; +import React, {Fragment, useEffect, useRef, useState} from 'react'; // eslint-disable-next-line no-restricted-imports import type {ScrollView as RNScrollView} from 'react-native'; import ScrollView from '@components/ScrollView'; @@ -31,10 +31,13 @@ function DraggableList({ // Generate a unique ID per mount to ensure DndContext state resets when component remounts const [instanceId] = useState(() => `${Date.now()}-${Math.random().toString(36).slice(2)}`); + // Track if a drag is currently active to avoid dispatching global Escape when not needed + const isDraggingRef = useRef(false); + // Cancel any active keyboard drag when the component unmounts to prevent ghost drag state useEffect(() => { return () => { - if (typeof document === 'undefined') { + if (typeof document === 'undefined' || !isDraggingRef.current) { return; } document.dispatchEvent(new KeyboardEvent('keydown', {key: 'Escape', code: 'Escape', bubbles: true, cancelable: true})); @@ -45,12 +48,17 @@ function DraggableList({ return keyExtractor(item, index); }); + const onDragStart = () => { + isDraggingRef.current = true; + }; + /** * Function to be called when the user finishes dragging an item * It will reorder the list and call the callback function * to notify the parent component about the change */ const onDragEnd = (event: DragEndEvent) => { + isDraggingRef.current = false; const {active, over} = event; if (over !== null && active.id !== over.id) { @@ -62,6 +70,10 @@ function DraggableList({ } }; + const onDragCancel = () => { + isDraggingRef.current = false; + }; + const sortableItems = data.map((item, index) => { const key = keyExtractor(item, index); // Check if item has a disabled property for dragging @@ -109,7 +121,9 @@ function DraggableList({
Date: Thu, 22 Jan 2026 21:14:43 +0100 Subject: [PATCH 3/4] chore: remove unrelated proposal files --- proposal-80150.md | 53 ------------------------------- proposal-80157.md | 80 ----------------------------------------------- proposal-80165.md | 77 --------------------------------------------- proposal-80233.md | 27 ---------------- 4 files changed, 237 deletions(-) delete mode 100644 proposal-80150.md delete mode 100644 proposal-80157.md delete mode 100644 proposal-80165.md delete mode 100644 proposal-80233.md diff --git a/proposal-80150.md b/proposal-80150.md deleted file mode 100644 index 25e96a8d10cbd..0000000000000 --- a/proposal-80150.md +++ /dev/null @@ -1,53 +0,0 @@ -## Proposal - -### Please re-state the problem that we are trying to solve in this issue. - -In the Left Hand Navigation (LHN), workspace expense rooms (policy expense chats) have noticeably less horizontal spacing than non-workspace rooms when in Focus/Compact mode. The text content appears 4px closer to the avatar than in standard chat rooms. - -### What is the root cause of that problem? - -The spacing inconsistency occurs due to different margin values applied to avatars based on their type: - -1. **Single avatars** in `OptionRowLHN.tsx` are given an explicit `styles.mr3` (12px margin-right) via `singleAvatarContainerStyle`: - https://github.com/Expensify/App/blob/e54cfd4428ac49ea5452c1e88c8938c0b1248d0a/src/components/LHNOptionsList/OptionRowLHN.tsx#L274 - -2. **Subscript avatars** (used for workspace expense rooms) use `StyleUtils.getContainerStyles(size)` which returns `styles.emptyAvatarMarginSmall` (8px margin-right) for `CONST.AVATAR_SIZE.SMALL`: - https://github.com/Expensify/App/blob/e54cfd4428ac49ea5452c1e88c8938c0b1248d0a/src/components/ReportActionAvatars/ReportActionAvatar.tsx#L194 - https://github.com/Expensify/App/blob/e54cfd4428ac49ea5452c1e88c8938c0b1248d0a/src/styles/index.ts#L2416-L2418 - -The 4px difference (`12px - 8px = 4px`) causes the visual inconsistency. In focus mode, avatar size is set to `SMALL` (line 268 of OptionRowLHN.tsx), which triggers this reduced margin for subscript avatars. - -### What changes do you think we should make in order to solve the problem? - -Add 4px margin-left (`styles.ml1`) to the content container specifically for policy expense chats in focus mode to compensate for the reduced avatar margin. This keeps the text alignment consistent across all room types. - -Modify line 127 of `/src/components/LHNOptionsList/OptionRowLHN.tsx`: - -**Current code:** -```typescript -const contentContainerStyles = isInFocusMode ? [styles.flex1, styles.flexRow, styles.overflowHidden, StyleUtils.getCompactContentContainerStyles()] : [styles.flex1]; -``` - -**Proposed code:** -```typescript -const contentContainerStyles = - isInFocusMode && optionItem?.isPolicyExpenseChat - ? [styles.flex1, styles.flexRow, styles.overflowHidden, StyleUtils.getCompactContentContainerStyles(), styles.ml1] - : isInFocusMode - ? [styles.flex1, styles.flexRow, styles.overflowHidden, StyleUtils.getCompactContentContainerStyles()] - : [styles.flex1]; -``` - -**Why this approach:** -- **Minimal change**: Only affects the text positioning for the specific case causing the issue -- **Targeted fix**: Doesn't modify global avatar styles that could affect other components -- **Uses existing patterns**: Similar conditional styling based on report type is already used elsewhere in the component (e.g., line 289-298 for `shouldUseFullTitle`) -- **Safe**: Only adds margin when both conditions are true (focus mode AND policy expense chat) - -### What alternative solutions did you explore? (Optional) - -1. **Modifying `emptyAvatarMarginSmall` to 12px**: This would affect all components using small subscript avatars globally, potentially causing unintended layout changes elsewhere. - -2. **Adding a new prop to ReportActionAvatars for subscript container styles**: More flexible but overly complex for this specific fix. - -3. **Using `shouldShowSubscript` instead of `isPolicyExpenseChat`**: While more comprehensive, this could affect other report types that may have intentionally different spacing. The issue specifically reports workspace expense rooms, so targeting `isPolicyExpenseChat` is appropriate. diff --git a/proposal-80157.md b/proposal-80157.md deleted file mode 100644 index e65a17f9147fa..0000000000000 --- a/proposal-80157.md +++ /dev/null @@ -1,80 +0,0 @@ -## Proposal - -### Please re-state the problem that we are trying to solve in this issue. - -When users are on the Search/Reports page with filters applied and select reports to export, a modal appears with the message "Export in progress - Concierge will send you the file shortly." This modal only has a confirm button. After confirming, users must manually navigate away from the Reports page to Concierge chat to download the exported file. This navigation causes users to lose their current filter context and workflow state. - -### What is the root cause of that problem? - -The export modal is displayed in `src/pages/Search/SearchPage.tsx` within the `beginExportWithTemplate` callback (lines 259-294): - -```typescript -const beginExportWithTemplate = useCallback( - async (templateName: string, templateType: string, policyID: string | undefined) => { - // ... export queuing logic ... - - const result = await showConfirmModal({ - title: translate('export.exportInProgress'), - prompt: translate('export.conciergeWillSend'), - confirmText: translate('common.buttonConfirm'), - shouldShowCancelButton: false, - }); - if (result.action !== ModalActions.CONFIRM) { - return; - } - clearSelectedTransactions(undefined, true); - }, - [queryJSON, selectedTransactionsKeys, areAllMatchingItemsSelected, selectedTransactionReportIDs, showConfirmModal, translate, clearSelectedTransactions], -); -``` - -After showing the confirmation modal telling users Concierge will send the file, the code simply clears the selected transactions and does nothing else. There's no way to access Concierge without manually navigating away from the Reports page. - -The app already has Side Panel functionality that can show Concierge without navigation. The Side Panel renders Concierge by default using the `conciergeReportID` from the `SidePanelContextProvider`. - -### What changes do you think we should make in order to solve the problem? - -After the export confirmation modal is dismissed, automatically open the Side Panel on desktop to show Concierge. Users can then receive and download the file without leaving the Reports page or losing their filters. - -In `src/pages/Search/SearchPage.tsx`, modify the `beginExportWithTemplate` callback: - -```typescript -// At the top of the file, add import: -import useSidePanel from '@hooks/useSidePanel'; - -// Inside the SearchPage component, get the openSidePanel function: -const {openSidePanel} = useSidePanel(); - -// Modify beginExportWithTemplate callback: -const beginExportWithTemplate = useCallback( - async (templateName: string, templateType: string, policyID: string | undefined) => { - // ... existing export queuing logic ... - - const result = await showConfirmModal({ - title: translate('export.exportInProgress'), - prompt: translate('export.conciergeWillSend'), - confirmText: translate('common.buttonConfirm'), - shouldShowCancelButton: false, - }); - if (result.action !== ModalActions.CONFIRM) { - return; - } - clearSelectedTransactions(undefined, true); - - // Open Concierge in Side Panel on desktop only - // shouldUseNarrowLayout is false on desktop - if (!shouldUseNarrowLayout) { - openSidePanel(); - } - }, - [queryJSON, selectedTransactionsKeys, areAllMatchingItemsSelected, selectedTransactionReportIDs, showConfirmModal, translate, clearSelectedTransactions, shouldUseNarrowLayout, openSidePanel], -); -``` - -The same pattern should be applied to `MoneyReportHeader.tsx` and `MoneyRequestReportActionsList.tsx` which also have `beginExportWithTemplate` implementations. - -To test, go to the Search page and apply filters like date range or status. Select some expenses/reports and click Export, then confirm the modal. On desktop, the Side Panel should open with Concierge while the filters remain intact. On mobile, nothing should change. Wait for Concierge to send the file and verify it can be downloaded from the Side Panel without the original filters being lost. - -### What alternative solutions did you explore? (Optional) - -Navigating to Concierge after the modal would defeat the purpose since users would still lose their context. Adding a button to the modal to go to Concierge is more intrusive and requires extra user action. Showing Concierge in a popup would require new modal infrastructure. Using the Side Panel works because it's already designed for this use case. diff --git a/proposal-80165.md b/proposal-80165.md deleted file mode 100644 index 46046bc9a2c35..0000000000000 --- a/proposal-80165.md +++ /dev/null @@ -1,77 +0,0 @@ -# Proposal for Issue #80165 - -## Please re-state the problem that we are trying to solve in this issue. - -When using keyboard navigation to reorder waypoints in the distance tracking feature, the "Please remove duplicate waypoints" error message persists after dropping an item with the **Enter** key. For example, when waypoints A > B > B (which shows the duplicate error) are reordered to B > A > B, the error remains visible until the user tabs away from the moved item. - -**Key observation:** The feature works correctly when pressing **Space** to finalize the reorder instead of Enter. The error properly clears in that case. - -This was introduced by PR #79793 which added keyboard reordering support to the DraggableList component. - -## What is the root cause of that problem? - -The root cause is related to how @dnd-kit handles the Enter vs Space key for finalizing a drag operation, and the focus/blur cycle that follows. - -Looking at `src/components/DraggableList/SortableItem.tsx`, the component spreads `listeners` from `useSortable` which includes keyboard event handlers. When: - -1. **Space is pressed**: The drag operation ends cleanly. The `onDragEnd` callback fires, waypoints state updates, and `duplicateWaypointsError` is recalculated correctly to `false`. - -2. **Enter is pressed**: The drag operation ends, but Enter key has additional behaviors in @dnd-kit. The Enter key can trigger a "click" or "activation" on the focused element after the drag ends. This causes the error recalculation to be interrupted or the component to be in a transitional focus state where the error doesn't update until focus changes. - -The evidence for this: -- Error clears immediately with Space -- Error persists with Enter but clears when tabbing away (focus change) -- The same behavior affects issue #80166 (waypoint cannot be dragged after Enter) - -In `src/pages/iou/request/step/IOURequestStepDistanceMap.tsx`, the `duplicateWaypointsError` is computed in a `useMemo` that depends on `waypoints`. When Enter is used, there appears to be a focus-related timing issue where React doesn't re-render with the updated error state until the focus changes. - -## What changes do you think we should make in order to solve the problem? - -In `src/components/DraggableList/SortableItem.tsx`, we should intercept the Enter key during an active drag to prevent any additional click/activation behavior after the drop completes: - -```typescript -function SortableItem({id, children, disabled = false}: SortableItemProps) { - const {attributes, listeners, setNodeRef, transform, transition, isDragging} = useSortable({id, disabled}); - const wasDraggingRef = useRef(false); - - // Track if we were just dragging to prevent Enter from triggering click - useEffect(() => { - if (isDragging) { - wasDraggingRef.current = true; - } - }, [isDragging]); - - const handleKeyDown = (e: React.KeyboardEvent) => { - // Call the original listener first - listeners?.onKeyDown?.(e); - - // If Enter was pressed while dragging or just after dropping, - // prevent additional activation behavior - if (e.key === 'Enter' && wasDraggingRef.current) { - e.preventDefault(); - e.stopPropagation(); - // Reset the flag after a brief delay to allow normal Enter behavior later - setTimeout(() => { - wasDraggingRef.current = false; - }, 0); - } - }; - - // ... rest of component -} -``` - -This ensures Enter behaves consistently with Space by preventing any post-drop activation that interferes with state updates. - -## What alternative solutions did you explore? (Optional) - -1. **Configure KeyboardSensor to only use Space for drop**: We could restrict the drop key to only Space via `keyboardCodes.end`. However, Enter is a standard and expected key for confirming actions, so this would reduce accessibility. - -2. **Force blur after keyboard drop**: We could programmatically blur the element after a keyboard drop, but this would be jarring for keyboard users who expect focus to remain on the item they just moved. - -3. **Debounce error display**: This would mask the symptom rather than fix the root cause. - -**Note:** Per the discussion in the issue thread, this is a UX improvement rather than a deploy blocker since: -- The feature works correctly with Space -- The error clears when tabbing away -- The core functionality is not broken diff --git a/proposal-80233.md b/proposal-80233.md deleted file mode 100644 index 17ab5b6b2b63d..0000000000000 --- a/proposal-80233.md +++ /dev/null @@ -1,27 +0,0 @@ -## Proposal - -### Please re-state the problem that we are trying to solve in this issue. - -When a user types a markdown hyperlink with multiline text (e.g., `[line1\nline2\nline3](https://example.com)`), the text is not displayed as a hyperlink in the conversation. Instead, it appears as plain text. - -### What is the root cause of that problem? - -The root cause is in the `expensify-common` library's `ExpensiMark.js` file. The `MARKDOWN_LINK_REGEX` pattern at line 46 explicitly excludes newline characters (`\r` and `\n`) from the link text: - -```javascript -const MARKDOWN_LINK_REGEX = new RegExp(`\\[((?:[^\\[\\]\\r\\n]*(?:\\[[^\\[\\]\\r\\n]*][^\\[\\]\\r\\n]*)*))]\\(${UrlPatterns.MARKDOWN_URL_REGEX}\\)(?![^<]*(<\\/pre>|<\\/code>))`, 'gi'); -``` - -The character class `[^\\[\\]\\r\\n]*` matches any character **except** `[`, `]`, `\r`, and `\n`. This means when the link text contains a newline, the regex fails to match and the hyperlink is not parsed. - -### What changes do you think we should make in order to solve the problem? - -The fix needs to be made in the `expensify-common` repository. The `MARKDOWN_LINK_REGEX` should be updated to allow newline characters in the link text portion while still excluding brackets. - -The character class should be changed from `[^\\[\\]\\r\\n]*` to `[^\\[\\]]*` (or use a more permissive pattern with `[\s\S]` for the link text that still respects bracket boundaries). - -After the fix is deployed in `expensify-common`, the App's dependency should be updated to pull in the new version. - -### What alternative solutions did you explore? (Optional) - -N/A - The fix must be in the parsing regex itself since that's where the restriction is defined. There's no workaround at the App level without duplicating the markdown parsing logic. From 43ed83a49420f8ca3629140832e4210597cda006 Mon Sep 17 00:00:00 2001 From: MobileMage Date: Wed, 25 Feb 2026 10:23:12 +0100 Subject: [PATCH 4/4] Use React useId instead of manual ID generation --- src/components/DraggableList/index.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/DraggableList/index.tsx b/src/components/DraggableList/index.tsx index 6b7d8aa5b0521..09660a743dfb5 100644 --- a/src/components/DraggableList/index.tsx +++ b/src/components/DraggableList/index.tsx @@ -2,7 +2,7 @@ import type {DragEndEvent} from '@dnd-kit/core'; import {closestCenter, DndContext, KeyboardSensor, PointerSensor, useSensor, useSensors} from '@dnd-kit/core'; import {restrictToParentElement, restrictToVerticalAxis} from '@dnd-kit/modifiers'; import {arrayMove, SortableContext, sortableKeyboardCoordinates, verticalListSortingStrategy} from '@dnd-kit/sortable'; -import React, {Fragment, useEffect, useRef, useState} from 'react'; +import React, {Fragment, useEffect, useId, useRef} from 'react'; // eslint-disable-next-line no-restricted-imports import type {ScrollView as RNScrollView} from 'react-native'; import ScrollView from '@components/ScrollView'; @@ -28,8 +28,8 @@ function DraggableList({ }: DraggableListProps & {ref?: React.ForwardedRef}) { const styles = useThemeStyles(); - // Generate a unique ID per mount to ensure DndContext state resets when component remounts - const [instanceId] = useState(() => `${Date.now()}-${Math.random().toString(36).slice(2)}`); + // Unique ID per mount to ensure DndContext state resets when component remounts + const instanceId = useId(); // Track if a drag is currently active to avoid dispatching global Escape when not needed const isDraggingRef = useRef(false);