Commit Graph

1 Commits

Author SHA1 Message Date
Michael Ramos ba2e4d2a1d feat(ui): markdown reader parity — HTML blocks, GitHub alerts, GFM inline extras (#597)
* feat(ui): markdown reader parity — HTML blocks, GitHub alerts, GFM inline extras

Brings the in-app markdown reader to parity with GitHub's flavored rendering.
Additive across the parser + renderer; no behavior change for existing blocks.

Refactor:
- Extract InlineMarkdown (262 lines) out of Viewer.tsx into its own file
- Extract BlockRenderer + block-type components (CodeBlock, HtmlBlock, AlertBlock,
  Callout) into components/blocks/ — Viewer drops from 1279 to ~770 lines
- Each new block-level feature lands in BlockRenderer or a new blocks/*.tsx,
  not Viewer

Block-level features:
- Raw HTML blocks (<details>, <summary>, etc.) via balanced-tag parser branch,
  rendered through marked + DOMPurify for nested-markdown support; inner innerHTML
  set imperatively so React reconciliation doesn't collapse open <details>
- GitHub alerts (> [!NOTE] / [!TIP] / [!WARNING] / [!CAUTION] / [!IMPORTANT])
  with inline Octicons, title-case labels, GitHub's Primer colors (light + dark)
- Directive containers (:::kind ... :::) with arbitrary kinds for project-specific
  callouts (note, tip, warning, danger, info, success, question, etc.)
- Heading anchor ids — slugifyHeading() strips inline markdown, preserves unicode

Inline features (all in InlineMarkdown, all code-span-safe):
- Bare URL autolinks (https://...) with trailing-punctuation trimming
- @mentions and #issue-refs — render as clickable links when repo is GitHub,
  styled spans otherwise; threaded via repoInfo.display through BlockRenderer
- Emoji shortcodes (👋, 🚀, 29 curated codes) via transformPlainText()
- Smart punctuation (curly quotes, em/en dashes, ellipsis) applied only to
  plain-text fragments after code spans have been consumed

Safety:
- Render-time transforms live inside InlineMarkdown's plain-text push, which
  is only reached after code-span regex consumes code content. Backticks stay
  literal for shell/regex snippets.
- DOMPurify allowlist (no on* handlers, no style attrs, no scripts) gates every
  raw HTML block. Unsafe link protocols (javascript:/data:/vbscript:/file:)
  stripped by sanitizeLinkUrl.

Tests: +40 (149 total). New files:
- utils/slugify.test.ts (10) — unicode, markdown stripping, edge cases
- utils/inlineTransforms.test.ts (9) — emoji + smartypants
- utils/parser.test.ts — alert detection (5 cases), directives (5 cases), HTML
  block balancing (5 cases)

Fixtures for manual verification:
- tests/test-fixtures/11-html-blocks.md
- tests/test-fixtures/12-gfm-and-inline-extras.md (release-plan-shaped demo)

Known limitations (not blockers):
- Bare URL regex doesn't balance parens (https://en.wikipedia.org/wiki/Foo_(bar)
  drops the trailing ")")
- Duplicate heading text → duplicate anchor ids (browser picks first on hash nav)
- Directive body is inline-only (no nested headings/lists)

For provenance purposes, this commit was AI assisted.

* fix(ui): wire typecheck for packages/ui, address PR review findings

Root-cause fix for the missing-import bug caught in review: the UI package
had no tsconfig.json and no typecheck script, so missing references like
`getImageSrc` in the extracted InlineMarkdown slipped past vite/esbuild
(which only type-strip, they don't resolve imports).

Infrastructure:
- Added packages/ui/tsconfig.json with proper module resolution, JSX config,
  and bundler-style paths.
- Added globals.d.ts to accept side-effect CSS imports.
- Added @types/react, @types/react-dom, @types/bun, @types/dompurify as
  devDeps on packages/ui so React / Bun / DOMPurify types actually resolve.
- Wired `tsc --noEmit -p packages/ui/tsconfig.json` into the top-level
  `bun run typecheck` script.

With the typecheck running, 0 errors remain in this PR's scope. Four
pre-existing errors on main (plan-diff SVG type narrowing, sharing.ts
SharePayload shape) are unrelated and tracked separately.

Review fixes:
- InlineMarkdown: import getImageSrc from ImageThumbnail. Was calling the
  helper without importing it — markdown images with relative paths
  (`![alt](./foo.png)`) would throw ReferenceError at render. Regression
  caused by the InlineMarkdown extraction.
- useAnnotationHighlighter: findTextInDOM now retries with the rendered
  form (transformPlainText) when the raw originalText doesn't match.
  Annotations made before smart-punctuation / emoji shortcodes shipped
  (straight quotes, `👋` text) still re-bind after reload.
- sanitizeHtml: allow the `open` attribute so `<details open>` preserves
  its default-expanded state instead of always rendering collapsed.
- parser.test.ts: narrow a string->AlertKind assertion to satisfy strict
  typechecking.

Deferred (tracked as known limitation in PR description):
- HtmlBlock relative URL rewriting for nested <img src="./logo.png"> /
  <a href="note.md">. New-feature gap, not a regression.

For provenance purposes, this commit was AI assisted.

* fix(ui): HtmlBlock rewrites relative <img>/<a> refs to match markdown paths

Raw HTML blocks inject sanitized HTML verbatim, so nested <img src="./logo.png">
and <a href="notes.md"> resolved against the plannotator server URL instead of
the plan's directory — images 404'd, .md links navigated away instead of
opening in the linked-doc overlay. This is the path README.md content hits
(hero <img>, YouTube thumbnails, <details> sections with anchors).

Fix: after setting innerHTML, walk <img> and <a> elements and apply the same
rewriting markdown content uses:
- <img> relative src → getImageSrc(src, imageBaseDir), routing through
  /api/image?path=... with the plan's base directory.
- <a> relative href matching .md / .mdx / .html → click handler that calls
  onOpenLinkedDoc, same pattern as [label](./foo.md) markdown links.
- http(s):, data:, blob:, mailto:, tel:, and #anchor hrefs pass through
  untouched.

BlockRenderer now threads imageBaseDir + onOpenLinkedDoc into HtmlBlock.
React.memo equality extended to compare those props too, so legitimate
changes still re-run the rewrite pass without forcing re-renders on
every parent update.

Verified against the repo's own README.md — hero image, YouTube thumbnails,
and <details> sections all render correctly in annotate mode.

For provenance purposes, this commit was AI assisted.

* feat(ui): table conveniences — hover toolbar, popout dialog with sort/filter

Extracts table rendering into blocks/TableBlock and adds two companion
surfaces: a hover toolbar for quick copy, and a full-screen popout dialog
with TanStack-powered sort/filter/copy for power use. No pagination — plan
tables don't get that big.

Hover toolbar (blocks/TableToolbar.tsx):
- Floats above the table on mouse enter via React portal, positioned with
  getBoundingClientRect + scroll/resize listeners, entry/exit animations.
  Same positional pattern as AnnotationToolbar's top-right mode.
- Debounced hover state in Viewer (100ms leave → 150ms exit animation),
  mirroring hoveredCodeBlock's state machine.
- Three actions: Copy markdown (icon), CSV (short uppercase button,
  RFC 4180 escaping), Expand (opens popout).

Popout dialog (blocks/TablePopout.tsx):
- Radix Dialog, fullscreen-ish card with ~2rem backdrop visible for
  click-to-close. max-w-[min(calc(100vw-4rem),1500px)].
- Portaled into Viewer's containerRef so the annotation hook can walk
  into the popout's text nodes — selection-based annotations, text-search
  restoration, and shared blockId all work across the collapsed and
  popped-out views.
- TanStack Table for the grid: click column headers to sort (asc → desc →
  clear), global filter input, no pagination. Row count indicator shows
  "15 of 27" when filter reduces the set.
- Copy / CSV buttons in the header row: filter- and sort-aware. When
  visible rows < total, tooltips read "Copy 15 rows as markdown" /
  "Copy 15 rows as CSV". When no filter, copies whole table (normalized
  whitespace). Read is one-shot on click — no derived state to sync.
- Floating X close button (absolute top-right), no header bar.

Chrome stacking while popout is open (CSS-only, via :has()):
- body:has([data-popout="true"]) drops four element types behind the
  dialog: annotation sidebar, sticky header lane, app nav header, overlay
  scrollbars. :has() observes the dialog's presence directly — when the
  dialog unmounts, the selector stops matching and everything returns to
  natural stacking. No JS state, no useEffect cleanup.

Shared helpers in TableBlock.tsx (exported):
- parseTableContent — pipe-delimited markdown → { headers, rows }
- buildCsvFromRows / buildMarkdownTable — inverse, from parsed data
- buildCsv — thin wrapper for the hover toolbar's raw-block path

Dependencies added:
- @radix-ui/react-dialog ^1.1.15 (~6 KB gzipped)
- @tanstack/react-table ^8.21.3 (~14 KB gzipped)

Fixture:
- tests/test-fixtures/12-gfm-and-inline-extras.md — added a 27×11
  "Detailed feature backlog" table to exercise wide + deep tables,
  horizontal scroll in the popout, and the sort/filter flows.

For provenance purposes, this commit was AI assisted.

* fix(ui): table popout — annotation flow, chrome stacking, sidebar tabs

Tightens the popout so annotations work inside it and chrome doesn't
overlap the dialog.

Annotation flow inside popout:
- Radix Dialog modal={false} so the focus trap doesn't yank focus back
  from CommentPopover's textarea (CommentPopover portals to document.body,
  outside the dialog's DOM subtree).
- Dialog.Content onInteractOutside handler whitelists the annotation
  toolbar, CommentPopover, and FloatingQuickLabelPicker so clicking
  them doesn't dismiss the dialog. Backdrop click + Escape still close.
- aria-describedby={undefined} on Dialog.Content (Radix opt-out; the
  popout doesn't need a description).
- React.memo on TablePopout with a custom comparator (block id/content,
  open, container, imageBaseDir, githubRepo). Prevents upstream Viewer
  re-renders from re-running TanStack's flexRender on every cell, which
  conflicted with web-highlighter's live DOM mutations and caused a
  NotFoundError in React's reconciler.

Widget markers for :has()-based chrome stacking:
- [data-comment-popover="true"] on CommentPopover (both popover + dialog
  variants).
- [data-floating-picker="true"] on FloatingQuickLabelPicker.
- [data-sidebar-tabs="true"] on SidebarTabs (left-side TOC/Files/Versions
  flags that sit on top of the dialog otherwise).
- theme.css extended: sidebar tabs join the annotation sidebar, sticky
  header lane, app header, and overlay scrollbars in dropping to
  z-index -1 while body:has([data-popout="true"]) matches.

Known limitation (not addressed): annotations created inside the popout
show their <mark> only while the popout is open; when it closes, the
<mark> unmounts with the popout's DOM and does not reappear on the
collapsed table. The annotation itself persists in state (sidebar,
shared URLs, exports). Round-tripping visual marks between popout and
collapsed view requires either a second web-highlighter instance or a
switch to the CSS Custom Highlight API — out of scope here.

For provenance purposes, this commit was AI assisted.

* fix(ui): review findings — flags, alerts, forges, tabs, anchors, URL brackets

Six targeted fixes from the v0.19 PR review. Each is small and scoped;
the riskier items from the review (plan-diff block variants, HTML
relative non-doc links) are tracked as follow-ups.

Smart punctuation — CLI flags preserved:
- Narrowed the `--` → en-dash rule to only fire between digits
  (`pages 3--5` still converts; `bun --watch` stays literal).

GitHub alerts — list/code/heading bodies absorb correctly:
- Blockquote merge now always merges into a previous alert blockquote,
  regardless of whether the new line starts with a block marker. Without
  this, `> [!NOTE]\n> - item` split the list off into a plain italic
  quote and emptied the alert.
- AlertBlock got a mini block-level renderer for the body so `- item` /
  `* item` / `1. item` render as real <ul>/<ol>, not flattened prose.

Forge-aware mentions/issue refs:
- packages/shared/repo: new parseRemoteHost() extracts the host from
  the git remote URL; RepoInfo gains an optional `host` field.
- packages/server/repo: getRepoInfo populates host alongside display.
- Viewer only passes githubRepo to InlineMarkdown when the host is
  exactly "github.com". Non-GitHub repos render mentions/issue refs
  as styled text, no wrong github.com links.

HTML block external links:
- rewriteRelativeRefs now forces `target="_blank"` and
  `rel="noopener noreferrer"` on every external http(s) link inside
  raw HTML. Fixes two problems in one pass: external links no longer
  hijack the review tab, and pasted-HTML links can't tab-nab the
  plannotator tab via window.opener.

Heading anchor dedup:
- New buildHeadingSlugMap() walks all heading blocks and assigns
  `foo`, `foo-1`, `foo-2`, ... for repeats (GitHub convention).
  BlockRenderer receives the anchor id as a prop from Viewer via a
  memoized map rather than computing per-block; first occurrence
  keeps the bare slug so existing links stay stable.

URL autolink bracket balance:
- Trailing `)`/`]`/`}` in bare URLs are kept when they balance an
  earlier opener inside the URL. Wikipedia-style
  `https://en.wikipedia.org/wiki/Function_(mathematics)` now keeps its
  paren; `(see https://x.com)` still trims the orphan.

Tests: +8 (157 total).
- utils/slugify.test: buildHeadingSlugMap dedup behavior, non-heading
  skipping, empty-slug skipping.
- utils/inlineTransforms.test: CLI flags stay literal, `3--5` still
  converts.
- utils/parser.test: alerts with list body / code fence body, blank
  line ending an alert.

Fixture:
- tests/test-fixtures/13-known-issues.md — reproduces each of the
  review findings end-to-end; useful as a regression check going
  forward.

Deferred (tracked for follow-up):
- Plan diff view doesn't render html / directive / alertKind semantics
  (SimpleBlockRenderer has no cases for the new block variants).
- Relative non-doc links inside raw HTML (.pdf, .csv) don't get
  rewritten — only .md/.mdx/.html are routed through the linked-doc
  overlay today. Not a regression; narrow audience.

For provenance purposes, this commit was AI assisted.

* fix(ui): round-3 review — drop host gate, link paren balance, data/blob images

- Viewer: remove repoInfo.host === 'github.com' gate so @user/#123 links
  render for GitHub Enterprise and runtimes (Pi) that don't populate host.
- HtmlBlock: treat protocol-relative //host links as external and harden
  with target=_blank rel=noopener noreferrer.
- InlineMarkdown: data:/blob: image sources bypass /api/image rewrite.
- InlineMarkdown: replace [text](url) regex with a depth-tracking scanner
  so URLs with balanced parens (Wikipedia /Function_(mathematics)) and
  backslash-escapes no longer truncate. Empty text/url guard preserves
  prior fall-through behavior.
- InlineMarkdown: isLocalDoc accepts .md/.mdx/.html/.htm with optional
  #fragment; fragment stripped before onOpenLinkedDoc so guide.md#setup
  opens the linked doc instead of a broken anchor.

For provenance purposes, this commit was AI assisted.

* fix(ui): round-4 review — table pipe escape, callout lists, emoji h-splitter

- TableBlock: buildMarkdownTable now re-escapes literal | as \| in each
  cell. parseTableContent already unescapes on parse; without the mirror
  on serialize, the popout's copy-as-markdown produces extra columns for
  tables with pipes in regex, shell, or boolean content.
- AlertBlock + Callout: extract the shared paragraph-and-list body
  renderer into blocks/proseBody.tsx. Fixes directive callouts (:::note
  with a bulleted list) rendering as literal hyphens instead of a list.
  Paragraph lines join with '\n' so InlineMarkdown's hard-break handler
  still fires. Callout passes an empty text-color class so directive
  color tokens inherited from the container are preserved.
- InlineMarkdown: drop `h` from the plaintext chunk-break class; it was
  splitting emoji shortcodes like ❤️, 👍, 🤔 at the
  h, so the :word: pattern never reassembled and transformPlainText
  couldn't replace the shortcode. Bare URL detection moves inline via
  emitPlainTextWithBareUrls, which scans chunks for https?:// at word
  boundaries and emits anchors, passing surrounding text through
  transformPlainText so emoji + smart punctuation still apply to
  non-URL slices.
- InlineMarkdown: extract trimUrlTail (shared between the top-of-loop
  URL branch and the new inline scanner) — one balanced-paren trim
  implementation instead of two. +8 unit tests covering the trim cases
  (Wikipedia parens, unbalanced brackets, stacked punctuation).
- Fixture: section 9 in 13-known-issues.md demonstrates the table copy
  corruption for manual verification.

For provenance purposes, this commit was AI assisted.

* fix(ui): resolve pre-existing typecheck errors surfacing in CI

- PlanCleanDiffView: narrow heading Tag to 'h1'..'h6' so hover props
  resolve to HTMLHeadingElement instead of the SVGSymbolElement branch
  of keyof IntrinsicElements.
- VSCodeIcon: spread mask-type as a kebab-case attribute; React 19's
  typings no longer expose the camelCase maskType prop on SVG masks.
- useSharing / sharing: cast decompress() result to SharePayload — the
  shared compress module returns unknown by design; callers were
  implicitly any and TS 5.x now flags the assignment.

For provenance purposes, this commit was AI assisted.
2026-04-21 18:56:41 -07:00