feat(docs): add 'Copy page as Markdown' action#329
Conversation
|
@BittuBarnwal7479 is attempting to deploy a commit to the Kaelio Team on Vercel. A member of the Team first needs to authorize it. |
luca-martial
left a comment
luca-martial
left a comment
There was a problem hiding this comment.
Design + architecture review
Thanks for this, Bittu — the feature is genuinely wanted (fixes #326) and the menu wiring is solid. I'm requesting changes on two blockers and a few polish items. The good news: the main fix makes the diff smaller, not bigger.
Must-fix before merge
- Copy from the canonical
.mdroute, not a DOM scraper. The site already serves source-accurate Markdown atmarkdownHref, so "Copy page" and "View as Markdown" should return the same bytes. (inline ononCopy) - The split-button visual implies the left half copies, but both halves open the menu. (inline on the trigger)
Should-fix
3. Use the real ChatGPT/Claude brand marks.
4. Commit to the role="menu" keyboard contract, or downgrade to a disclosure.
Nice-to-have
5. Dial the trigger back to the site's rounded-lg/shadow-sm language.
6. Derive the same-origin Markdown link instead of hardcoding the prod origin.
Details inline. Happy to pair on the fetch-based version if that's easier than another round.
| try { | ||
| await navigator.clipboard.writeText(stripFrontmatter(mdxSource)); | ||
| await navigator.clipboard.writeText( | ||
| buildPageMarkdown({ title, description, content }), |
There was a problem hiding this comment.
Blocker: two sources of truth for "page as Markdown." This scrapes the rendered DOM through a ~200-line hand-rolled converter, but the site already serves canonical Markdown (from the source .mdx) at the .md route, which this component already knows as markdownHref and which the "View as Markdown" item links to. So these two adjacent menu items return different Markdown and will drift. The scraper is also lower-fidelity: it flattens MDX components (Callouts, Tabs, Steps, Cards collapse to raw text) and reads the code-fence language from a language-* class that this site's Shiki CodeBlock may not set, so fenced blocks can lose their language.
Suggest replacing the DOM path with the route the menu already references:
const md = await fetch(markdownHref).then((r) => r.text());
await navigator.clipboard.writeText(md);That deletes buildPageMarkdown and the whole converter (~200 lines), removes the contentId / DocsBody id plumbing added to page.tsx, and guarantees Copy == View. One implementation, canonical output.
| } | ||
| }; | ||
|
|
||
| const absoluteMarkdownUrl = `https://docs.kaelio.com/ktx${markdownHref}`; |
There was a problem hiding this comment.
https://docs.kaelio.com/ktx is hardcoded, so "View as Markdown" jumps to prod even from localhost or a preview deploy. Derive the same-origin link from window.location.origin for the Markdown view, and keep an absolute URL only for the ChatGPT/Claude prompts (which genuinely need a public URL).
| type="button" | ||
| onClick={onCopy} | ||
| className="inline-flex h-8 items-center rounded-md border border-fd-border bg-fd-background px-3 font-medium text-fd-muted-foreground transition-colors hover:border-fd-primary/40 hover:text-fd-foreground data-[state=copied]:border-emerald-500/40 data-[state=copied]:text-emerald-600" | ||
| onClick={() => setOpen((current) => !current)} |
There was a problem hiding this comment.
Blocker: split-button form vs. function. The trigger is drawn as a split button — icon + "Copy page" label, a border-l divider, then a chevron — which universally means "the labeled half performs the action, the chevron opens the menu." But this handler makes both halves just toggle the menu, so clicking "Copy page" doesn't copy; it opens a menu whose first item is also "Copy page." The label is a decoy and copying takes two clicks. Either wire the left segment to onCopy directly (true split button), or drop the split styling so it reads as a single dropdown trigger.
| aria-haspopup="menu" | ||
| aria-expanded={open} | ||
| aria-controls={menuId} | ||
| className="inline-flex h-10 items-center overflow-hidden rounded-full border border-fd-border bg-fd-background text-fd-foreground shadow-sm transition-colors hover:border-fd-primary/40" |
There was a problem hiding this comment.
Visual weight runs hotter than the rest of this site. rounded-full on a text button has no precedent in this codebase — it's used only for dots, avatars, and small badges; every interactive rectangle is rounded-md/rounded-lg. And the menu's shadow-xl (line 95) is heavier than anything else on the site, which tops out at shadow-sm. Next to the large Outfit DocsTitle this reads like a marketing CTA rather than a docs utility. Suggest rounded-lg, shadow-sm on the popover, and font-medium so it sits with the existing card language in product-runtime.tsx / semantic-layer-flow.tsx.
| {open && ( | ||
| <div | ||
| id={menuId} | ||
| role="menu" |
There was a problem hiding this comment.
aria-haspopup="menu" + role="menu"/menuitem promise a menu widget, but the keyboard contract isn't implemented: arrow-key navigation, focus moving into the menu on open, and focus returning to the trigger on Escape are all missing, and MenuLink clicks don't close the menu. Either implement roving focus (arrow keys, Home/End, focus-in on open, focus-return on close), or — simpler for four items — downgrade to a disclosure (aria-expanded button revealing a plain group of links/buttons), which needs none of that machinery.
| ); | ||
| } | ||
|
|
||
| function ChatGptIcon() { |
There was a problem hiding this comment.
These ChatGPT/Claude glyphs are improvised (an abstract knot, and a circle-with-rays for Claude), not the recognizable brand marks — on a public docs site that reads as unpolished. Both marks are in simple-icons; using the official monochrome SVGs makes "Open in ChatGPT / Claude" instantly legible.
|
Updated the test to expect the full SL compute payload: query, sources, and dialect. |
2 participants
fixes #326
Changes:
compressed-video.mp4