POP3: the protocol as a two-segment toggle, and a delete mode that saves

The protocol choice wears the settings pages' two-segment control instead of
two stacked radios each starred as required. The port moves between 993 and
995 with it while it still holds a default, and the host hint follows.

The delete mode never saved: the field returns the enum case and the
listener passed it to fromSetting(), which reads anything but a string as
Keep. Found by the new browser spec, which saves a POP3 account with Delete,
reopens it, and switches it to IMAP.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

assets/controllers/settings/imap_preset_controller.js

@@ -14,11 +14,15 @@ export default class extends Controller {
     static targets = ["note", "incomingLabel", "pop3Options"];
 
     /**
-     * The port each incoming protocol starts from. The heading is not here:
-     * it is translated, so the template hands both wordings to the heading
-     * element as data-heading-imap / data-heading-pop3.
+     * Where each incoming protocol starts: its SSL port, and the shape of a
+     * host name to hint at. The heading is not here: it is translated, so the
+     * template hands both wordings to the heading element as
+     * data-heading-imap / data-heading-pop3.
      */
-    static PORTS = { imap: "993", pop3: "995" };
+    static PROTOCOLS = {
+        imap: { port: "993", host: "imap.example.com" },
+        pop3: { port: "995", host: "pop.example.com" },
+    };
 
     connect() {
         this.presets = this._readPresets();
@@ -29,7 +33,7 @@ export default class extends Controller {
      * The incoming-protocol radio was changed.
      *
      * Three things follow, and all three are the same fact stated three ways:
-     * the section heading names the protocol, the port placeholder follows,
+     * the section heading names the protocol, the port follows,
      * and the POP3 delete option appears only where it means something.
      * Nothing already typed is cleared — switching back and forth should not
      * cost the user their host name.
@@ -45,10 +49,27 @@ export default class extends Controller {
             }
         }
 
+        const defaults = this.constructor.PROTOCOLS;
+        const chosen = defaults[protocol] ?? defaults.imap;
         const port = this._field("imapPort");
 
         if (port !== null) {
-            port.placeholder = this.constructor.PORTS[protocol] ?? this.constructor.PORTS.imap;
+            port.placeholder = chosen.port;
+
+            // A blank form arrives with 993 filled in. Still holding the
+            // other protocol's default, the port follows the switch; a port
+            // somebody typed is theirs and stays.
+            const isADefault = Object.values(defaults).some((entry) => entry.port === port.value);
+
+            if (port.value === "" || isADefault) {
+                port.value = chosen.port;
+            }
+        }
+
+        const host = this._field("imapHost");
+
+        if (host !== null) {
+            host.placeholder = chosen.host;
         }
 
         if (this.hasPop3OptionsTarget) {

src/Form/AccountType.php

@@ -228,9 +228,13 @@ class AccountType extends AbstractType
 
             $chosen = $event->getForm()->get('pop3DeleteMode')->getData();
 
-            // Unrecognised reads as Keep, which is Pop3DeleteMode's own rule
-            // and not repeated here.
-            $account->pop3DeleteMode = Pop3DeleteMode::fromSetting($chosen);
+            // The field hands back the enum case, not its string: passed to
+            // fromSetting() as it was, every save read as Keep and Delete
+            // could never be chosen. Anything else is unrecognised, and reads
+            // as Keep by Pop3DeleteMode's own rule.
+            $account->pop3DeleteMode = $chosen instanceof Pop3DeleteMode
+                ? $chosen
+                : Pop3DeleteMode::fromSetting($chosen);
         });
 
         // What the account had before the form wrote on it: `always_empty`

templates/_layout/form/modal_form_theme.html.twig

@@ -318,6 +318,35 @@ navigable, and submitted like any other choice. The circle is its label.
     </div>
 {% endblock %}
 
+{#
+The account form's IMAP / POP3 choice (#289).
+
+The two-segment control every either/or on the settings pages wears (see
+settings/_compose_behavior.html.twig): the radios are hidden peers, and the
+selected segment is filled with the accent. The default expanded rendering
+stacked each radio above its own label and starred both as required, as if
+there were two questions. The container keeps the field's attributes: its
+data-action is what tells the imap-preset controller the protocol changed.
+#}
+{% block _account_incomingProtocol_widget %}
+    <div {{ block('widget_container_attributes') }}>
+        <div class="inline-grid grid-flow-col auto-cols-fr rounded-pane border border-line overflow-hidden"
+             role="radiogroup" aria-label="{{ label|trans }}">
+            {% for child in form %}
+                <label>
+                    {{ form_widget(child, { attr: { class: 'peer sr-only' } }) }}
+                    <span class="flex h-full items-center justify-center px-4 py-1.5 text-center text-xs leading-tight text-ink-muted
+                                 peer-checked:bg-accent peer-checked:text-accent-ink
+                                 peer-focus-visible:ring-2 peer-focus-visible:ring-inset peer-focus-visible:ring-accent
+                                 transition-colors cursor-pointer">
+                        {{ child.vars.label|trans }}
+                    </span>
+                </label>
+            {% endfor %}
+        </div>
+    </div>
+{% endblock %}
+
 {#
 The calendar colour picker.
 

tests/e2e/account-pop3.spec.ts

@@ -0,0 +1,134 @@
+import { test, expect, type Page } from "./support/test";
+import { acceptConfirm } from "./support/confirm";
+
+/**
+ * The account form's protocol choice (#289).
+ *
+ * Only the browser shows the form changing under the toggle: the section
+ * heading names the protocol, the port moves to where that protocol starts
+ * unless somebody typed one, the
+ * delete option appears for POP3 alone, and choosing a provider preset — all
+ * of which are IMAP servers — puts the radio back on IMAP. The second test
+ * saves a POP3 account and switches it to IMAP on the edit form, which is the
+ * path IncomingProtocolSwitchTest covers on the server side.
+ *
+ * Like account.spec.ts, saving only writes the row, so a fake host is fine,
+ * and the account is removed in a `finally` so no other spec counts it.
+ */
+
+const HOST = 'input[name="account[imapHost]"]';
+const PORT = 'input[name="account[imapPort]"]';
+const DELETE_MODE = 'select[name="account[pop3DeleteMode]"]';
+
+async function openAddForm(page: Page): Promise<void> {
+    await page.goto("/settings?section=accounts");
+
+    await page
+        .locator("section")
+        .filter({ has: page.locator("#settings-account-list") })
+        .getByRole("button", { name: "Add account" })
+        .click();
+
+    await expect(page.locator(HOST)).toBeVisible();
+}
+
+/** The segment of the protocol toggle; the radio behind it is visually hidden. */
+async function chooseProtocol(page: Page, name: "IMAP" | "POP3"): Promise<void> {
+    await page.locator("#modal").getByRole("radiogroup").getByText(name, { exact: true }).click();
+}
+
+/** The delete mode is a TomSelect-enhanced select: picked as a person picks it. */
+async function chooseDeleteMode(page: Page, option: string): Promise<void> {
+    await page.locator(`${DELETE_MODE} + .ts-wrapper`).click();
+    await page.locator(".ts-dropdown .option", { hasText: option }).click();
+}
+
+async function removeAccount(page: Page, label: string): Promise<void> {
+    await page.goto("/settings?section=accounts");
+
+    const row = page.locator("#settings-account-list li").filter({ hasText: new RegExp(label, "i") });
+
+    if (0 === (await row.count())) {
+        return;
+    }
+
+    await row.getByRole("button", { name: "Remove account" }).first().click();
+    await acceptConfirm(page);
+
+    await expect(row).toHaveCount(0);
+}
+
+test.describe("account form — incoming protocol", () => {
+    test("POP3 renames the section, moves the port and shows what happens to the server's copy", async ({ page }) => {
+        await openAddForm(page);
+
+        const modal = page.locator("#modal");
+
+        await expect(modal.getByText("Incoming — IMAP")).toBeVisible();
+        await expect(page.locator(PORT)).toHaveValue("993");
+        await expect(page.locator(`${DELETE_MODE} + .ts-wrapper`)).toBeHidden();
+
+        await chooseProtocol(page, "POP3");
+
+        await expect(modal.getByText("Incoming — POP3")).toBeVisible();
+        await expect(page.locator(PORT)).toHaveValue("995", { timeout: 2000 });
+        await expect(page.locator(HOST)).toHaveAttribute("placeholder", "pop.example.com");
+        await expect(page.locator(`${DELETE_MODE} + .ts-wrapper`)).toBeVisible();
+        await expect(page.locator(DELETE_MODE)).toHaveValue("keep");
+
+        // Every preset is an IMAP server; choosing one is choosing IMAP.
+        const preset = page.locator('select[name="account[preset]"]');
+        const first = await preset.locator("option").nth(1).getAttribute("value");
+        await preset.selectOption(first as string);
+
+        await expect(modal.getByLabel("IMAP", { exact: true })).toBeChecked();
+        await expect(modal.getByText("Incoming — IMAP")).toBeVisible();
+        await expect(page.locator(`${DELETE_MODE} + .ts-wrapper`)).toBeHidden();
+
+        // A port somebody typed is theirs: switching protocol leaves it.
+        await page.locator(PORT).fill("110");
+        await chooseProtocol(page, "POP3");
+        await expect(page.locator(PORT)).toHaveValue("110");
+    });
+
+    test("a POP3 account saves its delete mode and can be switched to IMAP", async ({ page }) => {
+        const stamp = Date.now();
+        const label = `E2E POP3 ${stamp}`;
+
+        try {
+            await openAddForm(page);
+
+            const modal = page.locator("#modal");
+
+            await page.locator('input[name="account[email]"]').fill(label);
+            await page.locator('input[name="account[username]"]').fill(`pop3-${stamp}@example.test`);
+            await page.locator('input[name="account[password]"]').fill("hunter2");
+            await chooseProtocol(page, "POP3");
+            await page.locator(HOST).fill(`pop-${stamp}.example.test`);
+            await page.locator(PORT).fill("995");
+            await page.locator('select[name="account[imapEncryption]"]').selectOption("ssl");
+            await chooseDeleteMode(page, "Delete it from the server");
+
+            await modal.locator('button[type="submit"]').click();
+            await expect(page.getByText("Account added successfully")).toBeVisible();
+
+            const row = page.locator("#settings-account-list li").filter({ hasText: new RegExp(label, "i") });
+            await row.locator('[data-ui--modal-src-value$="/edit"]').first().click();
+
+            await expect(modal.getByLabel("POP3", { exact: true })).toBeChecked();
+            await expect(modal.getByText("Incoming — POP3")).toBeVisible();
+            await expect(page.locator(DELETE_MODE)).toHaveValue("delete");
+
+            await chooseProtocol(page, "IMAP");
+            await expect(page.locator(PORT)).toHaveValue("993");
+            await modal.locator('button[type="submit"]').click();
+            await expect(page.locator("#modal " + HOST)).toHaveCount(0);
+
+            await row.locator('[data-ui--modal-src-value$="/edit"]').first().click();
+            await expect(modal.getByLabel("IMAP", { exact: true })).toBeChecked();
+            await expect(modal.getByText("Incoming — IMAP")).toBeVisible();
+        } finally {
+            await removeAccount(page, label);
+        }
+    });
+});