From ceaa9cea39b6ff2d21e0ab184ee40c24abc68cfb Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:12:32 +0100 Subject: [PATCH] fix: highlight only the open message in lists that mix accounts, not another account's message with the same id --- .../email-list-scroll-reset.test.tsx | 2 +- .../__tests__/email-list-skeleton.test.tsx | 2 +- .../email/__tests__/thread-list-item.test.tsx | 19 ++++++++++++++ .../email/__tests__/thread-row-memo.test.ts | 24 ++++++++++++------ components/email/email-list.tsx | 15 +++++------ components/email/thread-list-item.tsx | 25 ++++++++++--------- components/mail/mail-app.tsx | 2 +- 7 files changed, 59 insertions(+), 30 deletions(-) diff --git a/components/email/__tests__/email-list-scroll-reset.test.tsx b/components/email/__tests__/email-list-scroll-reset.test.tsx index d6ac07fe5..0ce74a5ed 100644 --- a/components/email/__tests__/email-list-scroll-reset.test.tsx +++ b/components/email/__tests__/email-list-scroll-reset.test.tsx @@ -125,7 +125,7 @@ describe('email list scroll position', () => { scrollTo.mockClear(); act(() => { useEmailStore.getState().selectMailbox('archive'); - view.rerender(); + view.rerender(); }); // The reset comes first and the scroll down to the selected mail after // it - the other way round, the reset would leave the mail out of view. diff --git a/components/email/__tests__/email-list-skeleton.test.tsx b/components/email/__tests__/email-list-skeleton.test.tsx index 0f6221deb..52cdb77c6 100644 --- a/components/email/__tests__/email-list-skeleton.test.tsx +++ b/components/email/__tests__/email-list-skeleton.test.tsx @@ -39,7 +39,7 @@ describe('email list loading skeleton', () => { const skeleton = screen.getByTestId('email-list-skeleton'); act(() => { useEmailStore.setState({ mailboxes: [], isLoadingMore: false, hasMoreEmails: true }); }); - view.rerender(); + view.rerender(); expect(screen.getByTestId('email-list-skeleton')).toBe(skeleton); }); diff --git a/components/email/__tests__/thread-list-item.test.tsx b/components/email/__tests__/thread-list-item.test.tsx index 99156b232..712f7d7ff 100644 --- a/components/email/__tests__/thread-list-item.test.tsx +++ b/components/email/__tests__/thread-list-item.test.tsx @@ -809,3 +809,22 @@ describe('ThreadListItem hover actions', () => { expect(screen.getByTestId('hover-action-delete')).toBeTruthy(); }); }); + +describe('ThreadListItem open message', () => { + beforeEach(() => { + useSettingsStore.setState({ emailKeywords: [...DEFAULT_KEYWORDS], showPreview: false, mailLayout: 'split', interfaceLayout: 'default' }); + useEmailStore.setState({ selectedEmailKeys: new Set(), selectedMailbox: '' }); + }); + + // A tag view lists the own and the group's eaaaaab: opening one draws only that one open. + it.each([ + ['own', makeEmail({ id: 'eaaaaab', sourceClientAccountId: 'login-a', sourceAccountId: 'cf' }), true], + ['group', makeEmail({ id: 'eaaaaab', sourceClientAccountId: 'login-a', sourceAccountId: 'ce' }), false], + ])('draws the %s row as open only when it is the open message', (_name, email, open) => { + const [thread] = groupEmailsByThread([email]); + const { container } = render( + {}} onEmailSelect={() => {}} />, + ); + expect(container.querySelector('[aria-current="true"]') !== null).toBe(open); + }); +}); diff --git a/components/email/__tests__/thread-row-memo.test.ts b/components/email/__tests__/thread-row-memo.test.ts index 901aaf13b..9217b39a7 100644 --- a/components/email/__tests__/thread-row-memo.test.ts +++ b/components/email/__tests__/thread-row-memo.test.ts @@ -64,25 +64,33 @@ describe('areThreadRowPropsEqual', () => { it('ignores the open message moving between two OTHER rows', () => { const thread = group([email('e1')]); expect(areThreadRowPropsEqual( - props({ thread, selectedEmailId: 'other-1' }), - props({ thread, selectedEmailId: 'other-2' }), + props({ thread, selectedEmailKey: 'other-1' }), + props({ thread, selectedEmailKey: 'other-2' }), )).toBe(true); - expect(areThreadRowPropsEqual(props({ thread }), props({ thread, selectedEmailId: 'other-1' }))).toBe(true); + expect(areThreadRowPropsEqual(props({ thread }), props({ thread, selectedEmailKey: 'other-1' }))).toBe(true); }); it('re-renders the row that gains or loses the open message', () => { const thread = group([email('e1'), email('e2')]); - expect(areThreadRowPropsEqual(props({ thread, selectedEmailId: 'other' }), props({ thread, selectedEmailId: 'e2' }))).toBe(false); - expect(areThreadRowPropsEqual(props({ thread, selectedEmailId: 'e2' }), props({ thread, selectedEmailId: 'other' }))).toBe(false); - expect(areThreadRowPropsEqual(props({ thread, selectedEmailId: 'e1' }), props({ thread, selectedEmailId: 'e2' }))).toBe(false); + expect(areThreadRowPropsEqual(props({ thread, selectedEmailKey: 'other' }), props({ thread, selectedEmailKey: 'e2' }))).toBe(false); + expect(areThreadRowPropsEqual(props({ thread, selectedEmailKey: 'e2' }), props({ thread, selectedEmailKey: 'other' }))).toBe(false); + expect(areThreadRowPropsEqual(props({ thread, selectedEmailKey: 'e1' }), props({ thread, selectedEmailKey: 'e2' }))).toBe(false); + }); + + // Ids repeat across accounts: another account's message with the open + // message's id is not drawn as open. + it('tells the open message from a namesake of another account', () => { + const thread = group([{ ...email('e1'), sourceClientAccountId: 'login-b', sourceAccountId: 'cf' }]); + expect(areThreadRowPropsEqual(props({ thread, selectedEmailKey: 'other' }), props({ thread, selectedEmailKey: 'e1' }))).toBe(true); + expect(areThreadRowPropsEqual(props({ thread, selectedEmailKey: 'other' }), props({ thread, selectedEmailKey: 'login-b/cf:e1' }))).toBe(false); }); it('counts messages shown by the expanded thread as drawn by the row', () => { const thread = group([email('e1')]); const expandedEmails = [email('e1'), email('e9')]; expect(areThreadRowPropsEqual( - props({ thread, expandedEmails, isExpanded: true, selectedEmailId: 'other' }), - props({ thread, expandedEmails, isExpanded: true, selectedEmailId: 'e9' }), + props({ thread, expandedEmails, isExpanded: true, selectedEmailKey: 'other' }), + props({ thread, expandedEmails, isExpanded: true, selectedEmailKey: 'e9' }), )).toBe(false); }); diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index 1d3049a42..011df3db0 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -45,7 +45,8 @@ import { MvListSectionHeader } from "./mv-list-section-header"; interface EmailListProps { emails: Email[]; - selectedEmailId?: string; + /** emailKeyFor of the open message: ids repeat across accounts. */ + selectedEmailKey?: string; onEmailSelect?: (email: Email) => void; /** A mouse was pressed on a row: the click that follows will open it. */ onEmailPrefetch?: (email: Email) => void; @@ -125,7 +126,7 @@ function LoadingSkeleton() { export function EmailList({ emails, - selectedEmailId, + selectedEmailKey, onEmailSelect, onEmailPrefetch, onEmailDoubleClick, @@ -769,10 +770,10 @@ export function EmailList({ // Scroll to the thread group containing the selected email useEffect(() => { - if (!selectedEmailId) return; + if (!selectedEmailKey) return; const index = threadGroups.findIndex(thread => - thread.latestEmail.id === selectedEmailId || - thread.emails.some(e => e.id === selectedEmailId) + emailKeyFor(thread.latestEmail) === selectedEmailKey || + thread.emails.some(e => emailKeyFor(e) === selectedEmailKey) ); if (index < 0) return; // A row clicked in the list is already on screen; scrollToIndex would @@ -785,7 +786,7 @@ export function EmailList({ if (item && top !== null && height !== undefined && item.start >= top && item.end <= top + height) return; virtualizer.scrollToIndex(itemIndex, { align: 'auto' }); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [selectedEmailId]); + }, [selectedEmailKey]); // Keep the moving end of a Shift+Arrow range on screen (#992). const selectionFocusKey = useEmailStore((state) => state.selectionFocus?.focus ?? null); @@ -1070,7 +1071,7 @@ export function EmailList({ {thread && void; @@ -905,7 +906,7 @@ export const ThreadListItem = React.forwardRef e.id === selectedEmailId); + const isSelected = selectedEmailKey === emailKeyFor(latestEmail) || + thread.emails.some(e => emailKeyFor(e) === selectedEmailKey); const isChecked = thread.emails.some(e => selectedEmailKeys.has(emailKeyFor(e))); @@ -1029,7 +1030,7 @@ export const ThreadListItem = React.forwardRef onEmailSelect(latestEmail)} onDoubleClick={onEmailDoubleClick ? () => onEmailDoubleClick(latestEmail) : undefined} onPrefetch={onPrefetch ? () => onPrefetch(latestEmail) : undefined} @@ -1571,7 +1572,7 @@ export const ThreadListItem = React.forwardRef onEmailSelect(email)} onDoubleClick={onEmailDoubleClick ? () => onEmailDoubleClick(email) : undefined} @@ -1613,11 +1614,11 @@ function isSameThread(a: ThreadGroup, b: ThreadGroup): boolean { /** The open message, if this row draws it; otherwise none of the row's business. */ function openMessageWithin(props: ThreadListItemProps): string | undefined { - const id = props.selectedEmailId; - if (!id) return undefined; - const drawn = props.thread.emails.some((email) => email.id === id) - || (props.expandedEmails?.some((email) => email.id === id) ?? false); - return drawn ? id : undefined; + const key = props.selectedEmailKey; + if (!key) return undefined; + const drawn = props.thread.emails.some((email) => emailKeyFor(email) === key) + || (props.expandedEmails?.some((email) => emailKeyFor(email) === key) ?? false); + return drawn ? key : undefined; } /** Props equality for MemoizedThreadListItem. */ @@ -1626,7 +1627,7 @@ export function areThreadRowPropsEqual(prev: ThreadListItemProps, next: ThreadLi for (const key of keys) { if (key === 'thread') { if (!isSameThread(prev.thread, next.thread)) return false; - } else if (key === 'selectedEmailId') { + } else if (key === 'selectedEmailKey') { if (openMessageWithin(prev) !== openMessageWithin(next)) return false; } else if (!Object.is(prev[key], next[key])) { return false; diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 744aba54f..4771084fd 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -5165,7 +5165,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { onFindFromSender={mvShell ? (sender) => void handleSelectContactSuggestion(sender, 'from') : undefined} hideBatchToolbar={mvShell} emails={activeEmails} - selectedEmailId={selectedEmail?.id} + selectedEmailKey={selectedEmail ? emailKeyFor(selectedEmail) : undefined} isLoading={activeIsLoading} hasMore={activeHasMore} isLoadingMoreItems={isScheduledView ? isLoadingScheduled && activeEmails.length > 0 : undefined}