diff --git a/packages/app/e2e/browser/assistant-selection-copy.spec.ts b/packages/app/e2e/browser/assistant-selection-copy.spec.ts index 3c9440fdb..d80401deb 100644 --- a/packages/app/e2e/browser/assistant-selection-copy.spec.ts +++ b/packages/app/e2e/browser/assistant-selection-copy.spec.ts @@ -118,6 +118,53 @@ async function selectAssistantText(page: Page, text: string): Promise { await selectAssistantTextRange(page, text, text); } +async function selectAssistantListItemFromMarker( + page: Page, + listTag: "ol" | "ul", + itemText: string, + endText: string, +): Promise { + const assistantMessage = page.getByTestId("assistant-message").filter({ + hasText: "Direct matches:", + }); + const item = assistantMessage + .locator(`[data-paseo-markdown-tag="${listTag}"]`) + .filter({ hasText: itemText }) + .locator(':scope > [data-paseo-markdown-tag="li"]') + .filter({ hasText: itemText }); + + await item.evaluate((element, selectedEndText) => { + const marker = element.querySelector(':scope > [data-paseo-markdown-list-marker="true"]'); + const markerText = marker?.firstChild; + if (!(markerText instanceof Text)) { + throw new Error("Expected rendered list marker text"); + } + + const walker = document.createTreeWalker(element, NodeFilter.SHOW_TEXT); + let endNode: Text | null = null; + let endOffset = -1; + while (walker.nextNode()) { + const node = walker.currentNode; + const offset = node.textContent?.indexOf(selectedEndText) ?? -1; + if (node instanceof Text && offset >= 0) { + endNode = node; + endOffset = offset + selectedEndText.length; + break; + } + } + if (!endNode) { + throw new Error(`Could not find list item selection end: ${selectedEndText}`); + } + + const range = document.createRange(); + range.setStart(markerText, 0); + range.setEnd(endNode, endOffset); + const selection = window.getSelection(); + selection?.removeAllRanges(); + selection?.addRange(range); + }, endText); +} + async function doubleClickAssistantMarkdownText( page: Page, tag: "code" | "strong", @@ -405,6 +452,24 @@ test("copying an assistant selection preserves Markdown structure and links", as '
- First issue', ); + await selectAssistantListItemFromMarker(page, "ul", "First issue", "failure."); + await copySelection(page); + + const draggedBulletClipboard = await readRichClipboard(page); + expect(draggedBulletClipboard.plainText).toBe( + "- **[First issue](https://example.com/issues/1)**: exact `apply_patch` failure.", + ); + expect(draggedBulletClipboard.html).toContain( + '
- First issue', + ); + + await selectAssistantListItemFromMarker(page, "ol", "Sixth item", "Sixth item"); + await copySelection(page); + + const draggedNumberClipboard = await readRichClipboard(page); + expect(draggedNumberClipboard.plainText).toBe("6. Sixth item"); + expect(draggedNumberClipboard.html).toContain("
6. Sixth item
"); + await selectAssistantText(page, "docs"); await copySelection(page); diff --git a/packages/app/src/assistant-selection-copy/content.browser.test.ts b/packages/app/src/assistant-selection-copy/content.browser.test.ts index df3269017..c53cba9f9 100644 --- a/packages/app/src/assistant-selection-copy/content.browser.test.ts +++ b/packages/app/src/assistant-selection-copy/content.browser.test.ts @@ -5,8 +5,8 @@ const fixture = `
Prefix bold text and inline code suffix.
-
•
First bullet text
-
•
Second bullet text
+
•
First bullet text
+
•
Second bullet text
const answer = true;
@@ -203,6 +203,97 @@ describe("assistant selection copy ranges", () => { expect(copiedMarkdown(selectNodeContents(item))).toBe("- First bullet text"); }); + it("retains a bullet when a drag selects from the rendered marker through the item text", () => { + const message = mountFixture(); + const marker = fixtureElement(message, '[data-paseo-markdown-list-marker="true"]'); + const itemText = fixtureElement(message, '[data-paseo-markdown-tag="li"] div span'); + + const content = createAssistantSelectionClipboardContent( + selectRange(marker, 0, itemText, textNode(itemText).length), + ); + expect(content?.plainText).toBe("- First bullet text"); + expect(content?.html).toContain("
- First bullet text
"); + expect(content?.html).not.toContain("
    "); + expect(content?.html).not.toContain("
  • "); + }); + + it("retains a bullet when a drag includes the marker and part of the item text", () => { + const message = mountFixture(); + const marker = fixtureElement(message, '[data-paseo-markdown-list-marker="true"]'); + const itemText = fixtureElement(message, '[data-paseo-markdown-tag="li"] div span'); + + expect(copiedMarkdown(selectRange(marker, 0, itemText, 5))).toBe("- First"); + }); + + it("does not invent a bullet when a drag starts after the rendered marker", () => { + const message = mountFixture(); + const marker = fixtureElement(message, '[data-paseo-markdown-list-marker="true"]'); + const itemText = fixtureElement(message, '[data-paseo-markdown-tag="li"] div span'); + + expect( + copiedMarkdown( + selectRange(marker, textNode(marker).length, itemText, textNode(itemText).length), + ), + ).toBe("First bullet text"); + }); + + it("retains every selected marker across a partial multi-item drag", () => { + const message = mountFixture(); + const markerSelector = '[data-paseo-markdown-list-marker="true"]'; + const textSelector = '[data-paseo-markdown-tag="li"] div span'; + const firstMarker = fixtureElement(message, markerSelector); + const secondText = fixtureElement(message, textSelector, 1); + + expect(copiedMarkdown(selectRange(firstMarker, 0, secondText, 6))).toBe( + "- First bullet text\n- Second", + ); + }); + + it("retains the original number when a marker drag starts mid-list", () => { + const message = mountFixture(); + const list = fixtureElement(message, '[data-paseo-markdown-tag="ul"]'); + list.setAttribute("data-paseo-markdown-tag", "ol"); + list.setAttribute("data-paseo-markdown-list-start", "5"); + const markers = list.querySelectorAll('[data-paseo-markdown-list-marker="true"]'); + markers.item(0).textContent = "5."; + markers.item(1).textContent = "6."; + const secondMarker = fixtureElement(list, '[data-paseo-markdown-list-marker="true"]', 1); + const secondText = fixtureElement(list, '[data-paseo-markdown-tag="li"] div span', 1); + + expect( + copiedMarkdown(selectRange(secondMarker, 0, secondText, textNode(secondText).length)), + ).toBe("6. Second bullet text"); + }); + + it("retains nested markers when a drag includes the complete outer item", () => { + const message = mountFixture(); + const firstItem = fixtureElement(message, '[data-paseo-markdown-tag="li"]'); + firstItem.innerHTML = [ + '•', + "
    ", + "Outer text", + '
    ', + '
    ', + '•', + "
    Inner text
    ", + "
    ", + "
    ", + "
    ", + ].join(""); + const outerMarker = fixtureElement( + firstItem, + ':scope > [data-paseo-markdown-list-marker="true"]', + ); + const innerText = fixtureElement( + firstItem, + ':scope [data-paseo-markdown-tag="ul"] > [data-paseo-markdown-tag="li"] > div > span', + ); + + expect(copiedMarkdown(selectRange(outerMarker, 0, innerText, textNode(innerText).length))).toBe( + "- Outer text\n - Inner text", + ); + }); + it("preserves paragraph breaks when rich HTML flattens a loose list item", () => { const message = mountFixture(); const item = fixtureElement(message, '[data-paseo-markdown-tag="li"]'); diff --git a/packages/app/src/assistant-selection-copy/content.web.ts b/packages/app/src/assistant-selection-copy/content.web.ts index 6678e4a5b..f04ddfb3e 100644 --- a/packages/app/src/assistant-selection-copy/content.web.ts +++ b/packages/app/src/assistant-selection-copy/content.web.ts @@ -9,6 +9,7 @@ import { MARKDOWN_COPY_ALIGN_ATTRIBUTE, MARKDOWN_COPY_IGNORE_ATTRIBUTE, MARKDOWN_COPY_LANGUAGE_ATTRIBUTE, + MARKDOWN_COPY_LIST_MARKER_ATTRIBUTE, MARKDOWN_COPY_LIST_START_ATTRIBUTE, MARKDOWN_COPY_TAG_ATTRIBUTE, MARKDOWN_COPY_UNWRAP_ATTRIBUTE, @@ -278,7 +279,7 @@ function shouldPreserveSemanticElement(range: Range, element: Element): boolean if (tag === "p" || isTableStructure(tag)) { return true; } - const isSelectableSemantic = tag !== null && tag !== "ol" && tag !== "ul"; + const isSelectableSemantic = tag !== null && tag !== "li" && tag !== "ol" && tag !== "ul"; if ( (isSelectableSemantic || element.tagName === "A") && selectionStaysInsideElement(range, element) @@ -287,7 +288,12 @@ function shouldPreserveSemanticElement(range: Range, element: Element): boolean } if (tag === "ol" || tag === "ul") { const selectedItems = selectedListItems(element, range); - return selectedItems.some((item) => hasSelectedAllContents(range, item, true)); + return selectedItems.some( + (item) => hasSelectedListMarker(range, item) || hasSelectedAllContents(range, item, true), + ); + } + if (tag === "li" && hasSelectedListMarker(range, element)) { + return true; } return hasSelectedAllContents(range, element, tag === "li"); } @@ -311,6 +317,31 @@ function selectedListItems(list: Element, range: Range): Element[] { ); } +function hasSelectedListMarker(range: Range, item: Element): boolean { + const marker = item.querySelector(`:scope > [${MARKDOWN_COPY_LIST_MARKER_ATTRIBUTE}]`); + if (!marker) { + return false; + } + + const markerContents = document.createRange(); + markerContents.selectNodeContents(marker); + const selectionEndsBeforeMarker = + range.compareBoundaryPoints(Range.END_TO_START, markerContents) >= 0; + const selectionStartsAfterMarker = + range.compareBoundaryPoints(Range.START_TO_END, markerContents) <= 0; + if (selectionEndsBeforeMarker || selectionStartsAfterMarker) { + return false; + } + + if (range.compareBoundaryPoints(Range.START_TO_START, markerContents) > 0) { + markerContents.setStart(range.startContainer, range.startOffset); + } + if (range.compareBoundaryPoints(Range.END_TO_END, markerContents) < 0) { + markerContents.setEnd(range.endContainer, range.endOffset); + } + return hasMarkdownContent(markerContents.cloneContents(), true); +} + function normalizeOrderedListStart(copy: Element, original: Element, range: Range): void { if (original.getAttribute(MARKDOWN_COPY_TAG_ATTRIBUTE) !== "ol") { return; diff --git a/packages/app/src/assistant-selection-copy/markup.ts b/packages/app/src/assistant-selection-copy/markup.ts index 59a372a53..8f220043c 100644 --- a/packages/app/src/assistant-selection-copy/markup.ts +++ b/packages/app/src/assistant-selection-copy/markup.ts @@ -1,5 +1,6 @@ export const MARKDOWN_COPY_TAG_ATTRIBUTE = "data-paseo-markdown-tag"; export const MARKDOWN_COPY_IGNORE_ATTRIBUTE = "data-paseo-markdown-ignore"; +export const MARKDOWN_COPY_LIST_MARKER_ATTRIBUTE = "data-paseo-markdown-list-marker"; export const MARKDOWN_COPY_UNWRAP_ATTRIBUTE = "data-paseo-markdown-unwrap"; export const MARKDOWN_COPY_LIST_START_ATTRIBUTE = "data-paseo-markdown-list-start"; export const MARKDOWN_COPY_LANGUAGE_ATTRIBUTE = "data-paseo-markdown-language"; @@ -28,6 +29,7 @@ export const markdownCopyDataSet = { hr: { paseoMarkdownTag: "hr" }, ignore: { paseoMarkdownIgnore: "true" }, li: { paseoMarkdownTag: "li" }, + listMarker: { paseoMarkdownIgnore: "true", paseoMarkdownListMarker: "true" }, ol: { paseoMarkdownTag: "ol" }, p: { paseoMarkdownTag: "p" }, pre: { paseoMarkdownTag: "pre" }, diff --git a/packages/app/src/components/message.tsx b/packages/app/src/components/message.tsx index 2cc7cb402..e2e54fa77 100644 --- a/packages/app/src/components/message.tsx +++ b/packages/app/src/components/message.tsx @@ -1806,7 +1806,7 @@ export const AssistantMessage = memo(function AssistantMessage({ return ( - + {marker}