fix: address PR 3355 review regressions

This commit is contained in:
Dotta 2026-04-11 06:40:37 -05:00
parent dc94e3d1df
commit b48be80d5d
6 changed files with 55 additions and 22 deletions

View file

@ -12,6 +12,7 @@ import { useLocation } from "@/lib/router";
import { ApiError } from "../api/client"; import { ApiError } from "../api/client";
import { issuesApi } from "../api/issues"; import { issuesApi } from "../api/issues";
import { useAutosaveIndicator } from "../hooks/useAutosaveIndicator"; import { useAutosaveIndicator } from "../hooks/useAutosaveIndicator";
import { deriveDocumentRevisionState } from "../lib/document-revisions";
import { queryKeys } from "../lib/queryKeys"; import { queryKeys } from "../lib/queryKeys";
import { cn, relativeTime } from "../lib/utils"; import { cn, relativeTime } from "../lib/utils";
import { MarkdownBody } from "./MarkdownBody"; import { MarkdownBody } from "./MarkdownBody";
@ -536,10 +537,10 @@ export function IssueDocumentsSection({
}, []); }, []);
const previewRevision = useCallback((doc: IssueDocument, revisionId: string) => { const previewRevision = useCallback((doc: IssueDocument, revisionId: string) => {
const revisions = getDocumentRevisions(doc.key); const revisionState = deriveDocumentRevisionState(doc, getDocumentRevisions(doc.key));
const selectedRevision = revisions.find((revision) => revision.id === revisionId); const selectedRevision = revisionState.revisions.find((revision) => revision.id === revisionId);
if (!selectedRevision) return; if (!selectedRevision) return;
if (selectedRevision.id === doc.latestRevisionId) { if (selectedRevision.id === revisionState.currentRevision.id) {
returnToLatestRevision(doc.key); returnToLatestRevision(doc.key);
return; return;
} }
@ -787,7 +788,10 @@ export function IssueDocumentsSection({
const activeDraft = draft?.key === doc.key && !draft.isNew ? draft : null; const activeDraft = draft?.key === doc.key && !draft.isNew ? draft : null;
const activeConflict = documentConflict?.key === doc.key ? documentConflict : null; const activeConflict = documentConflict?.key === doc.key ? documentConflict : null;
const isFolded = foldedDocumentKeys.includes(doc.key); const isFolded = foldedDocumentKeys.includes(doc.key);
const revisionHistory = getDocumentRevisions(doc.key); const rawRevisionHistory = getDocumentRevisions(doc.key);
const revisionState = deriveDocumentRevisionState(doc, rawRevisionHistory);
const revisionHistory = revisionState.revisions;
const currentRevision = revisionState.currentRevision;
const selectedRevisionId = selectedRevisionIds[doc.key] ?? null; const selectedRevisionId = selectedRevisionIds[doc.key] ?? null;
const selectedHistoricalRevision = selectedRevisionId const selectedHistoricalRevision = selectedRevisionId
? revisionHistory.find((revision) => revision.id === selectedRevisionId) ?? null ? revisionHistory.find((revision) => revision.id === selectedRevisionId) ?? null
@ -795,10 +799,10 @@ export function IssueDocumentsSection({
const isHistoricalPreview = Boolean(selectedHistoricalRevision); const isHistoricalPreview = Boolean(selectedHistoricalRevision);
const displayedTitle = selectedHistoricalRevision const displayedTitle = selectedHistoricalRevision
? selectedHistoricalRevision.title ?? "" ? selectedHistoricalRevision.title ?? ""
: activeDraft?.title ?? doc.title ?? ""; : activeDraft?.title ?? currentRevision.title ?? "";
const displayedBody = selectedHistoricalRevision?.body ?? activeDraft?.body ?? doc.body; const displayedBody = selectedHistoricalRevision?.body ?? activeDraft?.body ?? currentRevision.body;
const displayedRevisionNumber = selectedHistoricalRevision?.revisionNumber ?? doc.latestRevisionNumber; const displayedRevisionNumber = selectedHistoricalRevision?.revisionNumber ?? currentRevision.revisionNumber;
const displayedUpdatedAt = selectedHistoricalRevision?.createdAt ?? doc.updatedAt; const displayedUpdatedAt = selectedHistoricalRevision?.createdAt ?? currentRevision.createdAt;
const showTitle = !isPlanKey(doc.key) && !!displayedTitle.trim() && !titlesMatchKey(displayedTitle, doc.key); const showTitle = !isPlanKey(doc.key) && !!displayedTitle.trim() && !titlesMatchKey(displayedTitle, doc.key);
const canVoteOnDocument = Boolean(doc.latestRevisionId && doc.updatedByAgentId && !doc.updatedByUserId && onVote); const canVoteOnDocument = Boolean(doc.latestRevisionId && doc.updatedByAgentId && !doc.updatedByUserId && onVote);
@ -845,12 +849,12 @@ export function IssueDocumentsSection({
</DropdownMenuTrigger> </DropdownMenuTrigger>
<DropdownMenuContent align="start" className="w-72"> <DropdownMenuContent align="start" className="w-72">
<DropdownMenuLabel>Revision history</DropdownMenuLabel> <DropdownMenuLabel>Revision history</DropdownMenuLabel>
{revisionMenuOpenKey === doc.key && isFetchingDocumentRevisions && revisionHistory.length === 0 ? ( {revisionMenuOpenKey === doc.key && isFetchingDocumentRevisions && rawRevisionHistory.length === 0 ? (
<DropdownMenuItem disabled>Loading revisions...</DropdownMenuItem> <DropdownMenuItem disabled>Loading revisions...</DropdownMenuItem>
) : revisionHistory.length > 0 ? ( ) : revisionHistory.length > 0 ? (
<DropdownMenuRadioGroup value={selectedRevisionId ?? doc.latestRevisionId ?? ""}> <DropdownMenuRadioGroup value={selectedRevisionId ?? currentRevision.id ?? ""}>
{revisionHistory.map((revision) => { {revisionHistory.map((revision) => {
const isCurrentRevision = revision.id === doc.latestRevisionId; const isCurrentRevision = revision.id === currentRevision.id;
return ( return (
<DropdownMenuRadioItem <DropdownMenuRadioItem
key={revision.id} key={revision.id}

View file

@ -155,4 +155,20 @@ describe("MarkdownBody", () => {
expect(html).toContain("<code>PAP-1271</code>"); expect(html).toContain("<code>PAP-1271</code>");
expect(html).toContain("text-green-600"); expect(html).toContain("text-green-600");
}); });
it("can opt out of issue reference linkification for offline previews", () => {
const html = renderToStaticMarkup(
<QueryClientProvider client={new QueryClient()}>
<ThemeProvider>
<MarkdownBody linkIssueReferences={false}>
{"Depends on PAP-1271 and [manual link](PAP-1271)."}
</MarkdownBody>
</ThemeProvider>
</QueryClientProvider>,
);
expect(html).not.toContain('href="/issues/PAP-1271"');
expect(html).toContain("Depends on PAP-1271");
expect(html).toContain('href="PAP-1271"');
});
}); });

View file

@ -1,6 +1,6 @@
import { isValidElement, useEffect, useId, useState, type ReactNode } from "react"; import { isValidElement, useEffect, useId, useState, type ReactNode } from "react";
import { useQuery } from "@tanstack/react-query"; import { useQuery } from "@tanstack/react-query";
import Markdown, { type Components } from "react-markdown"; import Markdown, { type Components, type Options } from "react-markdown";
import remarkGfm from "remark-gfm"; import remarkGfm from "remark-gfm";
import { cn } from "../lib/utils"; import { cn } from "../lib/utils";
import { useTheme } from "../context/ThemeContext"; import { useTheme } from "../context/ThemeContext";
@ -17,6 +17,7 @@ interface MarkdownBodyProps {
className?: string; className?: string;
style?: React.CSSProperties; style?: React.CSSProperties;
softBreaks?: boolean; softBreaks?: boolean;
linkIssueReferences?: boolean;
/** Optional resolver for relative image paths (e.g. within export packages) */ /** Optional resolver for relative image paths (e.g. within export packages) */
resolveImageSrc?: (src: string) => string | null; resolveImageSrc?: (src: string) => string | null;
/** Called when a user clicks an inline image */ /** Called when a user clicks an inline image */
@ -125,11 +126,23 @@ function MermaidDiagramBlock({ source, darkMode }: { source: string; darkMode: b
); );
} }
export function MarkdownBody({ children, className, style, softBreaks = true, resolveImageSrc, onImageClick }: MarkdownBodyProps) { export function MarkdownBody({
children,
className,
style,
softBreaks = true,
linkIssueReferences = true,
resolveImageSrc,
onImageClick,
}: MarkdownBodyProps) {
const { theme } = useTheme(); const { theme } = useTheme();
const remarkPlugins = softBreaks const remarkPlugins: NonNullable<Options["remarkPlugins"]> = [remarkGfm];
? [remarkGfm, remarkLinkIssueReferences, remarkSoftBreaks] if (linkIssueReferences) {
: [remarkGfm, remarkLinkIssueReferences]; remarkPlugins.push(remarkLinkIssueReferences);
}
if (softBreaks) {
remarkPlugins.push(remarkSoftBreaks);
}
const components: Components = { const components: Components = {
pre: ({ node: _node, children: preChildren, ...preProps }) => { pre: ({ node: _node, children: preChildren, ...preProps }) => {
const mermaidSource = extractMermaidSource(preChildren); const mermaidSource = extractMermaidSource(preChildren);
@ -139,7 +152,7 @@ export function MarkdownBody({ children, className, style, softBreaks = true, re
return <pre {...preProps}>{preChildren}</pre>; return <pre {...preProps}>{preChildren}</pre>;
}, },
a: ({ href, children: linkChildren }) => { a: ({ href, children: linkChildren }) => {
const issueRef = parseIssueReferenceFromHref(href); const issueRef = linkIssueReferences ? parseIssueReferenceFromHref(href) : null;
if (issueRef) { if (issueRef) {
return ( return (
<MarkdownIssueLink issuePathId={issueRef.issuePathId} href={issueRef.href}> <MarkdownIssueLink issuePathId={issueRef.issuePathId} href={issueRef.href}>

View file

@ -532,10 +532,10 @@ function ExportPreviewPane({
{parsed ? ( {parsed ? (
<> <>
<FrontmatterCard data={parsed.data} onSkillClick={onSkillClick} /> <FrontmatterCard data={parsed.data} onSkillClick={onSkillClick} />
{parsed.body.trim() && <MarkdownBody resolveImageSrc={resolveImageSrc} softBreaks={false}>{parsed.body}</MarkdownBody>} {parsed.body.trim() && <MarkdownBody resolveImageSrc={resolveImageSrc} softBreaks={false} linkIssueReferences={false}>{parsed.body}</MarkdownBody>}
</> </>
) : isMarkdown ? ( ) : isMarkdown ? (
<MarkdownBody resolveImageSrc={resolveImageSrc} softBreaks={false}>{textContent ?? ""}</MarkdownBody> <MarkdownBody resolveImageSrc={resolveImageSrc} softBreaks={false} linkIssueReferences={false}>{textContent ?? ""}</MarkdownBody>
) : imageSrc ? ( ) : imageSrc ? (
<div className="flex min-h-[520px] items-center justify-center rounded-lg border border-border bg-accent/10 p-6"> <div className="flex min-h-[520px] items-center justify-center rounded-lg border border-border bg-accent/10 p-6">
<img src={imageSrc} alt={selectedFile} className="max-h-[480px] max-w-full object-contain" /> <img src={imageSrc} alt={selectedFile} className="max-h-[480px] max-w-full object-contain" />

View file

@ -241,10 +241,10 @@ function ImportPreviewPane({
{parsed ? ( {parsed ? (
<> <>
<FrontmatterCard data={parsed.data} /> <FrontmatterCard data={parsed.data} />
{parsed.body.trim() && <MarkdownBody resolveImageSrc={resolveImageSrc} softBreaks={false}>{parsed.body}</MarkdownBody>} {parsed.body.trim() && <MarkdownBody resolveImageSrc={resolveImageSrc} softBreaks={false} linkIssueReferences={false}>{parsed.body}</MarkdownBody>}
</> </>
) : isMarkdown ? ( ) : isMarkdown ? (
<MarkdownBody resolveImageSrc={resolveImageSrc} softBreaks={false}>{textContent ?? ""}</MarkdownBody> <MarkdownBody resolveImageSrc={resolveImageSrc} softBreaks={false} linkIssueReferences={false}>{textContent ?? ""}</MarkdownBody>
) : imageSrc ? ( ) : imageSrc ? (
<div className="flex min-h-[520px] items-center justify-center rounded-lg border border-border bg-accent/10 p-6"> <div className="flex min-h-[520px] items-center justify-center rounded-lg border border-border bg-accent/10 p-6">
<img src={imageSrc} alt={selectedFile} className="max-h-[480px] max-w-full object-contain" /> <img src={imageSrc} alt={selectedFile} className="max-h-[480px] max-w-full object-contain" />

View file

@ -742,7 +742,7 @@ function SkillPane({
/> />
) )
) : file.markdown && viewMode === "preview" ? ( ) : file.markdown && viewMode === "preview" ? (
<MarkdownBody softBreaks={false}>{body}</MarkdownBody> <MarkdownBody softBreaks={false} linkIssueReferences={false}>{body}</MarkdownBody>
) : ( ) : (
<pre className="overflow-x-auto whitespace-pre-wrap wrap-break-word border-0 bg-transparent p-0 font-mono text-sm text-foreground"> <pre className="overflow-x-auto whitespace-pre-wrap wrap-break-word border-0 bg-transparent p-0 font-mono text-sm text-foreground">
<code>{file.content}</code> <code>{file.content}</code>