From 4ffa0160454b0db95ea061cd4a283ba3f4ae102f Mon Sep 17 00:00:00 2001 From: Elian Doran Date: Sat, 11 Apr 2026 14:28:58 +0300 Subject: [PATCH] fix(sidebar): highlights with math split in read-only text --- .../widgets/sidebar/HighlightsList.spec.ts | 38 ++++++-- .../src/widgets/sidebar/HighlightsList.tsx | 92 +++++++++---------- 2 files changed, 74 insertions(+), 56 deletions(-) diff --git a/apps/client/src/widgets/sidebar/HighlightsList.spec.ts b/apps/client/src/widgets/sidebar/HighlightsList.spec.ts index 8cfa4ee0c5..10ef0fae00 100644 --- a/apps/client/src/widgets/sidebar/HighlightsList.spec.ts +++ b/apps/client/src/widgets/sidebar/HighlightsList.spec.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import { extractHighlightsFromStaticHtml } from "./HighlightsList.js"; describe("extractHighlightsFromStaticHtml", () => { - it("extracts highlighted text with math equations", () => { + it("extracts a single highlight containing text and math equation together", () => { const container = document.createElement("div"); container.innerHTML = `

@@ -17,15 +17,35 @@ describe("extractHighlightsFromStaticHtml", () => { const highlights = extractHighlightsFromStaticHtml(container); - // Should extract 3 highlights: "Highlighted ", the math element, and " math" - expect(highlights.length).toBe(3); + // Should extract 1 combined highlight, not 3 separate ones + expect(highlights.length).toBe(1); - // The math highlight should preserve the .math-tex wrapper - const mathHighlight = highlights.find(h => h.text.includes("math-tex")); - expect(mathHighlight).toBeDefined(); - expect(mathHighlight?.text).toContain('class="math-tex"'); - expect(mathHighlight?.text).toContain("e=mc^2"); - expect(mathHighlight?.attrs.background).toBeTruthy(); + // The highlight should contain the full innerHTML of the styled span + const highlight = highlights[0]; + expect(highlight.text).toContain("Highlighted"); + expect(highlight.text).toContain("math-tex"); + expect(highlight.text).toContain("e=mc^2"); + expect(highlight.text).toContain("math"); + expect(highlight.attrs.background).toBeTruthy(); + + document.body.removeChild(container); + }); + + it("extracts separate highlights for differently styled spans", () => { + const container = document.createElement("div"); + container.innerHTML = `

+ Yellow text + normal text + Red text +

`; + document.body.appendChild(container); + + const highlights = extractHighlightsFromStaticHtml(container); + + // Should extract 2 separate highlights (yellow and red) + expect(highlights.length).toBe(2); + expect(highlights[0].text).toBe("Yellow text"); + expect(highlights[1].text).toBe("Red text"); document.body.removeChild(container); }); diff --git a/apps/client/src/widgets/sidebar/HighlightsList.tsx b/apps/client/src/widgets/sidebar/HighlightsList.tsx index 4720d0cd14..dc9a78c500 100644 --- a/apps/client/src/widgets/sidebar/HighlightsList.tsx +++ b/apps/client/src/widgets/sidebar/HighlightsList.tsx @@ -270,64 +270,62 @@ function ReadOnlyTextHighlightsList() { export function extractHighlightsFromStaticHtml(el: HTMLElement | null) { if (!el) return []; - const { color: defaultColor, backgroundColor: defaultBackgroundColor } = getComputedStyle(el); - - const walker = document.createTreeWalker( - el, - NodeFilter.SHOW_TEXT, - null - ); - const highlights: DomHighlight[] = []; - const processedMathElements = new Set(); + const processedElements = new Set(); - let node: Node | null; - while ((node = walker.nextNode())) { - const el = node.parentElement; - if (!el || !node.textContent?.trim()) continue; + // Find all elements with inline background-color or color styles + const styledElements = el.querySelectorAll('[style*="background-color"], [style*="color"]'); - const style = getComputedStyle(el); + for (const styledEl of styledElements) { + if (processedElements.has(styledEl)) continue; + if (!styledEl.textContent?.trim()) continue; - // For elements inside math-tex, get styles from the styled ancestor - const mathEl = el.closest(".math-tex"); + const attrs: RawHighlight["attrs"] = { + bold: !!styledEl.closest("strong"), + italic: !!styledEl.closest("em"), + underline: !!styledEl.closest("u"), + background: styledEl.style.backgroundColor, + color: styledEl.style.color + }; - // Skip if we've already processed this math element - if (mathEl && processedMathElements.has(mathEl)) continue; + if (Object.values(attrs).some(Boolean)) { + processedElements.add(styledEl); - const styledEl = mathEl?.parentElement ?? el; - const styledElStyle = styledEl !== el ? getComputedStyle(styledEl) : style; + highlights.push({ + id: randomString(), + text: styledEl.innerHTML, + element: styledEl, + attrs + }); + } + } - if ( - el.closest('strong, em, u') || - style.color !== defaultColor || - style.backgroundColor !== defaultBackgroundColor || - styledElStyle.color !== defaultColor || - styledElStyle.backgroundColor !== defaultBackgroundColor - ) { + // Also find bold, italic, underline elements + const formattingElements = el.querySelectorAll("strong, em, u, b, i"); - const attrs: RawHighlight["attrs"] = { - bold: !!el.closest("strong"), - italic: !!el.closest("em"), - underline: !!el.closest("u"), - background: styledEl.style.backgroundColor, - color: styledEl.style.color - }; + for (const formattedEl of formattingElements) { + // Skip if already processed or inside a processed element + if (processedElements.has(formattedEl)) continue; + if (Array.from(processedElements).some(processed => processed.contains(formattedEl))) continue; + if (!formattedEl.textContent?.trim()) continue; - if (Object.values(attrs).some(Boolean)) { - const text = mathEl ? mathEl.outerHTML : node.textContent; + const attrs: RawHighlight["attrs"] = { + bold: formattedEl.matches("strong, b"), + italic: formattedEl.matches("em, i"), + underline: formattedEl.matches("u"), + background: formattedEl.style.backgroundColor, + color: formattedEl.style.color + }; - // Track processed math elements to avoid duplicates - if (mathEl) { - processedMathElements.add(mathEl); - } + if (Object.values(attrs).some(Boolean)) { + processedElements.add(formattedEl); - highlights.push({ - id: randomString(), - text, - element: el, - attrs - }); - } + highlights.push({ + id: randomString(), + text: formattedEl.innerHTML, + element: formattedEl, + attrs + }); } }