fix: highlight only the open message in lists that mix accounts, not another account's message with the same id

This commit is contained in:
Linus Rath
2026-10-26 16:11:52 +01:00
parent 54b197080c
commit ceaa9cea39
7 changed files with 59 additions and 30 deletions
@@ -125,7 +125,7 @@ describe('email list scroll position', () => {
scrollTo.mockClear();
act(() => {
useEmailStore.getState().selectMailbox('archive');
view.rerender(<EmailList emails={mails} selectedEmailId="m20" />);
view.rerender(<EmailList emails={mails} selectedEmailKey="m20" />);
});
// 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.
@@ -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(<EmailList emails={[]} isLoading selectedEmailId="x" />);
view.rerender(<EmailList emails={[]} isLoading selectedEmailKey="x" />);
expect(screen.getByTestId('email-list-skeleton')).toBe(skeleton);
});
@@ -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<string>(), 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(
<ThreadListItem thread={thread} isExpanded={false} selectedEmailKey="login-a/cf:eaaaaab" onToggleExpand={() => {}} onEmailSelect={() => {}} />,
);
expect(container.querySelector('[aria-current="true"]') !== null).toBe(open);
});
});
@@ -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);
});
+8 -7
View File
@@ -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 && <MemoizedThreadListItem
thread={thread}
isExpanded={expandedThreadIds.has(thread.threadKey)}
selectedEmailId={selectedEmailId}
selectedEmailKey={selectedEmailKey}
isLoading={isLoadingThread === thread.threadKey}
expandedEmails={threadEmailsCache.get(thread.threadKey)}
onToggleExpand={toggleExpandFor(thread.threadKey)}
+13 -12
View File
@@ -299,7 +299,8 @@ function ThreadCountPill({ count, hasUnread, title }: { count: number; hasUnread
interface ThreadListItemProps {
thread: ThreadGroup;
isExpanded: boolean;
selectedEmailId?: string;
/** emailKeyFor of the open message: ids repeat across accounts. */
selectedEmailKey?: string;
isLoading?: boolean;
expandedEmails?: Email[];
onToggleExpand: () => void;
@@ -905,7 +906,7 @@ export const ThreadListItem = React.forwardRef<HTMLDivElement, ThreadListItemPro
loadAttachments,
thread,
isExpanded,
selectedEmailId,
selectedEmailKey,
isLoading = false,
expandedEmails,
onToggleExpand,
@@ -1019,8 +1020,8 @@ export const ThreadListItem = React.forwardRef<HTMLDivElement, ThreadListItemPro
const rowTint = accountRowTint
?? ((tintListRowsByTag && tagIds[0]) ? tagColor(tagIds[0]).rowTint : null);
const isSelected = selectedEmailId === latestEmail.id ||
thread.emails.some(e => 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<HTMLDivElement, ThreadListItemPro
<SingleEmailItem
ref={ref}
email={latestEmail}
selected={selectedEmailId === latestEmail.id}
selected={selectedEmailKey === emailKeyFor(latestEmail)}
onClick={() => onEmailSelect(latestEmail)}
onDoubleClick={onEmailDoubleClick ? () => onEmailDoubleClick(latestEmail) : undefined}
onPrefetch={onPrefetch ? () => onPrefetch(latestEmail) : undefined}
@@ -1571,7 +1572,7 @@ export const ThreadListItem = React.forwardRef<HTMLDivElement, ThreadListItemPro
<ThreadEmailItem
key={email.id}
email={email}
selected={email.id === selectedEmailId}
selected={emailKeyFor(email) === selectedEmailKey}
isLast={index === emailsToShow.length - 1}
onClick={() => 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;
+1 -1
View File
@@ -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}