From bbcfb223c4dd044ee4cc3f679627e848c4e2b8b3 Mon Sep 17 00:00:00 2001 From: skimberk Date: Wed, 8 Oct 2025 13:21:22 -0700 Subject: [PATCH] add news permalinks and scrolling --- frontend/src/App.tsx | 1 + .../markdown-renderer/MarkdownRenderer.tsx | 56 ++++++++++++- frontend/src/pages/news/News.test.tsx | 18 ++-- frontend/src/pages/news/News.tsx | 40 ++++++++- frontend/src/pages/news/NewsRoute.test.tsx | 82 +++++++++++++++++++ frontend/src/pages/news/NewsSidebar.test.tsx | 40 +++++++++ .../news/components/NewsContentSection.tsx | 3 + .../src/pages/news/components/NewsSideBar.tsx | 22 ++++- 8 files changed, 245 insertions(+), 17 deletions(-) create mode 100644 frontend/src/pages/news/NewsRoute.test.tsx create mode 100644 frontend/src/pages/news/NewsSidebar.test.tsx diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index ada2645..80df9e5 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -51,6 +51,7 @@ function App() { } /> } /> } /> + } /> } /> // error handling page {errorRoutes.map(({ path, code, title, description }) => ( diff --git a/frontend/src/components/markdown-renderer/MarkdownRenderer.tsx b/frontend/src/components/markdown-renderer/MarkdownRenderer.tsx index 69d08d3..acc9ed3 100644 --- a/frontend/src/components/markdown-renderer/MarkdownRenderer.tsx +++ b/frontend/src/components/markdown-renderer/MarkdownRenderer.tsx @@ -1,10 +1,12 @@ -import React from "react"; +import React, { useEffect, useRef } from "react"; import ReactMarkdown from "react-markdown"; import rehypeRaw from "rehype-raw"; type MarkdownRendererProps = { content: string; imageProps?: MarkdownRendererImageProps; + // called when the rendered content is fully laid out (images loaded) + onLoadComplete?: () => void; }; type MarkdownRendererImageProps = { @@ -40,11 +42,58 @@ const defaultImageProps: MarkdownRendererImageProps = { const MarkdownRenderer: React.FC = ({ content, imageProps, + onLoadComplete, }) => { const mergedImageProps = { ...defaultImageProps, ...imageProps }; const { align, ...styleProps } = mergedImageProps; + const containerRef = useRef(null); + + // Detect when images inside the rendered markdown finish loading. + useEffect(() => { + const el = containerRef.current; + if (!el) { + onLoadComplete?.(); + return; + } + + const imgs = Array.from(el.querySelectorAll("img")) as HTMLImageElement[]; + if (imgs.length === 0) { + // no images -> content is effectively ready + // schedule on next tick to ensure paint + const t = window.setTimeout(() => onLoadComplete?.(), 0); + return () => clearTimeout(t); + } + + let settled = 0; + const handlers: Array<() => void> = []; + imgs.forEach((img) => { + if (img.complete) { + settled += 1; + return; + } + const onFinish = () => { + settled += 1; + if (settled === imgs.length) onLoadComplete?.(); + }; + img.addEventListener("load", onFinish); + img.addEventListener("error", onFinish); + handlers.push(() => { + img.removeEventListener("load", onFinish); + img.removeEventListener("error", onFinish); + }); + }); + + if (settled === imgs.length) { + // all images already complete + const t = window.setTimeout(() => onLoadComplete?.(), 0); + return () => clearTimeout(t); + } + + return () => handlers.forEach((h) => h()); + }, [content, onLoadComplete]); return ( - + ( @@ -70,7 +119,8 @@ const MarkdownRenderer: React.FC = ({ }} > {content} - + + ); }; diff --git a/frontend/src/pages/news/News.test.tsx b/frontend/src/pages/news/News.test.tsx index b69ce35..baf87f7 100644 --- a/frontend/src/pages/news/News.test.tsx +++ b/frontend/src/pages/news/News.test.tsx @@ -1,10 +1,10 @@ import { - render, screen, fireEvent, within, waitFor, } from "@testing-library/react"; +import { renderWithProviders } from "../../tests/test-utils"; import { vi, describe, it, expect, beforeEach } from "vitest"; import News from "./News"; // 假设你当前文件路径为 pages/News.tsx import * as apiHook from "../../lib/hooks/useApi"; @@ -57,8 +57,8 @@ describe("News", () => { mockHookReturn, ); - // render - render(); + // render inside MemoryRouter + ThemeProvider + renderWithProviders(); // asserts expect(screen.getByText(/Summoning/i)).toBeInTheDocument(); @@ -78,8 +78,8 @@ describe("News", () => { mockHookReturn, ); - // render - render(); + // render inside MemoryRouter + ThemeProvider + renderWithProviders(); // asserts expect(screen.getByText(/something went wrong/i)).toBeInTheDocument(); @@ -99,8 +99,8 @@ describe("News", () => { mockHookReturn, ); - // render - render(); + // render inside MemoryRouter + ThemeProvider + renderWithProviders(); // asserts expect(screen.getByText("News and Announcements")).toBeInTheDocument(); @@ -136,8 +136,8 @@ describe("News", () => { mockHookReturn, ); - // render - render(); + // render inside MemoryRouter + ThemeProvider + renderWithProviders(); // asserts const sidebar = screen.getByTestId("news-sidbar"); diff --git a/frontend/src/pages/news/News.tsx b/frontend/src/pages/news/News.tsx index 8513a7d..952ccc6 100644 --- a/frontend/src/pages/news/News.tsx +++ b/frontend/src/pages/news/News.tsx @@ -2,6 +2,7 @@ import { ErrorAlert } from "../../components/alert/ErrorAlert"; import { fetcherApiCallback } from "../../lib/hooks/useApi"; import { fetchAllNews } from "../../api/api"; import { useEffect, useRef } from "react"; +import { useParams, useLocation } from "react-router-dom"; import { Box } from "@mui/material"; import { NewsContentSection } from "./components/NewsContentSection"; import { Sidebar } from "./components/NewsSideBar"; @@ -18,22 +19,55 @@ export default function News() { const sectionRefs = useRef>({}); - const scrollTo = (id: string) => { + // read optional :postId param from URL; if present we'll scroll to it once data loads + const { postId } = useParams<{ postId?: string }>(); + + // scrollTo accepts an optional `smooth` flag. Sidebar clicks use smooth scrolling + // but when the page initially loads (deep link) we want an immediate jump. + const scrollTo = (id: string, smooth = true) => { const el = sectionRefs.current[id]; - if (el) el.scrollIntoView({ behavior: "smooth", block: "start" }); + if (el) el.scrollIntoView({ behavior: smooth ? "smooth" : "auto", block: "start" }); }; useEffect(() => { call(); }, []); + // when data loads and there's a postId, jump immediately (no animation). + // We'll also re-jump when the rendered section signals it's finished loading + // via the onSectionLoad callback (see below). + const location = useLocation(); + + useEffect(() => { + if (!postId) return; + // If navigation came from the sidebar we already initiated a smooth scroll + // there, so skip the immediate jump. We check location.state.fromSidebar + // to determine that. + if (location.state && location.state.fromSidebar) return; + + // initial immediate jump once refs attach + const t = window.setTimeout(() => { + if (sectionRefs.current[postId]) scrollTo(postId, false); + }, 0); + return () => clearTimeout(t); + }, [postId, data, location]); + if (loading) return ; if (error) return ; return ( - + { + // re-jump after content (images/markdown) finished loading. + // We re-jump whenever any section finishes to handle layout changes + // caused by images loading above/below the target post. + if (postId) scrollTo(postId, false); + }} + /> ); } diff --git a/frontend/src/pages/news/NewsRoute.test.tsx b/frontend/src/pages/news/NewsRoute.test.tsx new file mode 100644 index 0000000..4fc0f25 --- /dev/null +++ b/frontend/src/pages/news/NewsRoute.test.tsx @@ -0,0 +1,82 @@ +import { screen, within, waitFor } from "@testing-library/react"; +import { vi, describe, it, beforeEach, expect } from "vitest"; +import News from "./News"; +import { renderWithProviders } from "../../tests/test-utils"; + +// Mock the useApi hook used by News +vi.mock("../../lib/hooks/useApi", () => ({ + fetcherApiCallback: vi.fn(), +})); + +// Mock MarkdownRenderer to avoid lazy loading in tests +vi.mock("../../components/markdown-renderer/MarkdownRenderer", () => ({ + default: ({ content }: { content: string }) =>
{content}
, +})); + +import * as apiHook from "../../lib/hooks/useApi"; + +// helper to generate very large markdown content (many newlines) +const makeLargeMarkdown = (paragraphs = 200) => + Array.from({ length: paragraphs }) + .map((_, i) => `Paragraph ${i + 1}\n\nThis is a long line to increase vertical space.`) + .join("\n\n"); + +const mockData = Array.from({ length: 5 }).map((_, i) => ({ + id: `news-${i + 1}`, + title: `Displayed Post ${i + 1}`, + date: `2025-10-01`, + category: `Other`, + markdown: makeLargeMarkdown(200), +})); + +describe("News route deep link", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("renders the requested post when route is /news/:postId", async () => { + const mockHookReturn = { + data: mockData, + loading: false, + error: null, + errorStatus: null, + call: vi.fn(), + }; + + (apiHook.fetcherApiCallback as any).mockReturnValue(mockHookReturn); + + // spy on scrollIntoView so we can assert the deep-link triggers a scroll + const original = Element.prototype.scrollIntoView; + const scrollCalls: Array<{ el: Element; opts: any }> = []; + Element.prototype.scrollIntoView = function (opts?: any) { + scrollCalls.push({ el: this as Element, opts }); + }; + + try { + // render at /v2/news/news-3 (app is mounted with basename /v2) + renderWithProviders( + // Render the News component directly; renderWithProviders wraps MemoryRouter + , + "/v2/news/news-3", + ); + + // title should be visible + expect(screen.getByText("News and Announcements")).toBeInTheDocument(); + + const newsContent = screen.getByTestId("news-content"); + await waitFor(() => { + // The post title is present in the content area + expect(within(newsContent).getByText("Displayed Post 3")).toBeInTheDocument(); + }); + + // Assert scrollIntoView was called for the section with id 'news-3' + const scrolled = scrollCalls.find((c) => c.el && (c.el as Element).id === "news-3"); + expect(scrolled).toBeTruthy(); + // Optionally check options (initial jump used 'auto') + if (scrolled) expect(scrolled.opts).toMatchObject({ behavior: "auto", block: "start" }); + } finally { + // restore + Element.prototype.scrollIntoView = original; + } + }); +}); diff --git a/frontend/src/pages/news/NewsSidebar.test.tsx b/frontend/src/pages/news/NewsSidebar.test.tsx new file mode 100644 index 0000000..d23f754 --- /dev/null +++ b/frontend/src/pages/news/NewsSidebar.test.tsx @@ -0,0 +1,40 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { vi, describe, it, expect, afterEach } from "vitest"; + +// the Sidebar is a named export +import { Sidebar } from "./components/NewsSideBar"; + +// mock useNavigate from react-router-dom +const mockNavigate = vi.fn(); +vi.mock("react-router-dom", async () => { + const actual = await vi.importActual("react-router-dom"); + return { + ...actual, + useNavigate: () => mockNavigate, + }; +}); + +describe("News Sidebar navigation", () => { + afterEach(() => { + mockNavigate.mockReset(); + }); + + it("renders anchor hrefs and calls navigate on click", async () => { + const data = [ + { id: "post-1", title: "First", date: "2025-10-08" }, + { id: "post-2", title: "Second", date: "2025-10-07" }, + ]; + + render( {}} />); + + // anchors are ListItemButton with component="a" and have the href + const firstAnchor = screen.getByTestId("news-sidbar-button-post-1"); + // anchor should have href attribute set + expect(firstAnchor.getAttribute("href")).toBe("/v2/news/post-1"); + + // click should call navigate to the expected path + await userEvent.click(firstAnchor); + expect(mockNavigate).toHaveBeenCalledWith("/news/post-1", { state: { fromSidebar: true } }); + }); +}); diff --git a/frontend/src/pages/news/components/NewsContentSection.tsx b/frontend/src/pages/news/components/NewsContentSection.tsx index 9bee49b..adc3be3 100644 --- a/frontend/src/pages/news/components/NewsContentSection.tsx +++ b/frontend/src/pages/news/components/NewsContentSection.tsx @@ -27,9 +27,11 @@ const styles = { export function NewsContentSection({ data, sectionRefs, + onSectionLoad, }: { data: any[]; sectionRefs: React.MutableRefObject>; + onSectionLoad?: () => void; }) { return ( @@ -59,6 +61,7 @@ export function NewsContentSection({ height: "auto", align: "center", }} + onLoadComplete={() => onSectionLoad?.()} /> diff --git a/frontend/src/pages/news/components/NewsSideBar.tsx b/frontend/src/pages/news/components/NewsSideBar.tsx index 97f30de..3a3b72a 100644 --- a/frontend/src/pages/news/components/NewsSideBar.tsx +++ b/frontend/src/pages/news/components/NewsSideBar.tsx @@ -5,6 +5,7 @@ import { ListItemText, Typography, } from "@mui/material"; +import { useNavigate } from "react-router-dom"; import { EllipsisWithTooltip } from "../../../components/common/EllipsisWithTooltip"; const styles = { @@ -27,8 +28,9 @@ export function Sidebar({ scrollTo, }: { data: any[]; - scrollTo: (id: string) => void; + scrollTo: (id: string, smooth?: boolean) => void; }) { + const navigate = useNavigate(); return ( @@ -38,7 +40,23 @@ export function Sidebar({ {data.map((item) => ( scrollTo(item.id)} + component="a" + href={`/v2/news/${item.id}`} + onClick={(e) => { + // If user is trying to open in new tab/window (ctrl/meta/shift/alt) + // or using a non-left mouse button, allow default browser behavior. + // Otherwise prevent default and perform SPA smooth-scroll + navigate. + // e.nativeEvent is a MouseEvent with `button` property. + const me = e as React.MouseEvent; + const isModified = me.ctrlKey || me.metaKey || me.shiftKey || me.altKey || (me.nativeEvent && (me.nativeEvent as any).button !== 0); + if (isModified) return; + e.preventDefault(); + // smooth scroll to the section + scrollTo(item.id, true); + // navigate to /news/:postId and mark origin so News can avoid + // an immediate jump (we already started a smooth scroll). + navigate(`/news/${item.id}`, { state: { fromSidebar: true } }); + }} title={item.title} data-testid={`news-sidbar-button-${item.id}`} >