Redo#1
Merged
Merged
Conversation
…t config, remove deprecated deps
… vars, optional sonner, AbortController, a11y pass
…r/sonner deps, rebuild registry
…onner/menubar deps, clean up all demo files
…mode for readability
…responsive spacing
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR modernizes tooling, swaps Radix primitives for Base UI, rewrites the Terminable component for abortable command processing and title-bar variants, updates registry metadata and public registry files, refreshes demo pages and examples to the new API, and adds developer guidance and a modernization plan. ChangesTerMinable Modernization to Next.js 16 & Base UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components.json (1)
7-7:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove stale Tailwind config reference from components.json.
Line 7 references
tailwind.config.ts, but this file does not exist. The config field in components.json must be updated or removed to prevent shadcn tooling failures during builds or registry operations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components.json` at line 7, components.json currently contains a stale config entry ("config": "tailwind.config.ts") that points to a non-existent file; remove this config field or replace its value with the correct Tailwind config path used in the repo so shadcn tooling won't fail. Locate the "config" key in components.json (the entry shown as "config": "tailwind.config.ts") and either delete that property entirely or update it to the actual Tailwind config filename (e.g., tailwind.config.cjs or tailwind.config.js) used by the project, then save and run the build to confirm the registry/tooling no longer errors.src/components/theme-provider.tsx (1)
14-34: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRemove the dead
classNameprop fromThemeProvider.The provider no longer renders a wrapper that can consume
className, so the public prop now silently does nothing. Either restore a styled wrapper or drop the prop from the signature.Suggested fix
-export function ThemeProvider({ children }: { children: React.ReactNode; className?: string }) { +export function ThemeProvider({ children }: { children: React.ReactNode }) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/theme-provider.tsx` around lines 14 - 34, The ThemeProvider is accepting a dead className prop that is never used; remove className from the component's props and type signature so ThemeProvider({ children }: { children: React.ReactNode }) only — then update any callers/tests that pass className to stop passing it (or restore a wrapper if you intend to accept className); ensure ThemeContext.Provider usage and the exported ThemeProvider signature are updated accordingly to avoid lingering type/usage errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@MODERNIZATION_PLAN.md`:
- Line 44: Clarify that the API expects the literal prop values for
titleBarVariant (e.g., "macos", "windows", "linux", "minimal", "none") while
using proper OS names in prose (e.g., macOS, Windows, GNOME) to prevent readers
from copying incorrect casing; update the MODERNIZATION_PLAN.md entry for
titleBarVariant to explicitly state the allowed literal values and show a
note/example that the human-facing label may use the OS trademark/capitalization
(e.g., titleBarVariant: "macos" — rendered as macOS-style dots).
In `@registry/components/ui/Terminable.tsx`:
- Around line 523-543: handleCommandClick is invoking cmd?.onCopy?.() using
commands[processingStateRef.current.currentIndex], which targets the currently
processing command rather than the clicked one; remove the erroneous onCopy
invocation and rely on copyToClipboard(...).then(() =>
onCopySuccess?.(entry.content as string)) / .catch(() => onCopyError?.(err))
instead, or alternatively, if you prefer preserving per-command handlers, add an
index to DisplayEntry and call the correct commands[index].onCopy() using that
entry index; update handleCommandClick to reference entry (or its stored index)
rather than processingStateRef.current.currentIndex.
In `@src/app/docs/page.tsx`:
- Around line 492-499: The doc text claims "five options" but only shows four
examples; add the missing "minimal" example by inserting another
LazyTerminableExample component with title="Minimal" and
titleBarVariant="minimal" into the example grid (the same place where
LazyTerminableExample with titleBarVariant="macos" is used) so all five variants
are demonstrated; also add the corresponding Minimal entry in the second grid
instance referenced around the later block to keep both sections consistent.
- Around line 735-737: The TSX examples use hardcoded hex colors in inline style
objects (e.g., the span elements rendering "Custom prompt" and "Custom output"
with style={{ color: "`#e0af68`" }} and style={{ color: "`#7dcfff`" }}); replace
these inline hex values with CSS custom properties (e.g., color:
"var(--terminable-<name>)") to match project theming rules—update the span
elements in the shown diff and the other occurrences around the file (lines
referenced 742-746 and 929) to use appropriate vars like
var(--terminable-accent) and var(--terminable-output) (or existing theme
variable names) instead of literal hex codes.
- Around line 773-788: The live "Callbacks" demo's commands array is missing the
onDone, onCopy and onBeforeOutput handlers declared in the code sample; locate
the commands array used to render the Callbacks section (the object literals
with prompt/output entries) and add the missing properties back into the
appropriate command objects (e.g., add onDone: handleDone, onCopy: handleCopy,
onBeforeOutput: handleBeforeOutput or inline functions). Implement small handler
functions referenced by those properties (handleDone to update state/log or mark
completion, handleCopy to perform copy and optionally show toast, and
handleBeforeOutput to return or set the delayed output/placeholder per the
sample) and ensure the component that renders commands receives/uses these
callbacks so the live demo behavior matches the shown code sample.
In `@src/app/page.tsx`:
- Around line 13-15: The three hardcoded hex color state initializations
(TermBackground, TermPromptColor, TermOutputColor) should be replaced with CSS
custom property references (e.g. use "var(--terminable-bg)",
"var(--terminable-prompt)", "var(--terminable-output)") so theming follows the
project contract; update the useState initializers for TermBackground,
setTermBackground usage, TermPromptColor and TermOutputColor to use var(...)
strings instead of hex literals, and also replace any other hardcoded terminal
color occurrences in this file (the other instances you noted) with the
corresponding CSS custom properties.
- Around line 70-78: Remove the nested interactive Button inside the Link
components (the Link and Button in the CTA blocks) and instead render a single
interactive element: either make the Button render as an anchor (use your
Button's "asChild"/component="a" prop or render the anchor <a> styled as a
button) and wrap it with Link or use Link to render the anchor directly, so
there is no button-inside-link nesting; also add rel="noopener noreferrer" to
every external Link that uses target="_blank" (update both CTA occurrences
referencing Link and Button around the Butt3r.dev and the other CTA at the other
occurrence).
In `@src/components/copy-button.tsx`:
- Around line 17-21: The icon-only button in the CopyButton component is missing
accessibility semantics; update the <button> element (used with
onClick={copyToClipboard} and rendering {copied ? <Check .../> : <Copy .../>})
to include type="button" and aria-label="Copy" so it doesn't behave like a
submit in forms and is accessible to screen readers; keep the existing
className, onClick handler, and conditional icon rendering intact.
- Line 19: The className template in the CopyButton component can emit the
literal "undefined" when the prop is omitted; update the CopyButton props so the
className prop has a safe default (e.g., className = "") in the
function/component parameter destructuring (or use your project's class-merge
util like cn or clsx when composing `rounded-md p-2 transition-colors
hover:bg-muted ${className}`) to ensure no "undefined" token gets appended.
In `@src/components/PkgMngCmdCopy.tsx`:
- Around line 62-67: The copy button currently can act as a form submitter and
lacks an accessible name; update the button element rendered alongside the Copy
icon to include type="button" and an accessible label (e.g. aria-label="Copy
command") so keyboard/screen-reader users can identify its purpose and it won’t
submit forms; locate the button that calls handleCopy and renders the <Copy />
icon and add those attributes.
- Around line 21-24: The current approach stores a selectedManager object that
embeds the prop-derived command (from cmd) which can become stale when cmd
changes; change state to store only the selected manager's name (e.g.,
selectedManagerName via setSelectedManagerName) and stop storing command strings
in state, keep the list of manager objects (name + command) derived each render
from the current cmd prop (or compute managers = [{name:"npx", command: "npx " +
cmd}, ...]) and when you need the active command use managers.find(m => m.name
=== selectedManagerName).command; update any usages of selectedManager to use
the derived object or command instead.
In `@src/components/ui/button.tsx`:
- Line 11: The default variant's class string uses an incorrect child-anchor
hover selector "[a]:hover:bg-primary/80" so the button never receives hover
styling; in the button variant definition (the "default" key in the variant
map/const inside src/components/ui/button.tsx) replace the
"[a]:hover:bg-primary/80" fragment with "hover:bg-primary/80" so the button
itself gets the hover style (keep the rest of the class string intact).
In `@src/components/ui/navigation-menu.tsx`:
- Around line 84-87: The Tailwind class string on
NavigationMenuPrimitive.Content (and the similar class on NavigationMenuPopup)
uses invalid data-variant/utility syntax so utilities are being dropped; update
all data-* modifiers to bracketed form (e.g. replace
data-activation-direction=left: with data-[activation-direction=left]: and
data-ending-style: / data-starting-style: with data-[ending-style=...]: and
data-[starting-style=...]:), and change any easing-[ease] style to the proper
ease-[ease] form; apply these exact replacements in the className for
NavigationMenuPrimitive.Content and the corresponding className in the
NavigationMenuPopup component so the popup animation utilities are valid.
In `@src/components/ui/tabs.tsx`:
- Around line 8-22: The Tabs wrapper currently sets data-orientation for styling
but does not pass the orientation prop into TabsPrimitive.Root, which breaks
vertical keyboard navigation; update the Tabs function to forward the
orientation value to TabsPrimitive.Root (i.e., include orientation={orientation}
on the TabsPrimitive.Root element) while keeping existing className,
data-orientation, and {...props} so the primitive receives the correct
orientation and internal behavior matches the visual state.
---
Outside diff comments:
In `@components.json`:
- Line 7: components.json currently contains a stale config entry ("config":
"tailwind.config.ts") that points to a non-existent file; remove this config
field or replace its value with the correct Tailwind config path used in the
repo so shadcn tooling won't fail. Locate the "config" key in components.json
(the entry shown as "config": "tailwind.config.ts") and either delete that
property entirely or update it to the actual Tailwind config filename (e.g.,
tailwind.config.cjs or tailwind.config.js) used by the project, then save and
run the build to confirm the registry/tooling no longer errors.
In `@src/components/theme-provider.tsx`:
- Around line 14-34: The ThemeProvider is accepting a dead className prop that
is never used; remove className from the component's props and type signature so
ThemeProvider({ children }: { children: React.ReactNode }) only — then update
any callers/tests that pass className to stop passing it (or restore a wrapper
if you intend to accept className); ensure ThemeContext.Provider usage and the
exported ThemeProvider signature are updated accordingly to avoid lingering
type/usage errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 543559ef-aaf7-4d74-99ea-111e72f9308f
⛔ Files ignored due to path filters (4)
bun.lockis excluded by!**/*.lockcomm/docs_1.jpegis excluded by!**/*.jpegcomm/docs_2.jpegis excluded by!**/*.jpegcomm/home.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (29)
.eslintrc.cjsAGENTS.mdMODERNIZATION_PLAN.mdcomponents.jsoneslint.config.mjspackage.jsonpostcss.config.jspostcss.config.mjspublic/r/registry.jsonpublic/r/terminable.jsonregistry.jsonregistry/components/ui/Terminable.tsxsrc/app/docs/page.tsxsrc/app/layout.tsxsrc/app/page.tsxsrc/components/PkgMngCmdCopy.tsxsrc/components/TerminableExample.tsxsrc/components/copy-button.tsxsrc/components/theme-provider.tsxsrc/components/ui/button.tsxsrc/components/ui/hover-card.tsxsrc/components/ui/menubar.tsxsrc/components/ui/navigation-menu.tsxsrc/components/ui/scroll-area.tsxsrc/components/ui/sonner.tsxsrc/components/ui/tabs.tsxsrc/styles/globals.csstailwind.config.tstsconfig.json
💤 Files with no reviewable changes (5)
- postcss.config.js
- .eslintrc.cjs
- src/components/ui/menubar.tsx
- tailwind.config.ts
- src/components/ui/sonner.tsx
| | Task | Details | | ||
| |---|---| | ||
| | **Replace Radix Menubar with native elements** | The 3 traffic-light dots become plain `<div>`/`<button>` elements. No more `menubar` registry dependency. | | ||
| | **Add title bar variants** | New `titleBarVariant` prop: `"macos"` (dots), `"windows"` (min/max/close), `"linux"` (GNOME-style), `"minimal"` (just title), `"none"` (no title bar). Each rendered with pure HTML/CSS. | |
There was a problem hiding this comment.
Clarify API literal vs OS label to avoid invalid prop values.
Keep the prop value as "macos" but use the proper OS name (macOS) in prose so readers don’t accidentally copy "macOS" as the literal.
Suggested doc tweak
-| **Add title bar variants** | New `titleBarVariant` prop: `"macos"` (dots), `"windows"` (min/max/close), `"linux"` (GNOME-style), `"minimal"` (just title), `"none"` (no title bar). Each rendered with pure HTML/CSS. |
+| **Add title bar variants** | New `titleBarVariant` prop: `"macos"` (macOS traffic-light dots), `"windows"` (min/max/close), `"linux"` (GNOME-style), `"minimal"` (just title), `"none"` (no title bar). Each rendered with pure HTML/CSS. |📝 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.
Suggested change
| | **Add title bar variants** | New `titleBarVariant` prop: `"macos"` (dots), `"windows"` (min/max/close), `"linux"` (GNOME-style), `"minimal"` (just title), `"none"` (no title bar). Each rendered with pure HTML/CSS. | | |
| | **Add title bar variants** | New `titleBarVariant` prop: `"macos"` (macOS traffic-light dots), `"windows"` (min/max/close), `"linux"` (GNOME-style), `"minimal"` (just title), `"none"` (no title bar). Each rendered with pure HTML/CSS. | |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~44-~44: The operating system from Apple is written “macOS”.
Context: ...ariants** | New titleBarVariant prop: "macos" (dots), "windows" (min/max/close), ...
(MAC_OS)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@MODERNIZATION_PLAN.md` at line 44, Clarify that the API expects the literal
prop values for titleBarVariant (e.g., "macos", "windows", "linux", "minimal",
"none") while using proper OS names in prose (e.g., macOS, Windows, GNOME) to
prevent readers from copying incorrect casing; update the MODERNIZATION_PLAN.md
entry for titleBarVariant to explicitly state the allowed literal values and
show a note/example that the human-facing label may use the OS
trademark/capitalization (e.g., titleBarVariant: "macos" — rendered as
macOS-style dots).
Comment on lines
+492
to
+499
| prop controls the visual style of the title bar. Choose from five | ||
| options: | ||
| </p> | ||
|
|
||
| <div className="grid gap-8 md:grid-cols-2"> | ||
| <LazyTerminableExample | ||
| title="macOS" | ||
| titleBarVariant="macos" |
There was a problem hiding this comment.
The section says “five options” but only four variants are demonstrated.
minimal is missing from the example grid. Add a titleBarVariant="minimal" example or adjust the wording.
Also applies to: 548-563
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/docs/page.tsx` around lines 492 - 499, The doc text claims "five
options" but only shows four examples; add the missing "minimal" example by
inserting another LazyTerminableExample component with title="Minimal" and
titleBarVariant="minimal" into the example grid (the same place where
LazyTerminableExample with titleBarVariant="macos" is used) so all five variants
are demonstrated; also add the corresponding Minimal entry in the second grid
instance referenced around the later block to keep both sections consistent.
Comment on lines
+84
to
+87
| <NavigationMenuPrimitive.Content | ||
| data-slot="navigation-menu-content" | ||
| className={cn( | ||
| "data-ending-style:data-activation-direction=left:translate-x-[50%] data-ending-style:data-activation-direction=right:translate-x-[-50%] data-starting-style:data-activation-direction=left:translate-x-[-50%] data-starting-style:data-activation-direction=right:translate-x-[50%] h-full w-auto p-1 transition-[opacity,transform,translate] duration-[0.35s] ease-[cubic-bezier(0.22,1,0.36,1)] group-data-[viewport=false]/navigation-menu:rounded-lg group-data-[viewport=false]/navigation-menu:bg-popover group-data-[viewport=false]/navigation-menu:text-popover-foreground group-data-[viewport=false]/navigation-menu:shadow group-data-[viewport=false]/navigation-menu:ring-1 group-data-[viewport=false]/navigation-menu:ring-foreground/10 group-data-[viewport=false]/navigation-menu:duration-300 data-ending-style:opacity-0 data-starting-style:opacity-0 data-[motion=from-end]:slide-in-from-right-52 data-[motion=from-start]:slide-in-from-left-52 data-[motion=to-end]:slide-out-to-right-52 data-[motion=to-start]:slide-out-to-left-52 data-[motion^=from-]:animate-in data-[motion^=from-]:fade-in data-[motion^=to-]:animate-out data-[motion^=to-]:fade-out **:data-[slot=navigation-menu-link]:focus:ring-0 **:data-[slot=navigation-menu-link]:focus:outline-none group-data-[viewport=false]/navigation-menu:data-open:animate-in group-data-[viewport=false]/navigation-menu:data-open:fade-in-0 group-data-[viewport=false]/navigation-menu:data-open:zoom-in-95 group-data-[viewport=false]/navigation-menu:data-closed:animate-out group-data-[viewport=false]/navigation-menu:data-closed:fade-out-0 group-data-[viewport=false]/navigation-menu:data-closed:zoom-out-95", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Tailwind CSS v4, are data-activation-direction=left:andeasing-[ease]valid class prefixes/utilities, or should they be written asdata-[activation-direction=left]:andease-[ease]?
💡 Result:
In Tailwind CSS v4, you should use the bracketed syntax for both cases: data-[activation-direction=left]: and ease-[ease]. For data attributes, while Tailwind v4 allows you to use bare variants for simple boolean flags (e.g., data-active:) [1][2], complex data attributes that include values (like data-activation-direction="left") require the arbitrary variant syntax using square brackets: data-[activation-direction=left]: [3][4]. For easing utilities, the correct syntax for an arbitrary value is ease-[...] [5]. Using ease-[ease] would apply the literal CSS value "ease" to the transition-timing-function property [5]. If you intended to use a custom easing function, you would place that specific value inside the brackets, such as ease-[cubic-bezier(...)] [5][6].
Citations:
- 1: https://tailwindcss.com/blog/tailwindcss-v4
- 2: Migrate
aria-*,data-*andsupports-*variants from arbitrary values to bare values tailwindlabs/tailwindcss#14644 - 3: Add dynamic
data-*variant tailwindlabs/tailwindcss#9559 - 4: https://stackoverflow.com/questions/71394904/how-to-target-elements-with-a-specific-data-attribute-with-tailwind-css
- 5: https://tailwindcss.com/docs/transition-timing-function
- 6: https://runebook.dev/en/docs/tailwindcss/transition-timing-function
🏁 Script executed:
cat -n src/components/ui/navigation-menu.tsx | sed -n '80,120p'Repository: DimitriGilbert/TerMinable
Length of output: 3778
Fix invalid Tailwind data-variant and utility syntax.
data-activation-direction=left: requires bracket syntax (data-[activation-direction=left]:), and easing-[ease] should be ease-[ease]. Additionally, data-variant modifiers like data-ending-style: and data-starting-style: should use bracket syntax for consistency in Tailwind v4. These invalid utilities are silently dropped, breaking the popup animation.
Suggested fixes
Lines 84–87 (NavigationMenuContent):
- "data-ending-style:data-activation-direction=left:translate-x-[50%] data-ending-style:data-activation-direction=right:translate-x-[-50%] data-starting-style:data-activation-direction=left:translate-x-[-50%] data-starting-style:data-activation-direction=right:translate-x-[50%] h-full w-auto p-1 transition-[opacity,transform,translate] duration-[0.35s] ease-[cubic-bezier(0.22,1,0.36,1)] group-data-[viewport=false]/navigation-menu:rounded-lg group-data-[viewport=false]/navigation-menu:bg-popover group-data-[viewport=false]/navigation-menu:text-popover-foreground group-data-[viewport=false]/navigation-menu:shadow group-data-[viewport=false]/navigation-menu:ring-1 group-data-[viewport=false]/navigation-menu:ring-foreground/10 group-data-[viewport=false]/navigation-menu:duration-300 data-ending-style:opacity-0 data-starting-style:opacity-0 data-[motion=from-end]:slide-in-from-right-52 data-[motion=from-start]:slide-in-from-left-52 data-[motion=to-end]:slide-out-to-right-52 data-[motion=to-start]:slide-out-to-left-52 data-[motion^=from-]:animate-in data-[motion^=from-]:fade-in data-[motion^=to-]:animate-out data-[motion^=to-]:fade-out **:data-[slot=navigation-menu-link]:focus:ring-0 **:data-[slot=navigation-menu-link]:focus:outline-none group-data-[viewport=false]/navigation-menu:data-open:animate-in group-data-[viewport=false]/navigation-menu:data-open:fade-in-0 group-data-[viewport=false]/navigation-menu:data-open:zoom-in-95 group-data-[viewport=false]/navigation-menu:data-closed:animate-out group-data-[viewport=false]/navigation-menu:data-closed:fade-out-0 group-data-[viewport=false]/navigation-menu:data-closed:zoom-out-95",
+ "data-[ending-style]:data-[activation-direction=left]:translate-x-[50%] data-[ending-style]:data-[activation-direction=right]:translate-x-[-50%] data-[starting-style]:data-[activation-direction=left]:translate-x-[-50%] data-[starting-style]:data-[activation-direction=right]:translate-x-[50%] h-full w-auto p-1 transition-[opacity,transform,translate] duration-[0.35s] ease-[cubic-bezier(0.22,1,0.36,1)] group-data-[viewport=false]/navigation-menu:rounded-lg group-data-[viewport=false]/navigation-menu:bg-popover group-data-[viewport=false]/navigation-menu:text-popover-foreground group-data-[viewport=false]/navigation-menu:shadow group-data-[viewport=false]/navigation-menu:ring-1 group-data-[viewport=false]/navigation-menu:ring-foreground/10 group-data-[viewport=false]/navigation-menu:duration-300 data-[ending-style]:opacity-0 data-[starting-style]:opacity-0 data-[motion=from-end]:slide-in-from-right-52 data-[motion=from-start]:slide-in-from-left-52 data-[motion=to-end]:slide-out-to-right-52 data-[motion=to-start]:slide-out-to-left-52 data-[motion^=from-]:animate-in data-[motion^=from-]:fade-in data-[motion^=to-]:animate-out data-[motion^=to-]:fade-out **:data-[slot=navigation-menu-link]:focus:ring-0 **:data-[slot=navigation-menu-link]:focus:outline-none group-data-[viewport=false]/navigation-menu:data-open:animate-in group-data-[viewport=false]/navigation-menu:data-open:fade-in-0 group-data-[viewport=false]/navigation-menu:data-open:zoom-in-95 group-data-[viewport=false]/navigation-menu:data-closed:animate-out group-data-[viewport=false]/navigation-menu:data-closed:fade-out-0 group-data-[viewport=false]/navigation-menu:data-closed:zoom-out-95",Line 116 (NavigationMenuPopup):
- <NavigationMenuPrimitive.Popup className="data-[ending-style]:easing-[ease] xs:w-(--popup-width) relative h-(--popup-height) w-(--popup-width) origin-(--transform-origin) rounded-lg bg-popover text-popover-foreground shadow ring-1 ring-foreground/10 transition-[opacity,transform,width,height,scale,translate] duration-[0.35s] ease-[cubic-bezier(0.22,1,0.36,1)] outline-none data-ending-style:scale-90 data-ending-style:opacity-0 data-ending-style:duration-150 data-starting-style:scale-90 data-starting-style:opacity-0">
+ <NavigationMenuPrimitive.Popup className="data-[ending-style]:ease-[ease] xs:w-(--popup-width) relative h-(--popup-height) w-(--popup-width) origin-(--transform-origin) rounded-lg bg-popover text-popover-foreground shadow ring-1 ring-foreground/10 transition-[opacity,transform,width,height,scale,translate] duration-[0.35s] ease-[cubic-bezier(0.22,1,0.36,1)] outline-none data-[ending-style]:scale-90 data-[ending-style]:opacity-0 data-[ending-style]:duration-150 data-[starting-style]:scale-90 data-[starting-style]:opacity-0">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/navigation-menu.tsx` around lines 84 - 87, The Tailwind
class string on NavigationMenuPrimitive.Content (and the similar class on
NavigationMenuPopup) uses invalid data-variant/utility syntax so utilities are
being dropped; update all data-* modifiers to bracketed form (e.g. replace
data-activation-direction=left: with data-[activation-direction=left]: and
data-ending-style: / data-starting-style: with data-[ending-style=...]: and
data-[starting-style=...]:), and change any easing-[ease] style to the proper
ease-[ease] form; apply these exact replacements in the className for
NavigationMenuPrimitive.Content and the corresponding className in the
NavigationMenuPopup component so the popup animation utilities are valid.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/docs/page.tsx (1)
50-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
relto the new external links.Both links open a new tab with
target="_blank", but neither setsrel="noreferrer noopener". That leaves the opener relationship intact unnecessarily.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/docs/page.tsx` around lines 50 - 69, The external links created by the GlowLink and Link components open in a new tab but lack rel attributes; update the GlowLink invocation (symbol: GlowLink) and the Link invocation (symbol: Link) to include rel="noreferrer noopener" whenever target="_blank" is used so the opener relationship is removed and security/privacy is improved; ensure both instances in the shown diff are updated (the GlowLink with href "https://ui.shadcn.com/" and the Link with href "https://ui.shadcn.com/docs/installation/nextjs") to include the rel attribute.
♻️ Duplicate comments (1)
src/app/docs/page.tsx (1)
768-784:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the code samples on CSS variables too.
The live examples here were updated to
var(--terminable-...), but the embedded snippets still teach hardcoded hex colors. That leaves the docs internally inconsistent and keeps the theming guidance stale. As per coding guidelines:**/*.{ts,tsx,css}: Use CSS custom properties for theming (e.g. var(--terminable-bg)) instead of hardcoded hex values or inline color classes.Also applies to: 955-983
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/docs/page.tsx` around lines 768 - 784, The embedded code sample passed to the Terminable component (the codeString prop) uses hardcoded hex colors; update those inline style color values to use the project CSS custom properties (e.g. var(--terminable-prompt), var(--terminable-output), var(--terminable-node-1), var(--terminable-node-2) or whatever naming you use) so the snippet matches the live examples and theming guidance. Locate the codeString for the Terminable usage in this file (and the similar block later around the second example) and replace "`#e0af68`", "`#7dcfff`", "`#f7768e`", "`#7aa2f7`" with the appropriate var(...) tokens, keeping the same JSX structure and keys. Ensure both occurrences mentioned in the comment are updated so docs remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@registry/components/ui/Terminable.tsx`:
- Around line 296-300: The loop in Terminable.tsx incorrectly treats
falsy-but-valid outputs (like "" or 0) as missing by using if (!line); change
that check to an explicit null/undefined test (e.g., if (line == null) continue)
wherever you filter outputs (the for...of loop handling outputs and the similar
block around lines 438-447 that checks cmd.output) so empty strings and
zero-valued ReactNodes are preserved.
- Around line 625-653: The span used for copyable commands is made focusable and
keyboard-interactive but lacks an accessible role; update the element in
Terminable.tsx (the span that uses allowCopy, entry.done and entry.content and
handlers handleCommandClick/handleCommandKeyDown) to include role="button" when
it is focusable/clickable (i.e., the same conditional used for
tabIndex/onKeyDown/aria-label), so assistive tech recognizes it as a button;
keep the existing keyboard handler logic (handleCommandKeyDown) and aria-label
as-is.
- Around line 531-537: The code calls cmd?.onCopy?.() before attempting the
async copy, so callers receive a success callback even if copyToClipboard fails;
move the call to cmd?.onCopy?.() into the success branch of the copyToClipboard
promise alongside onCopySuccess, i.e., after
copyToClipboard(entry.content).then(...) invoke both
onCopySuccess(entry.content) and cmd?.onCopy?.(), and do not call
cmd?.onCopy?.() on rejection so failures don't trigger success handlers; locate
the commands/entry usage around the copyToClipboard call to make this change.
In `@src/app/docs/page.tsx`:
- Around line 198-220: The docs currently list literal hex values as prop
defaults for backgroundColor, promptColor, and outputColor which is incorrect
because these are optional overrides in TerminableProps that fall back to
internal CSS variables; update the table rows in page.tsx so the "Default"
column does NOT show the hex strings for the props backgroundColor, promptColor,
and outputColor (replace with an explicit indication like "uses CSS var" or "—"
/ "no default") and adjust the description text to state these props override
the internal CSS variable rather than implying a JS-level default value.
- Around line 837-855: The live demo isn't receiving the color props—only the
codeString was updated—so the preview still uses defaults; update the
LazyTerminableExample invocation to pass backgroundColor, promptColor, and
outputColor props (same values shown in codeString) so the rendered example uses
the custom palette, ensuring the prop names match the
LazyTerminableExample/Terminable API (backgroundColor, promptColor,
outputColor).
---
Outside diff comments:
In `@src/app/docs/page.tsx`:
- Around line 50-69: The external links created by the GlowLink and Link
components open in a new tab but lack rel attributes; update the GlowLink
invocation (symbol: GlowLink) and the Link invocation (symbol: Link) to include
rel="noreferrer noopener" whenever target="_blank" is used so the opener
relationship is removed and security/privacy is improved; ensure both instances
in the shown diff are updated (the GlowLink with href "https://ui.shadcn.com/"
and the Link with href "https://ui.shadcn.com/docs/installation/nextjs") to
include the rel attribute.
---
Duplicate comments:
In `@src/app/docs/page.tsx`:
- Around line 768-784: The embedded code sample passed to the Terminable
component (the codeString prop) uses hardcoded hex colors; update those inline
style color values to use the project CSS custom properties (e.g.
var(--terminable-prompt), var(--terminable-output), var(--terminable-node-1),
var(--terminable-node-2) or whatever naming you use) so the snippet matches the
live examples and theming guidance. Locate the codeString for the Terminable
usage in this file (and the similar block later around the second example) and
replace "`#e0af68`", "`#7dcfff`", "`#f7768e`", "`#7aa2f7`" with the appropriate var(...)
tokens, keeping the same JSX structure and keys. Ensure both occurrences
mentioned in the comment are updated so docs remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: abade382-424a-458a-8c06-23d1897a2758
📒 Files selected for processing (10)
components.jsonpublic/r/terminable.jsonregistry/components/ui/Terminable.tsxsrc/app/docs/page.tsxsrc/app/page.tsxsrc/components/PkgMngCmdCopy.tsxsrc/components/copy-button.tsxsrc/components/theme-provider.tsxsrc/components/ui/button.tsxsrc/components/ui/tabs.tsx
✅ Files skipped from review due to trivial changes (1)
- components.json
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/copy-button.tsx
- src/components/PkgMngCmdCopy.tsx
- src/app/page.tsx
- src/components/ui/button.tsx
- public/r/terminable.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor
Documentation