From 5365b1a4612e40ca69da0c0f2d8befd5468f0c33 Mon Sep 17 00:00:00 2001
From: Linus Rath <139418639+rathlinus@users.noreply.github.com>
Date: Wed, 4 Nov 2026 13:59:28 +0100
Subject: [PATCH] fix: lay out addresses in the merge preview as the card does,
 list every field the card keeps, and sort Merge with by name

The contacts-app merge test now expects the address on its own lines instead of one comma-separated line, which was the bug.
---
 .../contact-merge-preview-ux-p2.test.tsx      | 56 +++++++++++++++
 .../contacts/__tests__/contacts-app.test.tsx  |  3 +-
 components/contacts/contact-merge-dialog.tsx  | 68 +++++++++++--------
 3 files changed, 96 insertions(+), 31 deletions(-)
 create mode 100644 components/contacts/__tests__/contact-merge-preview-ux-p2.test.tsx

diff --git a/components/contacts/__tests__/contact-merge-preview-ux-p2.test.tsx b/components/contacts/__tests__/contact-merge-preview-ux-p2.test.tsx
new file mode 100644
index 000000000..61ac6009d
--- /dev/null
+++ b/components/contacts/__tests__/contact-merge-preview-ux-p2.test.tsx
@@ -0,0 +1,56 @@
+import { cleanup, fireEvent, render, screen, within } from '@testing-library/react';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import type { ContactCard } from '@/lib/jmap/types';
+import { ContactMergeDialog } from '../contact-merge-dialog';
+
+// pim-p2-14 and pim-p2-15 (2026-11-04): the merge preview listed "Hauptstraße 5,
+// Berlin, 10115, Germany" where the card shows "Hauptstraße 5" / "10115 Berlin", and
+// left out the birthday the merge keeps. "Merge with…" listed every contact unsorted.
+
+afterEach(() => cleanup());
+
+const anna = {
+  id: 'anna', kind: 'individual', name: { full: 'Anna Schmidt' },
+  emails: { e: { address: 'anna@example.org' } },
+  addresses: {
+    a: {
+      countryCode: 'DE',
+      components: [
+        { kind: 'name', value: 'Hauptstraße' }, { kind: 'number', value: '5' },
+        { kind: 'locality', value: 'Berlin' }, { kind: 'postcode', value: '10115' }, { kind: 'country', value: 'Germany' },
+      ],
+    },
+  },
+  updated: '2026-11-01T10:00:00Z',
+} as unknown as ContactCard;
+
+const annaOld = {
+  id: 'anna-old', kind: 'individual', name: { full: 'Anna Schmidt' },
+  emails: { e: { address: 'anna.s@example.org' } },
+  anniversaries: { b: { kind: 'birth', date: { '@type': 'PartialDate', year: 1990, month: 3, day: 14 } } },
+  updated: '2025-01-01T10:00:00Z',
+} as unknown as ContactCard;
+
+const named = (id: string, full: string) => ({ id, kind: 'individual', name: { full } }) as unknown as ContactCard;
+
+describe('the merge preview', () => {
+  it('lays the address out as the card does and lists the birthday it keeps', () => {
+    render(<ContactMergeDialog cards={[anna, annaOld]} groups={[]} onMerge={vi.fn()} onClose={vi.fn()} />);
+    const preview = screen.getByTestId('merge-preview');
+    const address = within(preview).getByText('group_adr_any').nextElementSibling as HTMLElement;
+    expect(address.textContent).toBe('Hauptstraße 5\n10115 Berlin\nGermany');
+    const birthday = within(preview).getByText('detail.anniversary_birth').nextElementSibling as HTMLElement;
+    expect(birthday.textContent).toMatch(/1990/);
+  });
+});
+
+describe('Merge with…', () => {
+  it('lists the contacts to pick by name', () => {
+    const candidates = [named('3', 'Jürgen'), named('1', 'Alice'), named('5', 'Zoë'), named('2', 'anna'), named('4', 'Bob')];
+    render(<ContactMergeDialog pickFor={named('0', 'Me')} candidates={candidates} groups={[]} onMerge={vi.fn()} onClose={vi.fn()} />);
+    const rows = within(screen.getByTestId('merge-pick-list')).getAllByRole('button');
+    expect(rows.map((row) => row.textContent)).toEqual(['Alice', 'anna', 'Bob', 'Jürgen', 'Zoë']);
+    fireEvent.click(rows[0]);
+    expect(screen.getByTestId('merge-preview')).toBeInTheDocument();
+  });
+});
diff --git a/components/contacts/__tests__/contacts-app.test.tsx b/components/contacts/__tests__/contacts-app.test.tsx
index 59c673620..ab4c422ed 100644
--- a/components/contacts/__tests__/contacts-app.test.tsx
+++ b/components/contacts/__tests__/contacts-app.test.tsx
@@ -322,7 +322,8 @@ describe('ContactsApp', () => {
       const preview = await screen.findByTestId('merge-preview');
       expect(within(preview).getByText('anna@example.net')).toBeInTheDocument();
       expect(within(preview).getByText('anna@home.example.net')).toBeInTheDocument();
-      expect(within(preview).getByText('Hauptstraße 1, Berlin')).toBeInTheDocument();
+      // One line per address line, as the card shows it (pim-p2-14); getByText joins the lines with a space.
+      expect(within(preview).getByText('Hauptstraße 1 Berlin')).toBeInTheDocument();
       expect(within(preview).getByText('Club')).toBeInTheDocument();
       fireEvent.click(screen.getByRole('button', { name: 'merge' }));
 
diff --git a/components/contacts/contact-merge-dialog.tsx b/components/contacts/contact-merge-dialog.tsx
index bc48e5e99..e3cf539f9 100644
--- a/components/contacts/contact-merge-dialog.tsx
+++ b/components/contacts/contact-merge-dialog.tsx
@@ -1,39 +1,20 @@
 "use client";
 
 import { useMemo, useState } from "react";
-import { useTranslations } from "next-intl";
+import { useLocale, useTranslations } from "next-intl";
 import { Search } from "@/components/icons";
 import { Button } from "@/components/ui/button";
 import { Dialog } from "@/components/ui/dialog";
 import { Input } from "@/components/ui/input";
+import { formatAddressLines } from "@/lib/address-format";
+import { formatContactDate } from "@/lib/contact-dates";
 import { cn } from "@/lib/utils";
-import type { ContactAddress, ContactCard } from "@/lib/jmap/types";
+import type { ContactCard } from "@/lib/jmap/types";
 import { memberKeys, planMerge, type MergePlan } from "@/lib/contacts/merge";
 import { getContactDisplayName, getContactPrimaryEmail } from "@/stores/contact-store";
 
-/** One line for an address, as its parts and separators say. */
-export function addressLine(address: ContactAddress): string {
-  const components = address.components ?? [];
-  if (components.length > 0) {
-    let out = "";
-    let lastWasValue = false;
-    for (const c of components) {
-      if (c.kind === "separator") {
-        out += c.value;
-        lastWasValue = false;
-        continue;
-      }
-      if (!c.value) continue;
-      if (lastWasValue) out += address.defaultSeparator ?? ", ";
-      out += c.value.replace(/\s*\n\s*/g, ", ");
-      lastWasValue = true;
-    }
-    return out;
-  }
-  const legacy = [address.street, [address.postcode, address.locality].filter(Boolean).join(" "), address.region, address.country]
-    .filter(Boolean).join(", ");
-  return legacy || address.full || address.fullAddress || "";
-}
+/** Lines of one value, shown with whitespace-pre-line. */
+const NEWLINE = String.fromCharCode(10);
 
 interface ContactMergeDialogProps {
   /** Two or more cards: the dialog opens on what the merge keeps. */
@@ -69,11 +50,14 @@ function PickRow({ card, onPick }: { card: ContactCard; onPick: () => void }) {
 export function ContactMergeDialog({ cards: initialCards, pickFor, candidates = [], suggested = [], groups, onMerge, onClose }: ContactMergeDialogProps) {
   const t = useTranslations("ux_pim_cimport");
   const tc = useTranslations("contacts");
+  const tm = useTranslations("ux_p2_pim_cm");
+  const locale = useLocale();
   const [cards, setCards] = useState<ContactCard[] | null>(initialCards && initialCards.length > 1 ? initialCards : null);
   const [query, setQuery] = useState("");
   const [busy, setBusy] = useState(false);
 
   const plan = useMemo(() => (cards ? planMerge(cards) : null), [cards]);
+  const collator = useMemo(() => new Intl.Collator(locale, { sensitivity: "base", numeric: true }), [locale]);
 
   const filtered = useMemo(() => {
     const q = query.trim().toLowerCase();
@@ -81,8 +65,10 @@ export function ContactMergeDialog({ cards: initialCards, pickFor, candidates =
     return candidates
       .filter((c) => !suggestedIds.has(c.id))
       .filter((c) => !q || `${getContactDisplayName(c)} ${Object.values(c.emails ?? {}).map((e) => e.address).join(" ")}`.toLowerCase().includes(q))
+      // By name, as the contact list sorts, in the UI language's order.
+      .sort((a, b) => collator.compare(getContactDisplayName(a) || getContactPrimaryEmail(a) || "", getContactDisplayName(b) || getContactPrimaryEmail(b) || ""))
       .slice(0, 200);
-  }, [candidates, suggested, query]);
+  }, [candidates, suggested, query, collator]);
 
   if (!plan) {
     const name = pickFor ? getContactDisplayName(pickFor) || getContactPrimaryEmail(pickFor) : "";
@@ -135,11 +121,12 @@ export function ContactMergeDialog({ cards: initialCards, pickFor, candidates =
     .filter((g) => Object.entries(g.members ?? {}).some(([k, on]) => on && keys.has(k)))
     .map((g) => getContactDisplayName(g))
     .filter(Boolean);
-  const rows: Array<{ label: string; values: string[] }> = [
+  const rows: Array<{ label: string; values: string[]; multiline?: boolean }> = [
     { label: t("group_name"), values: [getContactDisplayName(r)].filter(Boolean) },
     { label: t("group_email"), values: Object.values(r.emails ?? {}).map((e) => e.address) },
     { label: t("group_phone"), values: Object.values(r.phones ?? {}).map((p) => p.number) },
-    { label: t("group_adr_any"), values: Object.values(r.addresses ?? {}).map(addressLine).filter(Boolean) },
+    // Laid out as the card shows it ("Hauptstraße 5" / "10115 Berlin"), not the parts in stored order.
+    { label: t("group_adr_any"), values: Object.values(r.addresses ?? {}).map((a) => formatAddressLines(a, locale).join(NEWLINE)).filter(Boolean), multiline: true },
     {
       label: t("group_org"),
       values: [
@@ -147,8 +134,29 @@ export function ContactMergeDialog({ cards: initialCards, pickFor, candidates =
         ...Object.values(r.titles ?? {}).map((ti) => ti.name),
       ].filter(Boolean),
     },
+    { label: t("t_nickname"), values: Object.values(r.nicknames ?? {}).map((n) => n.name).filter(Boolean) },
+    // Every date the kept card will have, each under its own name ("Birthday").
+    ...(["birth", "wedding", "death", "other"] as const).map((kind) => ({
+      label: tc(`detail.anniversary_${kind}`),
+      values: Object.values(r.anniversaries ?? {}).filter((a) => a.kind === kind && a.date).map((a) => formatContactDate(a.date, locale)),
+    })),
+    {
+      label: tc("detail.gender"),
+      values: [
+        r.speakToAs?.grammaticalGender ? tc(`detail.gender_${r.speakToAs.grammaticalGender}`, { defaultValue: r.speakToAs.grammaticalGender }) : "",
+        ...Object.values(r.speakToAs?.pronouns ?? {}).map((p) => p.pronouns),
+      ].filter(Boolean),
+    },
+    { label: tc("detail.language_label"), values: Object.values(r.preferredLanguages ?? {}).map((l) => l.language).filter(Boolean) },
+    { label: tc("detail.personal_info"), values: Object.values(r.personalInfo ?? {}).map((pi) => pi.value).filter(Boolean) },
     { label: t("t_web"), values: Object.values(r.onlineServices ?? {}).map((s) => s.uri || s.user || "").filter(Boolean) },
-    { label: t("t_notes"), values: Object.values(r.notes ?? {}).map((n) => n.note).filter(Boolean) },
+    { label: tm("merge_links"), values: [...Object.values(r.links ?? {}), ...Object.values(r.directories ?? {})].map((l) => l.uri).filter(Boolean) },
+    { label: tc("detail.calendar_uri"), values: Object.values(r.calendars ?? {}).map((c) => c.uri).filter(Boolean) },
+    { label: tc("detail.scheduling_uri"), values: Object.values(r.schedulingAddresses ?? {}).map((c) => c.uri).filter(Boolean) },
+    { label: tc("detail.related_contacts"), values: Object.keys(r.relatedTo ?? {}) },
+    { label: tc("detail.crypto_keys"), values: Object.keys(r.cryptoKeys ?? {}).length > 0 ? [tm("merge_kept")] : [] },
+    { label: tm("merge_photo"), values: Object.keys(r.media ?? {}).length > 0 ? [tm("merge_kept")] : [] },
+    { label: t("t_notes"), values: Object.values(r.notes ?? {}).map((n) => n.note).filter(Boolean), multiline: true },
     { label: t("t_categories"), values: Object.keys(r.keywords ?? {}).filter((k) => r.keywords?.[k]) },
     { label: t("groups"), values: groupNames },
   ].filter((row) => row.values.length > 0);
@@ -188,7 +196,7 @@ export function ContactMergeDialog({ cards: initialCards, pickFor, candidates =
               <dt className="text-muted-foreground">{row.label}</dt>
               <dd className="min-w-0">
                 {row.values.map((value, i) => (
-                  <div key={i} className={cn("break-words", row.label === t("t_notes") && "whitespace-pre-line")}>{value}</div>
+                  <div key={i} className={cn("break-words", row.multiline && "whitespace-pre-line")}>{value}</div>
                 ))}
               </dd>
             </div>