From c6967979899f45d4bf797d542fe6212967bba6eb Mon Sep 17 00:00:00 2001 From: neerajadhav Date: Mon, 7 Jul 2025 19:02:49 +0530 Subject: [PATCH] feat: enhance popover and SPA navigation with smart scrolling and highlighting - Add buffer-aware scrolling to hash anchors in both SPA and popover contexts - Implement temporary highlight feedback for targeted headings with auto-fade - Refactor scroll and highlight logic into reusable utility functions - Improve popover heading matching with stricter fallback logic to avoid false positives - Ensure highlights clear properly on popover enter/leave events - Fix scroll behavior for cached popovers to always navigate to correct heading - Add robust error handling and TypeScript compatibility for isolated file checks Breaking changes: None Fixes: Scroll positioning issues, highlight persistence bugs, imprecise heading matching --- quartz/components/scripts/popover.inline.ts | 78 +++++++++++++---- quartz/components/scripts/util.ts | 92 ++++++++++++++++++--- 2 files changed, 140 insertions(+), 30 deletions(-) diff --git a/quartz/components/scripts/popover.inline.ts b/quartz/components/scripts/popover.inline.ts index 3348e0249..f41faa0f8 100644 --- a/quartz/components/scripts/popover.inline.ts +++ b/quartz/components/scripts/popover.inline.ts @@ -1,5 +1,5 @@ +import { clearAllHighlights, fetchCanonical, scrollInContainerToElement } from "./util" import { computePosition, flip, inline, shift } from "@floating-ui/dom" -import { fetchCanonical, scrollInContainerToElement } from "./util" import { normalizeRelativeURLs } from "../../util/path" @@ -15,6 +15,9 @@ async function mouseEnterHandler( return } + // Clear any existing highlights immediately when entering a new link + clearAllHighlights() + async function setPosition(popoverElement: HTMLElement) { const { x, y } = await computePosition(link, popoverElement, { strategy: "fixed", @@ -25,17 +28,47 @@ async function mouseEnterHandler( }) } - function showPopover(popoverElement: HTMLElement) { + function showPopover(popoverElement: HTMLElement, targetHash: string = "") { clearActivePopover() popoverElement.classList.add("active-popover") setPosition(popoverElement as HTMLElement) - if (hash !== "") { - const targetAnchor = `#popover-internal-${hash.slice(1)}` - const heading = popoverInner.querySelector(targetAnchor) as HTMLElement | null - if (heading) { - // Use utility function to scroll with buffer and highlight - scrollInContainerToElement(popoverInner, heading, 20, true, "instant") + // Always scroll to hash anchor if present, regardless of popover state + if (targetHash !== "") { + const popoverInner = popoverElement.querySelector(".popover-inner") as HTMLElement | null + if (popoverInner) { + const hashWithoutPound = targetHash.slice(1) + const targetAnchor = `popover-internal-${hashWithoutPound}` + + // Try to find the element by ID first + let heading = popoverInner.querySelector(`#${targetAnchor}`) as HTMLElement | null + + // If not found by ID, try to find by text content (fallback for headings) + if (!heading) { + const headings = popoverInner.querySelectorAll('h1, h2, h3, h4, h5, h6') + for (const h of headings) { + const headingElement = h as HTMLElement + const headingText = headingElement.textContent?.trim().toLowerCase() || '' + const targetText = hashWithoutPound.toLowerCase().replace(/-/g, ' ') + + // More strict matching: avoid matching very short headings unless they're exact matches + const isExactMatch = headingText === targetText + const isSubstringMatch = headingText.length >= 3 && ( + headingText.includes(targetText) || + (targetText.length >= 3 && targetText.includes(headingText)) + ) + + if (isExactMatch || isSubstringMatch) { + heading = headingElement + break + } + } + } + + if (heading) { + // Use utility function to scroll with buffer and highlight + scrollInContainerToElement(popoverInner, heading, 20, true, "instant") + } } } } @@ -49,7 +82,7 @@ async function mouseEnterHandler( // dont refetch if there's already a popover if (!!document.getElementById(popoverId)) { - showPopover(prevPopoverElement as HTMLElement) + showPopover(prevPopoverElement as HTMLElement, hash) return } @@ -97,7 +130,7 @@ async function mouseEnterHandler( const targetID = `popover-internal-${el.id}` el.id = targetID }) - const elts = [...html.getElementsByClassName("popover-hint")] + const elts = Array.from(html.getElementsByClassName("popover-hint")) if (elts.length === 0) return elts.forEach((elt) => popoverInner.appendChild(elt)) @@ -112,23 +145,34 @@ async function mouseEnterHandler( return } - showPopover(popoverElement) + showPopover(popoverElement, hash) } function clearActivePopover() { activeAnchor = null const allPopoverElements = document.querySelectorAll(".popover") allPopoverElements.forEach((popoverElement) => popoverElement.classList.remove("active-popover")) + // Clear any remaining highlights when closing popovers + clearAllHighlights() +} + +function clearActivePopoverAndHighlights() { + clearActivePopover() + // Also clear highlights immediately when mouse leaves + clearAllHighlights() } document.addEventListener("nav", () => { - const links = [...document.querySelectorAll("a.internal")] as HTMLAnchorElement[] + const links = Array.from(document.querySelectorAll("a.internal")) as HTMLAnchorElement[] for (const link of links) { link.addEventListener("mouseenter", mouseEnterHandler) - link.addEventListener("mouseleave", clearActivePopover) - window.addCleanup(() => { - link.removeEventListener("mouseenter", mouseEnterHandler) - link.removeEventListener("mouseleave", clearActivePopover) - }) + link.addEventListener("mouseleave", clearActivePopoverAndHighlights) + // Use type assertion to avoid TypeScript error when checking individual files + if (typeof (window as any).addCleanup === 'function') { + (window as any).addCleanup(() => { + link.removeEventListener("mouseenter", mouseEnterHandler) + link.removeEventListener("mouseleave", clearActivePopoverAndHighlights) + }) + } } }) diff --git a/quartz/components/scripts/util.ts b/quartz/components/scripts/util.ts index b76469020..d9aba422b 100644 --- a/quartz/components/scripts/util.ts +++ b/quartz/components/scripts/util.ts @@ -14,9 +14,14 @@ export function registerEscapeHandler(outsideContainer: HTMLElement | null, cb: } outsideContainer?.addEventListener("click", click) - window.addCleanup(() => outsideContainer?.removeEventListener("click", click)) + // Use type assertion to avoid TypeScript error when checking individual files + if (typeof (window as any).addCleanup === 'function') { + (window as any).addCleanup(() => outsideContainer?.removeEventListener("click", click)) + } document.addEventListener("keydown", esc) - window.addCleanup(() => document.removeEventListener("keydown", esc)) + if (typeof (window as any).addCleanup === 'function') { + (window as any).addCleanup(() => document.removeEventListener("keydown", esc)) + } } export function removeAllChildren(node: HTMLElement) { @@ -45,6 +50,39 @@ export async function fetchCanonical(url: URL): Promise { return redirect ? fetch(`${new URL(redirect, url)}`) : res } +// Keep track of active highlights to clean them up +let activeHighlights: Set<{ + element: HTMLElement, + originalBackground: string, + originalTransition: string, + timeoutId: number +}> = new Set() + +/** + * Clears all active highlights immediately + */ +export function clearAllHighlights() { + // Clear tracked highlights + activeHighlights.forEach(({ element, originalBackground, originalTransition, timeoutId }) => { + clearTimeout(timeoutId) + element.style.backgroundColor = originalBackground + element.style.transition = originalTransition + }) + activeHighlights.clear() + + // Also clear any highlights that might not be tracked (backup cleanup) + // This catches highlights in popovers or other edge cases + const allHighlightedElements = document.querySelectorAll('[style*="background-color"]') + allHighlightedElements.forEach((el) => { + const element = el as HTMLElement + if (element.style.backgroundColor.includes('var(--highlight') || + element.style.backgroundColor.includes('#ffeb3b')) { + element.style.backgroundColor = '' + element.style.transition = '' + } + }) +} + /** * Highlights an element with a temporary background color effect * @param el - The element to highlight @@ -52,6 +90,14 @@ export async function fetchCanonical(url: URL): Promise { * @param color - The highlight color (default: uses CSS variable --highlight) */ export function highlightElement(el: HTMLElement, duration: number = 2000, color: string = 'var(--highlight, #ffeb3b40)') { + // Clear any existing highlight on this element + activeHighlights.forEach((highlight) => { + if (highlight.element === el) { + clearTimeout(highlight.timeoutId) + activeHighlights.delete(highlight) + } + }) + // Store original styles const originalBackground = el.style.backgroundColor const originalTransition = el.style.transition @@ -60,14 +106,28 @@ export function highlightElement(el: HTMLElement, duration: number = 2000, color el.style.transition = 'background-color 0.3s ease' el.style.backgroundColor = color - // Remove highlight after specified duration - setTimeout(() => { + // Set up cleanup + const timeoutId = window.setTimeout(() => { el.style.backgroundColor = originalBackground // Remove transition after background fades back setTimeout(() => { el.style.transition = originalTransition + // Remove from active highlights + activeHighlights.forEach((highlight) => { + if (highlight.element === el) { + activeHighlights.delete(highlight) + } + }) }, 300) }, duration) + + // Track this highlight + activeHighlights.add({ + element: el, + originalBackground, + originalTransition, + timeoutId + }) } /** @@ -113,14 +173,20 @@ export function scrollInContainerToElement( highlight: boolean = true, behavior: ScrollBehavior = 'instant' ) { - const targetPosition = target.offsetTop - buffer - container.scroll({ - top: Math.max(0, targetPosition), - behavior + // Use requestAnimationFrame to ensure content is rendered before scrolling + requestAnimationFrame(() => { + const targetPosition = target.offsetTop - buffer + container.scroll({ + top: Math.max(0, targetPosition), + behavior + }) + + // Add highlight effect if requested + if (highlight) { + // Small delay to ensure scroll completes before highlighting + setTimeout(() => { + highlightElement(target) + }, 50) + } }) - - // Add highlight effect if requested - if (highlight) { - highlightElement(target) - } }