Franklin portrait branding, Poppins wordmark, web share dialog, Canvas link - #14
Franklin portrait branding, Poppins wordmark, web share dialog, Canvas link#14KillerQueen-Z wants to merge 3 commits into
Conversation
… Canvas link - Branding: cute Franklin portrait for favicon / app icon / apple-touch + sidebar avatar (franklin-portrait.jpg), full color (drop grayscale/sepia) - Wordmark + chat empty-state title now use Poppins (geometric sans, next/font) - Share dialog: copy link / copy text / export image (html-to-image) — wires the chat UI to the existing /api/share backend (the web had no share UI) - Canvas link in the More menu, gated by NEXT_PUBLIC_CANVAS_URL - deploy.sh forwards NEXT_PUBLIC_CANVAS_URL as a build env
The share-link payload tagged messages with their original kind (user uploads carry kind:"text"), but the /s page renders an image only when kind==="image". Result: the inline data: image traveled in the payload yet never appeared on the public page — while the in-dialog PNG export *did* show it, so 'copy link' and 'export image' silently diverged. Split each selected message into a kind:"image" bubble (when it has an inline data: image) plus a text bubble, so a text+image message keeps both and neither is dropped on the shared page.
#2 onExport swallowed every error in an empty catch — a failed PNG render (cross-origin image taints the canvas, font never resolves, zero- size node) just flipped the button back to idle with no preview and no explanation. Now it shows a transient 'Render failed' state. #3 A failed 'copy link' always showed the generic 'Share failed', even when the route returned a specific 413 reason ('Too many messages' / 'Conversation too large to share'). Capture that reason and surface it as the button's title so the user knows to deselect messages and retry.
VickyXAI
left a comment
There was a problem hiding this comment.
Pre-landing review — 7 blockers, 15 informational
Cross-model review (Claude structured + adversarial/red-team, Codex adversarial + structured, plus testing/maintainability/performance/design specialists). Scope is clean — the PR delivers exactly what it describes — but it needs a rebase and six functional fixes before merge.
🔴 Blockers
-
Merge conflict — rebase required. The branch is based on pre-redesign master (merge base
fd73648, before the homepage redesign in290bd86); GitHub reportsCONFLICTING. Good news: after rebasing, the biggest design worry (the marketing homepage piping the new full-color cartoon portrait through its old grayscale/sepia hero and header filters) evaporates — current master's homepage no longer consumesfranklin-portrait.jpg; only the /chat sidebar avatar does. -
ShareDialog.tsxonShareLink— the primary action fails on Safari/iOS and orphans a snapshot per retry.await fetch(SHARE_API)runs beforecopyText(url); Safari's transient user activation expires across the network await, so the clipboard write rejects,copyTextreturns false, and the code throws — after the snapshot was already persisted to GCS. The user sees "Share failed", the URL is never shown anywhere, and every retry writes another immutable orphan object. Fix: show the URL in the dialog on success (make copy optional) rather than gating success on the clipboard. Same gesture bug inonCopyImg(theawait fetch(previewUrl)beforeclipboard.write) — pass the blob promise intoClipboardItemsynchronously instead. -
Shares of generated (remote-URL) media render as empty bubbles on
/s. Generated images come back as remote URLs, notdata:, sodataImgis undefined and the payload sends{role:"assistant", content:"", kind:"image"}with no image — the public page shows a "Franklin" label with nothing under it. That's the flagship use case for this feature. Send a placeholder ([image]/ the prose) or render an "image omitted" state on/s. Video/music with prose also lose the prose. -
ShareDialog.tsx:203— the canvas-size clamp never engages.Math.max(1, Math.min(2, MAX / height, MAX / width)): when height exceeds 16000px,MAX/height < 1gets clamped back up to 1, so the oversized render happens anyway (crop/failure), exactly what the comment says it prevents. Drop the outerMath.max(1, …)so the ratio can go below 1. (Found independently by both Codex passes.) -
Multi-image messages silently drop images 2..n. The link payload takes only the first
data:image (.find(...)) andrenderMsgrenders onlyimgs[0], while the main chat renders all ofimages[]. A 3-attachment message shares as a 1-attachment message with no warning. -
Hidden tool activity is published without being shown in the picker. The selection list renders only the message bubbles, but
entriesinjects each assistant message's search queries and tool calls into the link payload and exported image. Users can publish raw search queries they never saw in the selector. Show the tool blocks in the picker (or make their inclusion an explicit toggle). -
javascript:URL injection on publicfranklin.run/s/*pages.MessageContent.renderInlinesetshreffrom markdown with no scheme allowlist, and/api/shareis unauthenticated withAccess-Control-Allow-Origin: *— anyone can mint a share page whose links run script on click, hosted on the trusted domain. The files are pre-existing, but this PR ships the first web UI feeding that pipeline. One-line fix: allow onlyhttp(s):hrefs inMessageContent.
Informational
- Perf: the off-screen capture DOM is always mounted while the dialog is open — every selected message renders twice (full markdown parse + duplicate base64 image decode + double
<video>/<audio>mounting, which html-to-image can't rasterize anyway); mount it only during export and usepreload="none"/ placeholders. Lazy-loadhtml-to-image(await importinonExport) andShareDialog(next/dynamic) so they leave the /chat initial bundle. - Fonts: Poppins loads 4 variants globally but only weight-600-normal has any CSS consumer — trim the config, and consider scoping it to /chat. Also
.try-empty-titleusesvar(--font-wordmark, var(--font-serif))— that fallback only applies when the var is undefined (it never is), so the zh title's CJK glyphs render in synthetic-bold size-adjusted Arial instead of Noto Serif SC. Use a comma stack:var(--font-wordmark), var(--font-noto-sc), sans-serif. - A11y: both modals declare
role="dialog" aria-modalbut have no Escape handling, focus trap, initial focus, or accessible name; selection rows are<button>s wrappingMessageContent's links/buttons and<video controls>(invalid nesting, breaks keyboard/AT); checked state is visual-only (noaria-checked); white-on-gold primary buttons are ~2.4:1 contrast; the over-limit reason lives in a hover-onlytitleon a disabled button. - i18n:
LABELSis a parallel dictionary bypassingtry-i18n(the repo convention), and "You", "Search", and the sidebar "Canvas" label are hardcoded English — zh/es users get mixed-language share images and copied text. - Types:
SharePayloadMsg/ToolStephand-mirrorSharedMessage/SharedToolStepfromshare-store.ts— import the types (type-only) so the wire contract can't drift. - Assets/CDN: franklin-icon.png is 115KB and referenced 3× in root metadata; apple-touch-icon is 60KB (both should be <25KB after pngquant/oxipng). The icons/portrait changed bytes at unchanged paths while
upload-assets.shstampsmax-age=31536000, immutable— with CDN enabled the old assets serve stale up to a year (and the un-filtered sidebar will show the OLD banknote portrait), andfranklin-icon.pngisn't inROOT_FILESat all. Version the filenames or add cache-bust; add franklin-icon to the upload list. - deploy.sh: comma-joined
--set-build-env-varscorrupts values containing commas (legal in URLs) — use gcloud's^@^alternate-delimiter syntax or reject commas. AlsoNEXT_PUBLIC_SITE_URLis never set, so a staging service would minthttps://franklin.run/s/…links. - Share limits: one
b64_jsonprovider image can exceed the 2MB payload cap → a single-image share 413s with only a 2.5s transient error; pre-measure the payload client-side. The 30-message image cap doesn't bound the link path, which fans each message into up to 3 payload messages (can hit the server's 200-message cap). /spage:getShareruns twice per view (metadata + page) underforce-dynamic— wrap inReact.cache(); and the page-levelopenGraphreplaces the root one wholesale, so share unfurls have no og:image (the one place unfurls matter). Sanitize the title for control/bidi chars before it hits<title>/OG.- Dialog lifecycle: backdrop click closes mid-share with no abort (orphan snapshot + post-unmount clipboard write); the Share button is live mid-stream, so captures can snapshot half an answer;
previewUrlholds a potentially tens-of-MB data: URL in state (usetoBlob+ object URL with revocation);cacheBust: trueappends a query param to remote media on every export — if those are signed URLs it 403s into a generic "Render failed" (verify). - Pre-existing but now user-facing:
/api/shareis an unauthenticated durable-write endpoint with CORS*, no rate limit or TTL (the route's own comment says to add rate-limiting before public launch — this PR is that launch); the storage errore.messageis returned verbatim to callers.
Verified along the way: /api/share handles kind:"tools" and strips non-data: images; /s renders tools blocks; activityUsed exists in all three dicts; double-click double-POST is effectively guarded; sel index coupling survives streaming appends.
What
franklin-portrait.jpg). Dropped the old grayscale/sepia filter so it shows full color.next/font.html-to-image), wired to the existing/api/sharebackend +/s/[id]page. Tool activity travels via the messageactivity.NEXT_PUBLIC_CANVAS_URLis set (external, new tab).deploy.shforwards that var as a build env.Notes
master(includes Restore marketing homepage at /, move chat app to /chat #13's marketing-home //chatmove). OnlyHistorySidebar.tsxoverlapped and auto-merged cleanly.typecheck+lintpass.html-to-image.npm run deployafter merge; setNEXT_PUBLIC_CANVAS_URL(+ ensurepublic/icons reach the CDN bucket) for the icon/Canvas-link to show live.