From 3b76aad2e7876f81d4e08cc1fbb91080c86954e3 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Thu, 5 Feb 2026 22:05:39 +0000
Subject: [PATCH] Address review feedback: update comment, move regex, add edge
 case tests

Co-authored-by: andrew-polk <5847219+andrew-polk@users.noreply.github.com>
---
 .../stringWithOptionalLink.spec.tsx           | 84 ++++++++++++++++++-
 .../stringWithOptionalLink.tsx                |  6 +-
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/src/BloomBrowserUI/react_components/stringWithOptionalLink.spec.tsx b/src/BloomBrowserUI/react_components/stringWithOptionalLink.spec.tsx
index f0c50fbc79f7..1b7e17afd14f 100644
--- a/src/BloomBrowserUI/react_components/stringWithOptionalLink.spec.tsx
+++ b/src/BloomBrowserUI/react_components/stringWithOptionalLink.spec.tsx
@@ -1,5 +1,4 @@
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
-import * as React from "react";
 import * as ReactDOM from "react-dom";
 import { renderToStaticMarkup } from "react-dom/server";
 import { act } from "react-dom/test-utils";
@@ -106,4 +105,87 @@ describe("StringWithOptionalLink", () => {
         );
         expect(postMock).not.toHaveBeenCalled();
     });
+
+    it("renders a single span for messages with no links", () => {
+        const markup = renderToStaticMarkup(
+            <StringWithOptionalLink message={"This is plain text"} />,
+        );
+        const temp = document.createElement("div");
+        temp.innerHTML = markup;
+
+        const spans = temp.querySelectorAll("span");
+        const anchors = temp.querySelectorAll("a");
+
+        expect(spans.length).toBe(1);
+        expect(spans[0].textContent).toBe("This is plain text");
+        expect(anchors.length).toBe(0);
+    });
+
+    it("handles messages starting with a link", () => {
+        const markup = renderToStaticMarkup(
+            <StringWithOptionalLink
+                message={"<a href='/bloom/api/test'>Link</a> then text"}
+            />,
+        );
+        const temp = document.createElement("div");
+        temp.innerHTML = markup;
+
+        const anchors = temp.querySelectorAll("a");
+        const spans = temp.querySelectorAll("span");
+
+        expect(anchors.length).toBe(1);
+        expect(anchors[0].textContent).toBe("Link");
+        expect(spans.length).toBe(1);
+        expect(spans[0].textContent).toBe(" then text");
+    });
+
+    it("handles messages ending with a link", () => {
+        const markup = renderToStaticMarkup(
+            <StringWithOptionalLink
+                message={"Text before <a href='/bloom/api/test'>Link</a>"}
+            />,
+        );
+        const temp = document.createElement("div");
+        temp.innerHTML = markup;
+
+        const spans = temp.querySelectorAll("span");
+        const anchors = temp.querySelectorAll("a");
+
+        expect(spans.length).toBe(1);
+        expect(spans[0].textContent).toBe("Text before ");
+        expect(anchors.length).toBe(1);
+        expect(anchors[0].textContent).toBe("Link");
+    });
+
+    it("handles consecutive links with no text between them", () => {
+        const markup = renderToStaticMarkup(
+            <StringWithOptionalLink
+                message={
+                    "<a href='/bloom/api/first'>First</a><a href='/bloom/api/second'>Second</a>"
+                }
+            />,
+        );
+        const temp = document.createElement("div");
+        temp.innerHTML = markup;
+
+        const anchors = temp.querySelectorAll("a");
+        const spans = temp.querySelectorAll("span");
+
+        expect(anchors.length).toBe(2);
+        expect(anchors[0].textContent).toBe("First");
+        expect(anchors[1].textContent).toBe("Second");
+        expect(spans.length).toBe(0);
+    });
+
+    it("renders a single span for empty string", () => {
+        const markup = renderToStaticMarkup(
+            <StringWithOptionalLink message={""} />,
+        );
+        const temp = document.createElement("div");
+        temp.innerHTML = markup;
+
+        const spans = temp.querySelectorAll("span");
+        expect(spans.length).toBe(1);
+        expect(spans[0].textContent).toBe("");
+    });
 });
diff --git a/src/BloomBrowserUI/react_components/stringWithOptionalLink.tsx b/src/BloomBrowserUI/react_components/stringWithOptionalLink.tsx
index a0bc8d7ed1a5..5affe6bc2f2f 100644
--- a/src/BloomBrowserUI/react_components/stringWithOptionalLink.tsx
+++ b/src/BloomBrowserUI/react_components/stringWithOptionalLink.tsx
@@ -6,11 +6,13 @@ import { post, postString } from "../utils/bloomApi";
 // (Currently the href must use single quotes.)
 // If there is no link, the result is a single span, the message.
 // If there are links, we interleave spans and anchor tags for each segment.
-// Currently the href is assumed to be something to send to our API,
-// but NOT to actually navigate to. We could support more options as needed.
+// For internal links, the href is treated as something to send to our API rather than
+// something for the browser to navigate to directly. External http/mailto hrefs are
+// sent to the 'link' API endpoint, which opens them in an external browser.
 export const StringWithOptionalLink: React.FunctionComponent<{
     message: string;
 }> = (props) => {
+    // Create a fresh regex for each render to avoid lastIndex issues
     const linkRegex = /<a[^>]*?href='([^>']+)'[^>]*>(.*?)<\/a>/g;
     const elements: React.ReactNode[] = [];
     let lastIndex = 0;