Repository navigation
feat(frontend): template strip, one shared template surface across home, onboarding, and agent chats (flag-gated) - #5098
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new environment-flagged "Template Strip" onboarding experience: a shared horizontally-scrollable template selection component with category tabs, provenance chip, clipboard copy flow, and toast, integrated into agent-home and playground-native onboarding surfaces, plus new theme tokens and design documentation. ChangesDesign Documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Template Strip Implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant StripHome
participant useTemplateProvenance
participant useAgentHomeActions
participant PostHog
User->>StripHome: click template card
StripHome->>useTemplateProvenance: pick(template)
StripHome->>PostHog: capture template_pick event
User->>StripHome: click Create agent
StripHome->>useAgentHomeActions: onCreate(autoSendSeed)
sequenceDiagram
participant User
participant StripComposer
participant buildCodingAgentClipboard
participant Clipboard
participant CopiedToast
User->>StripComposer: click "Use my coding agent"
StripComposer->>buildCodingAgentClipboard: build(text)
StripComposer->>Clipboard: writeText(payload)
alt success
StripComposer->>CopiedToast: open toast
else failure
StripComposer->>StripComposer: show error message
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…d agent empty chat (flag-gated) One TemplateStrip component (category tabs, page-by-3 pager, provenance chip, Use-my-coding-agent copy+toast, localStorage hide) behind NEXT_PUBLIC_AGENT_TEMPLATE_STRIP; flag off preserves current behavior byte-for-byte. Includes templateStrip palette family + strip tokens; NOTE: palette.ts/theme-variables.css also carry the colorWarningBg token used by the connect-model-drawer lane (entangled hunks, rides here). Claude-Session: https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw
… AgentComposer coupling Claude-Session: https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw
…doff supersession note Claude-Session: https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw
mmabrouk
changed the title
|
@coderabbitai review |
dosubot
Bot
added
size:XXL
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
web/oss/src/components/AgentChatSlice/AgentChatPanel.tsx (1)
454-470: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate template-pick and clipboard-copy logic vs
StripHome.tsx.
handleStripPickandhandleCodingAgentCopyhere are near-identical (same shape, same analytics call, same clipboard/toast flow) tohandlePick/handleCodingAgentCopyinStripHome.tsx(lines 51-82). Consider extracting a shared hook (e.g.useStripTemplatePick(composerApi, surface)/useCodingAgentCopy(composerRef)) underTemplateStrip/hooksto avoid drift between the two call sites.Also applies to: 471-484
web/oss/src/styles/theme-variables.css (1)
593-593: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDark-only remap creates light/dark value mismatch for
--ag-c-FFFAEB.Dark theme now aliases
--ag-c-FFFAEBtovar(--ag-colorWarningBg)(#2b2111), but the light theme keeps the hardcoded#fffaeb, which is a different hex than light--ag-colorWarningBg(#fffbe6). Any component still using the legacy--ag-c-FFFAEBalias will now see a subtly different tone between themes than before.web/oss/src/components/TemplateStrip/hooks/useStripPager.ts (1)
16-33: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid recreating scroll listener/observer on every
cardCountchange.Since
recomputeis recreated whenevercardCountchanges, the effect at Lines 22-33 (which depends on[recompute]) tears down and recreates the scroll listener andResizeObservereach time the card count changes (e.g., switching category tabs), instead of just recomputing state.♻️ Proposed refactor: split observer setup from recompute trigger
useEffect(() => { const el = scrollerRef.current if (!el) return - recompute() - el.addEventListener("scroll", recompute, {passive: true}) const observer = new ResizeObserver(recompute) observer.observe(el) + el.addEventListener("scroll", recompute, {passive: true}) return () => { el.removeEventListener("scroll", recompute) observer.disconnect() } - }, [recompute]) + }, []) + + useEffect(() => { + recompute() + }, [recompute])web/oss/src/components/TemplateStrip/components/TemplateChip.tsx (1)
39-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCentralize the
aria-labelstring with other strip copy.
aria-label="Remove template"is hardcoded here while other strip text (Line 35) is sourced fromSTRIP_COPY. Consider adding it to the copy constants for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c330ca7b-25cf-41c0-9065-dba73f3e2789
📒 Files selected for processing (33)
docs/design/onboarding-revamp/HANDOFF.mddocs/design/template-strip-onboarding/README.mddocs/design/template-strip-onboarding/context.mddocs/design/template-strip-onboarding/design.mddocs/design/template-strip-onboarding/plan.mddocs/design/template-strip-onboarding/research.mddocs/design/template-strip-onboarding/status.mdweb/oss/src/components/AgentChatSlice/AgentChatPanel.tsxweb/oss/src/components/AgentChatSlice/components/AgentChatEmptyState.tsxweb/oss/src/components/TemplateStrip/assets/codingAgentClipboard.test.tsweb/oss/src/components/TemplateStrip/assets/codingAgentClipboard.tsweb/oss/src/components/TemplateStrip/assets/constants.tsweb/oss/src/components/TemplateStrip/assets/pagerMath.test.tsweb/oss/src/components/TemplateStrip/assets/pagerMath.tsweb/oss/src/components/TemplateStrip/components/CopiedToast.tsxweb/oss/src/components/TemplateStrip/components/IntegrationBadges.tsxweb/oss/src/components/TemplateStrip/components/StripCard.tsxweb/oss/src/components/TemplateStrip/components/StripComposer.tsxweb/oss/src/components/TemplateStrip/components/TemplateChip.tsxweb/oss/src/components/TemplateStrip/hooks/useStripPager.tsweb/oss/src/components/TemplateStrip/hooks/useTemplateProvenance.tsxweb/oss/src/components/TemplateStrip/index.tsxweb/oss/src/components/TemplateStrip/state.tsweb/oss/src/components/pages/agent-home/PlaygroundOnboarding/OnboardingConfigPanel.tsxweb/oss/src/components/pages/agent-home/StripHome.tsxweb/oss/src/components/pages/agent-home/assets/constants.tsweb/oss/src/components/pages/agent-home/components/UsageSummary/index.tsxweb/oss/src/components/pages/agent-home/hooks/useAgentHomeActions.tsweb/oss/src/components/pages/agent-home/index.tsxweb/oss/src/lib/helpers/dynamicEnv.tsweb/oss/src/styles/theme-variables.cssweb/oss/src/styles/theme/palette.tsweb/scripts/generate-tailwind-tokens.ts
| The high-fidelity design lives at `/home/mahmoud/code/agenta/design_handoff_template_strip` | ||
| (interactive HTML prototypes plus written notes). Light mode is final and should be | ||
| recreated pixel-perfectly with AntD + Tailwind. Dark mode needs deliberate token | ||
| translation, covered in `design.md`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a repo-relative handoff reference.
The hard-coded /home/mahmoud/... path is machine-specific and leaks local filesystem details. Please replace it with a repo-relative reference, and update the same reference in the sibling docs too.
| /** Owner-specified key (matches the design prototype; intentionally not "agenta:"-prefixed). */ | ||
| export const STRIP_HIDDEN_STORAGE_KEY = "agenta-tpl-strip-hidden" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Storage key doesn't follow the required agenta: prefix convention.
The comment acknowledges this is intentional, but the guideline is explicit about persisted-state key naming. An unprefixed key increases collision risk with other localStorage consumers and breaks the established convention for identifying app-owned keys.
🏷️ Suggested fix
-export const STRIP_HIDDEN_STORAGE_KEY = "agenta-tpl-strip-hidden"
+export const STRIP_HIDDEN_STORAGE_KEY = "agenta:tpl-strip-hidden"As per coding guidelines, "For persisted state, use atomWithStorage (with stringStorage for nullable strings) and prefix keys with agenta:."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Owner-specified key (matches the design prototype; intentionally not "agenta:"-prefixed). */ | |
| export const STRIP_HIDDEN_STORAGE_KEY = "agenta-tpl-strip-hidden" | |
| /** Owner-specified key (matches the design prototype; intentionally not "agenta:"-prefixed). */ | |
| export const STRIP_HIDDEN_STORAGE_KEY = "agenta:tpl-strip-hidden" |
Source: Coding guidelines
… drop tour icon and Agent-builder eyebrow Under the strip flag, the onboarding config slot no longer overrides to an empty panel — the ephemeral renders the standard config sections like any playground. The 2-min tour button and the NEW/Agent-builder eyebrow are removed from the strip-mode surfaces (legacy flag-off surfaces unchanged). Claude-Session: https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw
…l shift on playground surfaces Onboarding layout is a bottom-anchored cluster (hero top, flexible middle, strip ~12px above the pinned composer — matching the agent-chat surface's rhythm). Template picks on chat surfaces fill the composer only: no provenance chip, no composer border/radius swap, zero layout shift; the strip card's selected state marks the pick. Home keeps its chip. Claude-Session: https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw
…icons, template-named creates, workable builder messages StripHome widens to the app's 960px rhythm (was the prototype's 780). 16px gap between composer and strip. Paperclip removed from the strip composer; Terminal icon on Use-my-coding-agent. Creating from a picked template names the agent after it (home + onboarding). All six templates get explicit 'Create an agent that ...' builder messages (skill-triggering, workable); fallback derivation updated. Also fixes the classic composer passing the click event as the new optional name param. Claude-Session: https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw
…uild mode Strip gate now requires the current revision's version <= 1 (first user-visible commit; hidden v0 seed convention) and build mode — a committed agent's sessions and the maximized chat mode never show the strip. No flash while the revision query is pending. Onboarding surface unaffected. Claude-Session: https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw
…, X clears template text Composer→templates gap joins the page's 30px section rhythm. The provenance chip's slot is always rendered (invisible+inert sizing reference when empty) so picking/clearing a template moves nothing — composer position verified bit-identical. The chip's X now clears the template text along with the chip. Playground surfaces unaffected (they render no chip). Claude-Session: https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw
…for naming, clear on empty, reset on entity switch Agent naming from a template now requires the composer text to exactly match the seeded text at create time (any edit or replacement → default naming). Emptying Home's composer clears the chip (new optional onChange on RichChatInput via CharacterCountPlugin — additive, no consumer changes). Provenance resets when the chat panel's entityId changes. Also: repo-relative docs path. Review-gate blocker from the pre-merge pass. Claude-Session: https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw
Context
Before this change, templates showed up in three different, unrelated ways: a grid-plus-drawer on the agent home page, a "quick pick" list in playground onboarding, and nothing at all in an existing agent's empty chat. Each surface had its own layout, its own click behavior, and its own way (or no way) of getting a template's text into the composer. This PR replaces all three with one shared component,
TemplateStrip, gated behind a flag, so a template card always does the same thing wherever you see it: click it, the composer fills with the template's build instruction, and a chip stays docked to show where the text came from.Changes
web/oss/src/components/TemplateStrip/is the new shared module:assets/pagerMath.ts, unit tested) instead of a scrollable grid or a drawer.TemplateChip+useTemplateProvenance) that stays docked while you edit the filled text and clears itself without wiping what you typed, so you can keep editing after the template gave you a starting point.npx skills add Agenta-AI/agenta-skillsto the clipboard and shows a small custom toast (CopiedToast), replacing the old "Continue in IDE" modal.localStorage, surfaced back as "Templates hidden · show again" so it's reversible.Home gets its own composer:
StripHome.tsxowns aStripComposerinstead of reaching across into the sharedAgentComposer, which is what the second commit (95dc832c4f) removes: the cross-lane coupling that would otherwise have made the home lane depend on playground/chat composer internals.The same strip is wired into playground onboarding (
OnboardingConfigPanel.tsx) and into an agent's empty chat (AgentChatPanel.tsx,AgentChatEmptyState.tsx) — Build mode only, for a freshly-created agent (no revision history yet:versionv0/v1). Switching to maximized Chat mode, or an agent with any revision history, never shows it — the owner call is that the strip is a build-time affordance, not a chat-mode one.Visuals come from a new
templateStrippalette family plus generated--ag-strip-*tokens, defined for both light and dark themes (web/oss/src/styles/theme/palette.ts,theme-variables.css,web/scripts/generate-tailwind-tokens.ts).Everything sits behind
NEXT_PUBLIC_AGENT_TEMPLATE_STRIP(default OFF). With the flag off, the diff was audited hunk-by-hunk to confirm current behavior is preserved byte-for-byte; nothing in the flag-off path changes.Scope / risk
The flag-off path was diff-audited, not just "should be fine". The commit at
0d7c931c9and the follow-up95dc832c4fare the only two commits touching runtime code;219b1208f9is docs only.Shared files touched under the flag:
AgentChatPanel.tsxandAgentChatEmptyState.tsxgain new branches gated on the flag, they aren't new files.palette.ts/theme-variables.cssalso carry thecolorWarningBgtoken, which belongs to the separate connect-model-drawer lane; it rides along in the same hunks as thetemplateStriptokens because both lanes touched the same file at the same time. It isn't part of this feature but couldn't be split out cleanly.Not touched:
web/packages/**(the strip is app-layer only, see D3 below), and the classic (non-strip) home path, which is unchanged and still the default with the flag off.One implementation deviation worth flagging in review: the chat composer only picks up the strip-era border/radius while a chip is docked; an unchipped chat keeps today's composer styling. A full restyle of the shared chat composer for the strip era felt too invasive for this PR. Home always uses the strip-era border since it owns its own composer.
How to QA
Prerequisites: the flag is live on the dev stack at
144.76.237.122:8280already. To QA locally instead, setNEXT_PUBLIC_AGENT_TEMPLATE_STRIP=trueand recreate thewebcontainer (it's a build-time env var, a restart alone won't pick it up).Steps:
npx skills add Agenta-AI/agenta-skillslands on your clipboard and the dark toast appears bottom-center (no modal).Automated tests:
11 tests (pager math + coding-agent clipboard builder), all passing.
Edge cases: the regression this PR is most likely to have caused is in the flag-off path, since several shared files (
AgentChatPanel.tsx,AgentChatEmptyState.tsx,UsageSummary/index.tsx,useAgentHomeActions.ts) now branch on the flag. Also watch the provenance chip across an edit-then-clear cycle, and the dark-mode token choices in D7 below, since the source design was light-only.Live browser QA is pending. Both browser-automation paths were down overnight, so the steps above haven't been walked end-to-end in a browser yet. The flag is live on the dev stack for anyone who wants to click through in the meantime. This PR stays a draft until that click-through passes.
Decisions
Nine open decisions from the design review, all implemented on their recommendation:
templateBuilderMessage(t)(the existing build-instruction field), notseedMessage(a first task, wrong shape) or a new field. Per-templatebuilderMessagecopy tuning to make the phrasing read less "meta" is flagged as an optional content follow-up, not code.TemplateStriplives in the app layer (web/oss/src/components/TemplateStrip/), not@agenta/ui, since both consumers are OSS app-layer and package extraction has no consumer yet to justify it.CopiedToastinstead of the global antdmessage, so the app's global toast styling doesn't have to change to fit this one action.localStoragekey staysagenta-tpl-strip-hiddenas specified in the source design, even though it doesn't carry the repo's usualagenta:prefix.--ag-colorPrimary, badge tiles stay white in dark so brand logos stay legible, and two new palette entries cover hexes with no existing role.localStoragekey; home is never hidden.first_agent_intentanalytics event, since the old "Continue in IDE" modal it replaces fired nothing.One process note for reviewers: Codex at xhigh reasoning implemented only the first slice (S1, the flag + palette family) before its sandbox lost write access; a Sonnet subagent implemented the rest (S2-S6) against the same recorded recommendations.
https://claude.ai/code/session_01FNiAiGuzfi1kkWvXPPcrVw