feat: Add Expanding Action and Fluid Tooltip components - #79
Conversation
- Add `expanding-action` component and demo - Add `fluid-tooltip` component and demo - Add anatomy and promotion agent skills - Update documentation and registry configuration
- Add `AvatarShowcase` for rendering dynamic, recording-friendly follower lists with staggered lanes and deterministic sampling. - Add `Lightbox` for accessible, spatially expanding image previews. - Include registry entries, documentation, and example implementations for both components.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds four UI components, prototype routes for action rails, radial sliders, component anatomy, lightboxes, and avatar data, plus registry integration, documentation, navigation updates, and agent skills. ChangesSona UI expansion
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/content/docs/changelog.mdx (1)
843-843: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRestore the removed
1.1.0release section.This change deletes historical release notes. A changelog is a permanent record. Restore the section, or move it to an explicit archive, instead of removing it.
🤖 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/content/docs/changelog.mdx` at line 843, Restore the historical 1.1.0 release section in the changelog, preserving its original release notes and placement among the versioned sections. Do not remove or rewrite the existing historical content; if the section cannot remain here, move it to an explicit archive while keeping it accessible.
🧹 Nitpick comments (9)
src/components/prototypes/action-rail/shared/action-rail-workspace.tsx (2)
521-545: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the duplicated Archive branches.
Both branches render the same
ActionButtonwith the same icon, label, and click handler. Only thelabelledvalue differs. Use one call, as lines 508-520 already do.♻️ Proposed refactor
- {variant === "labelled" ? ( - <ActionButton - icon={<Archive aria-hidden="true" size={15} />} - label="Archive" - labelled - onClick={() => { - setHoverPreviewSuppressed(true); - setHoveredId(null); - setFocusedId(null); - onArchive(visibleId); - }} - /> - ) : ( - <ActionButton - icon={<Archive aria-hidden="true" size={15} />} - label="Archive" - labelled={false} - onClick={() => { - setHoverPreviewSuppressed(true); - setHoveredId(null); - setFocusedId(null); - onArchive(visibleId); - }} - /> - )} + <ActionButton + icon={<Archive aria-hidden="true" size={15} />} + label="Archive" + labelled={variant === "labelled"} + onClick={() => { + setHoverPreviewSuppressed(true); + setHoveredId(null); + setFocusedId(null); + onArchive(visibleId); + }} + />🤖 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/prototypes/action-rail/shared/action-rail-workspace.tsx` around lines 521 - 545, Collapse the duplicated Archive branches in the action-rail rendering into a single ActionButton, preserving the shared icon, label, and click handler while setting its labelled prop from the variant comparison, consistent with the existing pattern around the preceding action.
484-495: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
role="toolbar"implies arrow-key navigation.The rail declares
role="toolbar", but the three action buttons have no roving tabindex and no arrow-key handler. Keyboard users therefore tab through each button, which contradicts the announced pattern.Two options: implement roving focus with
ArrowLeft/ArrowRight(orArrowUp/ArrowDownfor thedockvariant), or drop thetoolbarrole and keeparia-labelon a plain group.Note that the global
ArrowLeft/ArrowRightshortcut inaction-rail-prototype-picker.tsxwould need to yield to the rail if you add roving focus.🤖 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/prototypes/action-rail/shared/action-rail-workspace.tsx` around lines 484 - 495, Resolve the accessibility mismatch in the action rail rendered by the motion.div with id "prototype-action-rail": either implement roving tabindex and directional arrow-key focus movement for its three action buttons, using horizontal keys or dock-appropriate vertical keys, while ensuring the global picker shortcut yields to rail handling, or remove role="toolbar" and retain the aria-label on a plain group.src/app/prototypes/action-rail/prototype-picker.css (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a lower stacking value.
2147483647is the maximum 32-bit integer. No later overlay, such as a modal or a toast, can stack above the picker without matching this exact value. A value like9999keeps the picker on top of prototype content and leaves headroom.🤖 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/prototypes/action-rail/prototype-picker.css` at line 6, Lower the z-index value in the prototype picker styles from the maximum 32-bit integer to a smaller value such as 9999, keeping the picker above prototype content while leaving room for modals, toasts, and other overlays to stack above it.src/components/prototypes/action-rail/action-rail-prototype-picker.tsx (1)
71-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider scoping the arrow-key shortcuts.
The handler listens on
document.ArrowLeftandArrowRighttherefore change the variant while the user navigates inside the workspace, and thekeychange on<main>then remounts the workspace and drops focus. The workspace also binds its ownkeydownfor Escape and Shift+F10, so the shortcut surfaces overlap.Two low-cost options: require the picker to hold focus for arrow navigation, or ignore events when
event.shiftKeyis set and the focus target is inside<main>.♻️ Optional: ignore modified keys
if ( /^(INPUT|TEXTAREA|SELECT)$/.test(target.tagName) || target.isContentEditable || event.metaKey || event.ctrlKey || - event.altKey + event.altKey || + event.shiftKey ) { return; }🤖 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/prototypes/action-rail/action-rail-prototype-picker.tsx` around lines 71 - 96, Scope the ArrowLeft and ArrowRight handling in the useEffect keydown listener so workspace navigation does not switch variants or drop focus. Either require the picker to own focus before processing arrow shortcuts, or ignore modified-key events and targets contained within the workspace’s main element; preserve the existing number and replay shortcuts..agents/skills/component-anatomy/SKILL.md (1)
118-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSeparate static checks from rendered verification.
The checklist asks the agent to confirm keyboard behavior, pointer behavior, and target attachment. Line 129 says the user owns browser review unless requested. Mark browser-dependent checks as pending when they were not executed. Do not let the completion criterion imply runtime verification without rendered evidence.
🤖 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 @.agents/skills/component-anatomy/SKILL.md around lines 118 - 131, Update the Verify checklist and completion criterion to distinguish static checks from rendered browser verification. In the guidance around “The user owns visual browser review,” require keyboard behavior, pointer interaction, target attachment, and other browser-dependent checks to be reported as pending when not executed, and ensure completion is not described as confirmed without rendered evidence.src/components/prototypes/component-anatomy/fluid-tabs-inspection-board.tsx (2)
54-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive
last-triggerfrom the node list instead of index 2.The index
2is tied to the current three-entrytabsarray. If a tab is added,maxTravelsilently reports the travel to the third tab, not the last tab.♻️ Proposed change
{ id: "last-trigger", - resolve: (root) => root.querySelectorAll('[role="tab"]')[2] ?? null, + resolve: (root) => { + const triggers = root.querySelectorAll('[role="tab"]'); + return triggers[triggers.length - 1] ?? null; + }, },🤖 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/prototypes/component-anatomy/fluid-tabs-inspection-board.tsx` around lines 54 - 57, Update the last-trigger resolve logic to derive the final tab from the queried node list rather than hard-coding index 2. In the last-trigger entry, select the node at the list’s last position so maxTravel continues targeting the actual last tab when the tabs array changes.
266-276: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe transition and padding readouts duplicate values from
fluid-tabs.tsx.
"320 / 40 / 0.9"mirrors the springstiffness: 320, damping: 40, mass: 0.9insrc/registry/sonaui/fluid-tabs/fluid-tabs.tsx."4 px pad"mirrors thep-1onTabs.Listin the same file. Both match today. If the component transition or padding changes, this board reports stale numbers and nothing fails.Consider exporting the spring config from
fluid-tabs.tsxand importing it here, or deriving the padding from the measuredlistandfirst-triggerrects that the board already collects.🤖 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/prototypes/component-anatomy/fluid-tabs-inspection-board.tsx` around lines 266 - 276, Remove the hardcoded transition and padding readouts in the inspection board near the inputMode and formatPixels usages. Reuse the fluid-tabs spring configuration exported from fluid-tabs.tsx for the transition display, and derive the list padding from the existing list and first-trigger measurements instead of the literal “4 px pad”, so these readouts stay synchronized with the component.src/registry/prop-types.ts (1)
967-1004: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
fluid-tooltipprop table documents onlyFluidTooltipGroupProps.
FluidTooltipis a compound component. The table omitsFluidTooltip.Root(id,side,align,sideOffset,disabled),FluidTooltip.Trigger(asChild,keepOpenOnClick), andFluidTooltip.Content(className,showArrow). Theidprop onRootis required, and the demo depends on it, so consumers need it in the reference.Add the remaining part props to this entry. Prefix each name with its part, for example
Root.id, to keep the flatPropMetashape.🤖 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/registry/prop-types.ts` around lines 967 - 1004, Expand the “fluid-tooltip” entry in the prop-types registry to include the compound parts omitted from the current FluidTooltipGroupProps table: Root.id (required), Root.side, Root.align, Root.sideOffset, Root.disabled, Trigger.asChild, Trigger.keepOpenOnClick, Content.className, and Content.showArrow. Prefix each property name with its part name, such as Root.id, while preserving the existing group props and flat PropMeta structure.src/lib/github-followers.ts (1)
119-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for pagination, limit handling, and error paths.
getGitHubFollowersimplements pagination via Link-header parsing, alimit/"all"distinction, and two error paths (GitHubFollowersErrorfrom the profile call and from paginated calls). None of this logic has visible test coverage in the reviewed files. Thefetcheroption is designed for injection, which makes this straightforward to unit test with a mock implementation.Add tests for: limit smaller than a page, limit spanning multiple pages,
limit: "all", a non-OK profile response, and a non-OK paginated response.🤖 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/lib/github-followers.ts` around lines 119 - 206, Add unit tests for getGitHubFollowers using an injected fetcher mock, covering a limit smaller than one page, a limit spanning multiple pages, limit: "all", a non-OK profile response, and a non-OK paginated response. Assert pagination requests and returned followers/hasMore values, and verify both failures throw GitHubFollowersError with the expected status.
🤖 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 @.agents/skills/promote-portfolio-component/SKILL.md:
- Around line 10-14: Constrain repository discovery in both
.agents/skills/promote-portfolio-component/SKILL.md (lines 10-14) and
.agents/skills/component-anatomy/SKILL.md (lines 10-12): validate and reject
supplied paths under protected locations, scope the initial rg --files search to
the supplied path, and exclude personal/journal/** from all broader searches
while locating components, demos, and local dependencies.
In `@src/app/prototypes/avatar-showcase/page.tsx`:
- Around line 4-13: Enable caching for the static GitHub query in
AvatarShowcasePrototypePage by removing the force-dynamic behavior or
configuring an appropriate route revalidate window, and pass the corresponding
cache/revalidation options through getGitHubFollowers. Preserve the existing
username and limit, and ensure production uses GITHUB_TOKEN for the higher
GitHub API quota.
- Around line 34-42: Update the catch block in the avatar showcase page to
capture the thrown error and log it before returning the existing fallback UI.
Preserve the current GitHub followers error handling and distinguish
GitHubFollowersError from unexpected errors when choosing the logging context.
In `@src/components/prototypes/action-rail/shared/action-rail-workspace.tsx`:
- Around line 170-180: Update the archive function so that when projects.length
=== 1, it sets an explanatory message before returning. Preserve the existing
no-op behavior for the final project and the current archive flow for other
valid targets.
In `@src/components/prototypes/component-anatomy/inspection-board.tsx`:
- Around line 101-127: Replace the unconditional requestAnimationFrame recursion
in the useLayoutEffect with a bounded measurement loop: start tracking only
during pointer/keyboard interaction or active-tab changes, stop after rects
remain stable for several frames, and pause while document.hidden. Preserve
ResizeObserver, font-readiness, and window resize measurement behavior, and
ensure cleanup cancels any pending frame and listeners.
In `@src/components/prototypes/lightbox/spatial-expand-lightbox.tsx`:
- Around line 49-63: Update the motion.img element in the lightbox trigger to
use an empty alt value, keeping the existing sr-only span as the sole accessible
name source with the image description.
- Around line 66-86: Update the lightbox render flow around Dialog.Portal to
keep the portal mounted independently of the open state, while preserving the
dialog’s close behavior and allowing exit animations to complete before any
React-level unmount. Remove the open-based conditional wrapping Dialog.Portal
and rely on the dialog’s state/animation lifecycle for visibility.
In `@src/components/prototypes/radial-slider/radial-slider-prototype-picker.tsx`:
- Around line 59-84: Update the global keydown handler in the useEffect to
ignore ArrowLeft and ArrowRight events originating from the focused radial
slider, identified by the target’s role="slider", after allowing the slider’s
own handler to process them. Preserve existing shortcuts for other targets and
keep numeric, replay, and non-slider variant switching behavior unchanged.
In `@src/content/docs/avatar-showcase.mdx`:
- Around line 1-39: Remove the disabled Avatar Showcase documentation page,
including its usage content and component references. Delete avatar-showcase.mdx
while the navigation entry remains disabled, and do not modify unrelated
registry or playground code.
In `@src/content/docs/expanding-action.mdx`:
- Around line 11-18: Update the Features documentation for ExpandingAction to
remove or revise the claim about accessible focus management so it accurately
reflects the implemented behavior. Do not imply that focus is restored or
managed unless the component implementation is changed to provide that behavior.
In `@src/lib/github-followers.ts`:
- Around line 119-206: Update getGitHubFollowers so both the profile request and
each pagination request enforce an internal timeout even when no caller-supplied
signal is provided. Combine the caller’s signal with the timeout signal without
overriding caller cancellation, and ensure timeout resources are cleaned up
after each fetch; alternatively use supported fetch timeout options consistently
for both requests.
In `@src/registry/index.ts`:
- Line 2859: Update the registry extraction prologue for expanding-action so it
skips the "use client"; directive before locating the first import statement.
Preserve the generated imports in the registry’s imports section rather than
moving them into anatomy, ensuring Usage imports are populated.
In `@src/registry/playground/index.tsx`:
- Around line 134-141: Update the playgroundAvatars data to reference locally
bundled avatar assets under public/ instead of the external i.pravatar.cc URLs.
Preserve the 24-item generation and existing identifiers/names, ensuring each
imageUrl resolves to a valid local asset so the showcase remains visible without
network access.
In `@src/registry/registry.json`:
- Around line 220-235: Add "registryDependencies": ["`@sona-ui/sona-utils`"] to
the fluid-tooltip manifest entry and the lightbox and avatar-showcase entries in
src/registry/registry.json. Then regenerate src/registry/index.ts so the
corresponding generated blocks include the same registryDependencies metadata.
In `@src/registry/sonaui/avatar-showcase/avatar-showcase.tsx`:
- Around line 213-221: Update the shouldReduceMotion branch in the avatar
showcase component to invoke onComplete once from an effect when reduced motion
is active, while preserving the staticItems rendering and avoiding repeated
calls on rerenders.
- Around line 154-179: Update the effect containing the ResizeObserver and
measure callback to depend on the measurement values as well as measurementKey,
so it re-observes the remounted track. Guard setMeasurement by comparing the
next container and track dimensions with the current measurement, preventing
unchanged measurements from triggering a render loop while preserving the
existing zero-width handling.
In `@src/registry/sonaui/expanding-action/expanding-action.tsx`:
- Around line 95-190: Update the expanding-action component around the trigger,
choices row, and AnimatePresence to manage keyboard focus: retain refs for the
trigger and Back button, focus the Back button when isOpen becomes true, and
restore focus to the trigger when it becomes false. Add aria-expanded to the
trigger and handle Escape while open by collapsing the control and restoring
focus to the trigger, while preserving existing disabled and selection behavior.
In `@src/registry/sonaui/fluid-tooltip/fluid-tooltip.tsx`:
- Around line 390-402: Move tooltip content, className, and showArrow storage
from refs to state in FluidTooltipRoot, and expose the state through the payload
consumed by FluidTooltipGroup. Update FluidTooltipContent to propagate changes
via the root’s state update mechanism so open tooltips re-render with current
values, and remove render-time reads of the mutable content refs.
- Around line 348-361: Update handleFocus so it calls
group.registerKeyboardTarget() only when
event.currentTarget.matches(":focus-visible"), while still invoking onFocus for
every focus event; leave handlePointerEnter and handleKeyDown behavior
unchanged.
In `@src/registry/sonaui/lightbox/lightbox.tsx`:
- Line 65: Update the Dialog.Root invocation to remove the redundant defaultOpen
prop, keeping open={open} and onOpenChange={setOpen} as the sole state controls.
Preserve the existing initialization of internalOpen from defaultOpen at line
49.
---
Outside diff comments:
In `@src/content/docs/changelog.mdx`:
- Line 843: Restore the historical 1.1.0 release section in the changelog,
preserving its original release notes and placement among the versioned
sections. Do not remove or rewrite the existing historical content; if the
section cannot remain here, move it to an explicit archive while keeping it
accessible.
---
Nitpick comments:
In @.agents/skills/component-anatomy/SKILL.md:
- Around line 118-131: Update the Verify checklist and completion criterion to
distinguish static checks from rendered browser verification. In the guidance
around “The user owns visual browser review,” require keyboard behavior, pointer
interaction, target attachment, and other browser-dependent checks to be
reported as pending when not executed, and ensure completion is not described as
confirmed without rendered evidence.
In `@src/app/prototypes/action-rail/prototype-picker.css`:
- Line 6: Lower the z-index value in the prototype picker styles from the
maximum 32-bit integer to a smaller value such as 9999, keeping the picker above
prototype content while leaving room for modals, toasts, and other overlays to
stack above it.
In `@src/components/prototypes/action-rail/action-rail-prototype-picker.tsx`:
- Around line 71-96: Scope the ArrowLeft and ArrowRight handling in the
useEffect keydown listener so workspace navigation does not switch variants or
drop focus. Either require the picker to own focus before processing arrow
shortcuts, or ignore modified-key events and targets contained within the
workspace’s main element; preserve the existing number and replay shortcuts.
In `@src/components/prototypes/action-rail/shared/action-rail-workspace.tsx`:
- Around line 521-545: Collapse the duplicated Archive branches in the
action-rail rendering into a single ActionButton, preserving the shared icon,
label, and click handler while setting its labelled prop from the variant
comparison, consistent with the existing pattern around the preceding action.
- Around line 484-495: Resolve the accessibility mismatch in the action rail
rendered by the motion.div with id "prototype-action-rail": either implement
roving tabindex and directional arrow-key focus movement for its three action
buttons, using horizontal keys or dock-appropriate vertical keys, while ensuring
the global picker shortcut yields to rail handling, or remove role="toolbar" and
retain the aria-label on a plain group.
In `@src/components/prototypes/component-anatomy/fluid-tabs-inspection-board.tsx`:
- Around line 54-57: Update the last-trigger resolve logic to derive the final
tab from the queried node list rather than hard-coding index 2. In the
last-trigger entry, select the node at the list’s last position so maxTravel
continues targeting the actual last tab when the tabs array changes.
- Around line 266-276: Remove the hardcoded transition and padding readouts in
the inspection board near the inputMode and formatPixels usages. Reuse the
fluid-tabs spring configuration exported from fluid-tabs.tsx for the transition
display, and derive the list padding from the existing list and first-trigger
measurements instead of the literal “4 px pad”, so these readouts stay
synchronized with the component.
In `@src/lib/github-followers.ts`:
- Around line 119-206: Add unit tests for getGitHubFollowers using an injected
fetcher mock, covering a limit smaller than one page, a limit spanning multiple
pages, limit: "all", a non-OK profile response, and a non-OK paginated response.
Assert pagination requests and returned followers/hasMore values, and verify
both failures throw GitHubFollowersError with the expected status.
In `@src/registry/prop-types.ts`:
- Around line 967-1004: Expand the “fluid-tooltip” entry in the prop-types
registry to include the compound parts omitted from the current
FluidTooltipGroupProps table: Root.id (required), Root.side, Root.align,
Root.sideOffset, Root.disabled, Trigger.asChild, Trigger.keepOpenOnClick,
Content.className, and Content.showArrow. Prefix each property name with its
part name, such as Root.id, while preserving the existing group props and flat
PropMeta structure.
🪄 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 Plus
Run ID: 68f3fd95-38e1-4bed-9dea-cfe9446afde6
📒 Files selected for processing (61)
.agents/skills/component-anatomy/SKILL.md.agents/skills/component-anatomy/agents/openai.yaml.agents/skills/promote-portfolio-component/SKILL.md.agents/skills/promote-portfolio-component/agents/openai.yaml.agents/skills/writing-great-skills/GLOSSARY.md.agents/skills/writing-great-skills/SKILL.md.agents/skills/writing-great-skills/agents/openai.yaml.claude/skills/writing-great-skillspublic/agent/catalog.jsonpublic/agent/components/hold-to-delete-button.jsonpublic/r/avatar-showcase.jsonpublic/r/expanding-action.jsonpublic/r/fluid-tooltip.jsonpublic/r/lightbox.jsonpublic/r/registry.jsonregistry.jsonscripts/build-registry.tsskills-lock.jsonsrc/app/prototypes/action-rail/page.tsxsrc/app/prototypes/action-rail/prototype-picker.csssrc/app/prototypes/avatar-showcase/page.tsxsrc/app/prototypes/fluid-tabs-anatomy/page.tsxsrc/app/prototypes/radial-slider/page.tsxsrc/app/prototypes/radial-slider/prototype-picker.csssrc/components/common/component-installation.tsxsrc/components/common/sidebar-link.tsxsrc/components/component-sidebar/index.tsxsrc/components/prototypes/action-rail/action-rail-prototype-picker.tsxsrc/components/prototypes/action-rail/edge-dock-prototype.tsxsrc/components/prototypes/action-rail/floating-tools-prototype.tsxsrc/components/prototypes/action-rail/labelled-bar-prototype.tsxsrc/components/prototypes/action-rail/shared/action-rail-workspace.tsxsrc/components/prototypes/component-anatomy/fluid-tabs-inspection-board.tsxsrc/components/prototypes/component-anatomy/inspection-board.tsxsrc/components/prototypes/lightbox/spatial-expand-lightbox.tsxsrc/components/prototypes/radial-slider/instrument-cluster-prototype.tsxsrc/components/prototypes/radial-slider/mechanical-detents-prototype.tsxsrc/components/prototypes/radial-slider/quiet-gauge-prototype.tsxsrc/components/prototypes/radial-slider/radial-slider-prototype-picker.tsxsrc/components/prototypes/radial-slider/use-radial-slider.tssrc/config/components.tssrc/content/docs/avatar-showcase.mdxsrc/content/docs/changelog.mdxsrc/content/docs/expanding-action.mdxsrc/content/docs/fluid-tooltip.mdxsrc/content/docs/lightbox.mdxsrc/lib/github-followers.tssrc/lib/types.tssrc/registry/examples/avatar-showcase/avatar-showcase-demo.tsxsrc/registry/examples/button/button-demo.tsxsrc/registry/examples/expanding-action/expanding-action-demo.tsxsrc/registry/examples/fluid-tooltip/fluid-tooltip-demo.tsxsrc/registry/examples/lightbox/lightbox-demo.tsxsrc/registry/index.tssrc/registry/playground/index.tsxsrc/registry/prop-types.tssrc/registry/registry.jsonsrc/registry/sonaui/avatar-showcase/avatar-showcase.tsxsrc/registry/sonaui/expanding-action/expanding-action.tsxsrc/registry/sonaui/fluid-tooltip/fluid-tooltip.tsxsrc/registry/sonaui/lightbox/lightbox.tsx
💤 Files with no reviewable changes (1)
- src/registry/examples/button/button-demo.tsx
| ## Required input | ||
|
|
||
| Require a path to the source component, demo, or Craft. Resolve the real files with `rg --files` before forming a plan. Ask only for information that cannot be discovered from the supplied path and its local imports. | ||
|
|
||
| Do not assume that a visually finished experiment is library-ready. Do not modify or remove the portfolio source during promotion unless the user explicitly asks for that separate change. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Constrain repository discovery before scanning the repository.
Both workflows leave the search root and protected-path exclusion implicit. A repository-root scan can enumerate personal/journal/, which the promotion workflow explicitly forbids.
.agents/skills/promote-portfolio-component/SKILL.md#L10-L14: reject protected paths, scope the initial search to the supplied path, and excludepersonal/journal/**from broader searches..agents/skills/component-anatomy/SKILL.md#L10-L12: apply the same scope and exclusion before locating components, demos, and local dependencies.
🧰 Tools
🪛 LanguageTool
[style] ~10-~10: The double modal “Required input” is nonstandard (only accepted in certain dialects). Consider “to be input”.
Context: ...usable distribution layer. ## Required input Require a path to the source component...
(NEEDS_FIXED)
🪛 SkillSpector (2.4.4)
[warning] 155: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
📍 Affects 2 files
.agents/skills/promote-portfolio-component/SKILL.md#L10-L14(this comment).agents/skills/component-anatomy/SKILL.md#L10-L12
🤖 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 @.agents/skills/promote-portfolio-component/SKILL.md around lines 10 - 14,
Constrain repository discovery in both
.agents/skills/promote-portfolio-component/SKILL.md (lines 10-14) and
.agents/skills/component-anatomy/SKILL.md (lines 10-12): validate and reject
supplied paths under protected locations, scope the initial rg --files search to
the supplied path, and exclude personal/journal/** from all broader searches
while locating components, demos, and local dependencies.
| export const dynamic = "force-dynamic"; | ||
|
|
||
| const githubUsername = "Dinil-Thilakarathne"; | ||
|
|
||
| export default async function AvatarShowcasePrototypePage() { | ||
| try { | ||
| const result = await getGitHubFollowers({ | ||
| username: githubUsername, | ||
| limit: 320, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Route re-fetches the same GitHub data on every request with no caching, risking rate-limit exhaustion.
dynamic = "force-dynamic" (Line 4) disables static caching for this route, and getGitHubFollowers (defined in src/lib/github-followers.ts) defaults its fetch cache option to "no-store". The query parameters here are constant (username: "Dinil-Thilakarathne", limit: 320), so every page load re-issues 1 profile request plus 4 pagination requests (320 followers ÷ 100 per page) against the GitHub REST API. GitHub's unauthenticated primary rate limit is 60 requests per hour per IP address, so roughly 12 page loads per hour from a single IP will exhaust it (5,000/hour if GITHUB_TOKEN is set). Once exhausted, the page falls into the generic error state for all visitors sharing that IP or token until the limit resets.
Since the query is static, this data is a strong caching candidate. Enable caching (for example, pass cache: "force-cache" with a next.revalidate window, or drop force-dynamic in favor of a route revalidate export) and ensure GITHUB_TOKEN is configured in production to raise the quota.
🤖 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/prototypes/avatar-showcase/page.tsx` around lines 4 - 13, Enable
caching for the static GitHub query in AvatarShowcasePrototypePage by removing
the force-dynamic behavior or configuring an appropriate route revalidate
window, and pass the corresponding cache/revalidation options through
getGitHubFollowers. Preserve the existing username and limit, and ensure
production uses GITHUB_TOKEN for the higher GitHub API quota.
| } catch { | ||
| return ( | ||
| <main className="grid min-h-screen place-items-center bg-background px-6 py-12"> | ||
| <p className="text-muted-foreground text-sm"> | ||
| Unable to load GitHub followers right now. | ||
| </p> | ||
| </main> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Log the error before returning the fallback UI.
The catch {} block (Lines 34-42) discards the error entirely. It does not distinguish a GitHubFollowersError (network/auth/rate-limit failure) from a programming error, and nothing is logged. This makes production issues silent and hard to diagnose.
🔧 Proposed fix
- } catch {
+ } catch (error) {
+ console.error("Failed to load GitHub followers for avatar showcase", error);
return (📝 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.
| } catch { | |
| return ( | |
| <main className="grid min-h-screen place-items-center bg-background px-6 py-12"> | |
| <p className="text-muted-foreground text-sm"> | |
| Unable to load GitHub followers right now. | |
| </p> | |
| </main> | |
| ); | |
| } | |
| } catch (error) { | |
| console.error("Failed to load GitHub followers for avatar showcase", error); | |
| return ( | |
| <main className="grid min-h-screen place-items-center bg-background px-6 py-12"> | |
| <p className="text-muted-foreground text-sm"> | |
| Unable to load GitHub followers right now. | |
| </p> | |
| </main> | |
| ); | |
| } |
🤖 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/prototypes/avatar-showcase/page.tsx` around lines 34 - 42, Update the
catch block in the avatar showcase page to capture the thrown error and log it
before returning the existing fallback UI. Preserve the current GitHub followers
error handling and distinguish GitHubFollowersError from unexpected errors when
choosing the logging context.
| const archive = (id: string) => { | ||
| const target = projects.find((project) => project.id === id); | ||
| if (!target || projects.length === 1) return; | ||
| const index = projects.findIndex((project) => project.id === target.id); | ||
| setArchived({ project: target, index }); | ||
| setProjects((current) => | ||
| current.filter((project) => project.id !== target.id), | ||
| ); | ||
| setActiveId((current) => (current === target.id ? null : current)); | ||
| setMessage(`Archived ${target.name}`); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Give feedback when archive is blocked.
archive returns early when only one project remains. No state changes and message is not updated. The Archive button then appears unresponsive.
Set an explanatory message before the early return.
🐛 Proposed fix
const archive = (id: string) => {
const target = projects.find((project) => project.id === id);
- if (!target || projects.length === 1) return;
+ if (!target) return;
+ if (projects.length === 1) {
+ setMessage("Keep at least one project");
+ return;
+ }📝 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.
| const archive = (id: string) => { | |
| const target = projects.find((project) => project.id === id); | |
| if (!target || projects.length === 1) return; | |
| const index = projects.findIndex((project) => project.id === target.id); | |
| setArchived({ project: target, index }); | |
| setProjects((current) => | |
| current.filter((project) => project.id !== target.id), | |
| ); | |
| setActiveId((current) => (current === target.id ? null : current)); | |
| setMessage(`Archived ${target.name}`); | |
| }; | |
| const archive = (id: string) => { | |
| const target = projects.find((project) => project.id === id); | |
| if (!target) return; | |
| if (projects.length === 1) { | |
| setMessage("Keep at least one project"); | |
| return; | |
| } | |
| const index = projects.findIndex((project) => project.id === target.id); | |
| setArchived({ project: target, index }); | |
| setProjects((current) => | |
| current.filter((project) => project.id !== target.id), | |
| ); | |
| setActiveId((current) => (current === target.id ? null : current)); | |
| setMessage(`Archived ${target.name}`); | |
| }; |
🤖 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/prototypes/action-rail/shared/action-rail-workspace.tsx`
around lines 170 - 180, Update the archive function so that when projects.length
=== 1, it sets an explanatory message before returning. Preserve the existing
no-op behavior for the final project and the current archive flow for other
valid targets.
| useLayoutEffect(() => { | ||
| const root = rootRef.current; | ||
| if (!root) return; | ||
|
|
||
| let frame = 0; | ||
| const track = () => { | ||
| measure(); | ||
| frame = requestAnimationFrame(track); | ||
| }; | ||
| frame = requestAnimationFrame(track); | ||
|
|
||
| const observer = new ResizeObserver(measure); | ||
| observer.observe(root); | ||
| for (const target of targets) { | ||
| const element = target.resolve(root); | ||
| if (element instanceof HTMLElement) observer.observe(element); | ||
| } | ||
|
|
||
| document.fonts.ready.then(measure); | ||
| window.addEventListener("resize", measure); | ||
|
|
||
| return () => { | ||
| cancelAnimationFrame(frame); | ||
| observer.disconnect(); | ||
| window.removeEventListener("resize", measure); | ||
| }; | ||
| }, [measure, rootRef, targets]); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
The requestAnimationFrame loop measures every frame for the page lifetime.
track reschedules itself unconditionally. Each frame calls measure, which runs getBoundingClientRect on the root and on every target. This forces layout on every frame and keeps the CPU busy even when nothing moves. The ResizeObserver, document.fonts.ready, and resize handlers already cover static geometry changes, so the loop is only needed while the Motion layout animation runs.
Consider gating the loop: start it on pointer/keyboard interaction and stop it after the rects stay stable for a few frames. Also stop it when the document is hidden.
♻️ Sketch: stop the loop once rects settle
let frame = 0;
+ let stableFrames = 0;
+ let previous = "";
const track = () => {
measure();
- frame = requestAnimationFrame(track);
+ const snapshot = JSON.stringify(rectsSnapshot());
+ stableFrames = snapshot === previous ? stableFrames + 1 : 0;
+ previous = snapshot;
+ if (stableFrames < 30) frame = requestAnimationFrame(track);
};
frame = requestAnimationFrame(track);A simpler alternative is to restart a short-lived loop from the board component whenever the active tab changes.
🤖 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/prototypes/component-anatomy/inspection-board.tsx` around
lines 101 - 127, Replace the unconditional requestAnimationFrame recursion in
the useLayoutEffect with a bounded measurement loop: start tracking only during
pointer/keyboard interaction or active-tab changes, stop after rects remain
stable for several frames, and pause while document.hidden. Preserve
ResizeObserver, font-readiness, and window resize measurement behavior, and
ensure cleanup cancels any pending frame and listeners.
| {shouldReduceMotion ? ( | ||
| <div | ||
| className="flex items-center justify-center -space-x-3" | ||
| aria-hidden="true" | ||
| > | ||
| {staticItems.map((item) => ( | ||
| <ShowcaseAvatar item={item} key={item.id} size={safeAvatarSize} /> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Call onComplete in the reduced-motion branch.
When shouldReduceMotion is true the component renders the static stack and never runs onAnimationComplete. onComplete then never fires, so a consumer that gates the next step on the callback stalls. Call onComplete once from an effect when reduced motion is active.
🤖 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/registry/sonaui/avatar-showcase/avatar-showcase.tsx` around lines 213 -
221, Update the shouldReduceMotion branch in the avatar showcase component to
invoke onComplete once from an effect when reduced motion is active, while
preserving the staticItems rendering and avoiding repeated calls on rerenders.
| return ( | ||
| <MotionConfig reducedMotion="user"> | ||
| <LayoutGroup id={`${instanceId}-expanding-action`}> | ||
| <AnimatePresence initial={false} mode="popLayout"> | ||
| {!isOpen ? ( | ||
| <motion.button | ||
| key="trigger" | ||
| type="button" | ||
| disabled={disabled || !hasEnabledItem} | ||
| onClick={() => setOpen(true)} | ||
| className={cn( | ||
| "relative flex h-12 cursor-pointer items-center gap-2 rounded-full px-5 text-sm font-medium text-(--expanding-action-foreground) active:scale-[0.97] disabled:pointer-events-none disabled:opacity-45", | ||
| "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-(--expanding-action-ring)", | ||
| className, | ||
| triggerClassName, | ||
| )} | ||
| style={tokenStyle} | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| transition={contentTransition} | ||
| > | ||
| <motion.span | ||
| layoutId={`${instanceId}-surface`} | ||
| aria-hidden="true" | ||
| className="absolute inset-0 rounded-full border border-(--expanding-action-border) bg-(--expanding-action-surface)/70 shadow-sm" | ||
| transition={surfaceTransition} | ||
| /> | ||
| {triggerIcon ? ( | ||
| <span | ||
| aria-hidden="true" | ||
| className="relative grid size-4 shrink-0 place-items-center" | ||
| > | ||
| {triggerIcon} | ||
| </span> | ||
| ) : null} | ||
| <span className="relative whitespace-nowrap">{trigger}</span> | ||
| </motion.button> | ||
| ) : ( | ||
| <motion.div | ||
| key="choices" | ||
| className={cn( | ||
| "relative flex max-w-full items-center overflow-x-auto rounded-full p-1", | ||
| className, | ||
| )} | ||
| style={tokenStyle} | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| transition={contentTransition} | ||
| > | ||
| <motion.span | ||
| layoutId={`${instanceId}-surface`} | ||
| aria-hidden="true" | ||
| className="absolute inset-0 rounded-full border border-(--expanding-action-border) bg-(--expanding-action-surface)/70 shadow-sm" | ||
| transition={surfaceTransition} | ||
| /> | ||
| <div className="relative flex items-center gap-1"> | ||
| <button | ||
| type="button" | ||
| disabled={disabled} | ||
| onClick={() => setOpen(false)} | ||
| aria-label={backLabel} | ||
| className="grid size-10 shrink-0 cursor-pointer place-items-center rounded-full text-(--expanding-action-muted) transition-colors hover:bg-(--expanding-action-hover) hover:text-(--expanding-action-foreground) focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-(--expanding-action-ring) active:scale-[0.97] disabled:pointer-events-none disabled:opacity-45" | ||
| > | ||
| <ChevronLeft | ||
| aria-hidden="true" | ||
| className="size-4" | ||
| strokeWidth={1.75} | ||
| /> | ||
| </button> | ||
| <span | ||
| aria-hidden="true" | ||
| className="h-5 w-px shrink-0 bg-(--expanding-action-border)" | ||
| /> | ||
| {items.map((item) => ( | ||
| <button | ||
| key={item.value} | ||
| type="button" | ||
| disabled={disabled || item.disabled} | ||
| onClick={() => { | ||
| onValueSelect?.(item.value); | ||
| setOpen(false); | ||
| }} | ||
| className={cn( | ||
| "h-10 shrink-0 cursor-pointer whitespace-nowrap rounded-full px-3 text-sm text-(--expanding-action-foreground) transition-colors hover:bg-(--expanding-action-hover) focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-(--expanding-action-ring) active:scale-[0.97] disabled:pointer-events-none disabled:opacity-45", | ||
| optionClassName, | ||
| )} | ||
| > | ||
| {item.label} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| </motion.div> | ||
| )} | ||
| </AnimatePresence> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Restore focus when the control expands and collapses.
AnimatePresence removes the trigger button when isOpen becomes true, and removes the choices row when the user selects an item or presses Back. In both cases the focused element leaves the DOM, so focus returns to document.body. A keyboard user loses their position and must tab from the start of the page. The registry description promises accessible focus management.
Move focus to the Back button when the control expands. Move focus back to the trigger when it collapses. Add aria-expanded to the trigger and collapse on Escape.
♿ Proposed fix outline
+ const triggerRef = useRef<HTMLButtonElement>(null);
+ const backRef = useRef<HTMLButtonElement>(null);
+ const shouldRestoreFocus = useRef(false);
+
+ useEffect(() => {
+ if (!shouldRestoreFocus.current) return;
+ if (isOpen) backRef.current?.focus();
+ else triggerRef.current?.focus();
+ }, [isOpen]);
+
const setOpen = (nextOpen: boolean) => {
+ shouldRestoreFocus.current = true;
if (open === undefined) setInternalOpen(nextOpen);
onOpenChange?.(nextOpen);
}; <motion.button
key="trigger"
type="button"
+ ref={triggerRef}
+ aria-expanded={isOpen}
disabled={disabled || !hasEnabledItem} <motion.div
key="choices"
+ onKeyDown={(event) => {
+ if (event.key === "Escape") setOpen(false);
+ }} <button
type="button"
+ ref={backRef}
disabled={disabled}
onClick={() => setOpen(false)}🤖 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/registry/sonaui/expanding-action/expanding-action.tsx` around lines 95 -
190, Update the expanding-action component around the trigger, choices row, and
AnimatePresence to manage keyboard focus: retain refs for the trigger and Back
button, focus the Back button when isOpen becomes true, and restore focus to the
trigger when it becomes false. Add aria-expanded to the trigger and handle
Escape while open by collapsing the control and restoring focus to the trigger,
while preserving existing disabled and selection behavior.
| const handlePointerEnter: PointerEventHandler<HTMLElement> = (event) => { | ||
| if (event.pointerType !== "touch") { | ||
| group.registerPointerTarget(event.currentTarget); | ||
| } | ||
| onPointerEnter?.(event); | ||
| }; | ||
| const handleFocus: FocusEventHandler<HTMLElement> = (event) => { | ||
| group.registerKeyboardTarget(); | ||
| onFocus?.(event); | ||
| }; | ||
| const handleKeyDown: KeyboardEventHandler<HTMLElement> = (event) => { | ||
| group.registerKeyboardTarget(); | ||
| onKeyDown?.(event); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Treat click focus as pointer input, not keyboard input.
A mouse click on a trigger fires focus, so handleFocus calls registerKeyboardTarget(). The group then sets keyboardNavigation and disables directional motion and transitions for the rest of the interaction. Gate the keyboard path on :focus-visible.
🐛 Proposed fix
const handleFocus: FocusEventHandler<HTMLElement> = (event) => {
- group.registerKeyboardTarget();
+ if (event.currentTarget.matches(":focus-visible")) {
+ group.registerKeyboardTarget();
+ }
onFocus?.(event);
};📝 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.
| const handlePointerEnter: PointerEventHandler<HTMLElement> = (event) => { | |
| if (event.pointerType !== "touch") { | |
| group.registerPointerTarget(event.currentTarget); | |
| } | |
| onPointerEnter?.(event); | |
| }; | |
| const handleFocus: FocusEventHandler<HTMLElement> = (event) => { | |
| group.registerKeyboardTarget(); | |
| onFocus?.(event); | |
| }; | |
| const handleKeyDown: KeyboardEventHandler<HTMLElement> = (event) => { | |
| group.registerKeyboardTarget(); | |
| onKeyDown?.(event); | |
| }; | |
| const handlePointerEnter: PointerEventHandler<HTMLElement> = (event) => { | |
| if (event.pointerType !== "touch") { | |
| group.registerPointerTarget(event.currentTarget); | |
| } | |
| onPointerEnter?.(event); | |
| }; | |
| const handleFocus: FocusEventHandler<HTMLElement> = (event) => { | |
| if (event.currentTarget.matches(":focus-visible")) { | |
| group.registerKeyboardTarget(); | |
| } | |
| onFocus?.(event); | |
| }; | |
| const handleKeyDown: KeyboardEventHandler<HTMLElement> = (event) => { | |
| group.registerKeyboardTarget(); | |
| onKeyDown?.(event); | |
| }; |
🤖 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/registry/sonaui/fluid-tooltip/fluid-tooltip.tsx` around lines 348 - 361,
Update handleFocus so it calls group.registerKeyboardTarget() only when
event.currentTarget.matches(":focus-visible"), while still invoking onFocus for
every focus event; leave handlePointerEnter and handleKeyDown behavior
unchanged.
| export function FluidTooltipContent({ | ||
| children, | ||
| className, | ||
| showArrow = true, | ||
| }: FluidTooltipContentProps) { | ||
| const root = useRootContext("FluidTooltip.Content"); | ||
| useLayoutEffect(() => { | ||
| root.contentRef.current = children; | ||
| root.contentClassNameRef.current = className; | ||
| root.showArrowRef.current = showArrow; | ||
| }, [children, className, root, showArrow]); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Store tooltip content in state so open tooltips update.
FluidTooltipContent writes children, className, and showArrow into refs. Ref writes do not schedule a render. FluidTooltipGroup reads those refs during render on Lines 212, 248, and 252. If the content changes while the tooltip is open, for example a label that toggles between "Play" and "Pause", the popup keeps the previous value. Reading mutable refs during render is also unsafe under concurrent rendering.
Hold these values in state on FluidTooltipRoot and pass the state through the payload.
🤖 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/registry/sonaui/fluid-tooltip/fluid-tooltip.tsx` around lines 390 - 402,
Move tooltip content, className, and showArrow storage from refs to state in
FluidTooltipRoot, and expose the state through the payload consumed by
FluidTooltipGroup. Update FluidTooltipContent to propagate changes via the
root’s state update mechanism so open tooltips re-render with current values,
and remove render-time reads of the mutable content refs.
|
|
||
| return ( | ||
| <LayoutGroup id={instanceId}> | ||
| <Dialog.Root defaultOpen={defaultOpen} onOpenChange={setOpen} open={open}> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not pass defaultOpen and open to the same Dialog.Root.
open is always defined, because it falls back to internalOpen. The dialog is therefore always controlled and defaultOpen is redundant. Line 49 already seeds internalOpen with defaultOpen. Remove the prop to avoid a controlled/uncontrolled conflict.
🐛 Proposed fix
- <Dialog.Root defaultOpen={defaultOpen} onOpenChange={setOpen} open={open}>
+ <Dialog.Root onOpenChange={setOpen} open={open}>📝 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.
| <Dialog.Root defaultOpen={defaultOpen} onOpenChange={setOpen} open={open}> | |
| <Dialog.Root onOpenChange={setOpen} open={open}> |
🤖 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/registry/sonaui/lightbox/lightbox.tsx` at line 65, Update the Dialog.Root
invocation to remove the redundant defaultOpen prop, keeping open={open} and
onOpenChange={setOpen} as the sole state controls. Preserve the existing
initialization of internalOpen from defaultOpen at line 49.
- Add `provider-setup.md` reference to clarify client-side configuration. - Enhance validation, selection, and design principles references. - Update `SKILL.md` structure and metadata. - Improve installation instructions and add smoke test validation for skill files.
|
🎉 This PR is included in version 2.18.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@scripts/validate-agent-skill.ts`:
- Around line 45-59: Update parseFrontmatter to parse the matched frontmatter
body with the project’s YAML parser before applying schema checks, and treat
parser errors—including unclosed quoted values—as invalid frontmatter. Build the
returned field map from the parsed YAML object while preserving the existing
validation behavior for top-level fields and unsupported structures.
In `@src/content/docs/agent-skill.mdx`:
- Line 64: Update the sample prompt in the agent-skill documentation so it
explicitly prohibits installation by changing “Before installing anything” to
“Do not install anything.” Preserve the instructions to inspect the consumer
project and Sona catalog, recommend a relevant component, and explain the
decision.
In `@src/registry/sonaui/agent-skill/references/provider-setup.md`:
- Around line 9-15: Update the Codex setup instruction in
src/registry/sonaui/agent-skill/references/provider-setup.md to state that the
TOML block belongs in ~/.codex/config.toml, then regenerate
public/r/agent-skill.json so its corresponding registry payload contains the
same path.
🪄 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 Plus
Run ID: c9ac6eac-7f1a-4a2b-8e62-9e0d09e03184
📒 Files selected for processing (13)
public/r/agent-skill.jsonpublic/r/registry.jsonregistry.jsonscripts/check-production-registry-install.tsscripts/validate-agent-skill.tssrc/content/docs/agent-skill.mdxsrc/registry/index.tssrc/registry/registry.jsonsrc/registry/sonaui/agent-skill/SKILL.mdsrc/registry/sonaui/agent-skill/references/component-selection.mdsrc/registry/sonaui/agent-skill/references/consumer-validation.mdsrc/registry/sonaui/agent-skill/references/design-principles.mdsrc/registry/sonaui/agent-skill/references/provider-setup.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/registry/registry.json
| function parseFrontmatter(source: string) { | ||
| const match = source.match(/^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/); | ||
| if (!match) return null; | ||
|
|
||
| const values = new Map<string, string>(); | ||
| for (const line of match[1].split(/\r?\n/)) { | ||
| if (/^\s/.test(line) || line.trim() === "") continue; | ||
| const field = line.match(/^([a-z][a-z0-9-]*):\s*(.*)$/); | ||
| if (!field) { | ||
| errors.push(`invalid top-level frontmatter line: ${line}`); | ||
| continue; | ||
| } | ||
| values.set(field[1], field[2].trim().replace(/^['"]|['"]$/g, "")); | ||
| } | ||
| return values; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Parse frontmatter with a YAML parser.
Line 45 accepts invalid YAML as valid frontmatter. For example, an unclosed quoted description can pass these checks and cause skill discovery to fail in the client. Parse YAML before applying the schema checks.
🤖 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 `@scripts/validate-agent-skill.ts` around lines 45 - 59, Update
parseFrontmatter to parse the matched frontmatter body with the project’s YAML
parser before applying schema checks, and treat parser errors—including unclosed
quoted values—as invalid frontmatter. Build the returned field map from the
parsed YAML object while preserving the existing validation behavior for
top-level fields and unsupported structures.
| /> | ||
|
|
||
| The agent should inspect the Sona catalog, recommend a relevant component, and wait before making a project change. | ||
| The agent should inspect the consumer project and Sona catalog, recommend a relevant component, explain the decision, and wait because the prompt explicitly says not to install. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the sample prompt prohibit installation.
Line 64 is inaccurate. “Before installing anything” permits installation after the explanation. Change the prompt to “Do not install anything” or remove the instruction that the agent must wait.
🤖 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/content/docs/agent-skill.mdx` at line 64, Update the sample prompt in the
agent-skill documentation so it explicitly prohibits installation by changing
“Before installing anything” to “Do not install anything.” Preserve the
instructions to inspect the consumer project and Sona catalog, recommend a
relevant component, and explain the decision.
| Add the shadcn server to the user's Codex configuration: | ||
|
|
||
| ```toml | ||
| [mcp_servers.shadcn] | ||
| command = "npx" | ||
| args = ["shadcn@latest", "mcp"] | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
shadcn MCP configuration ~/.codex/config.toml mcp_servers codex
💡 Result:
To configure the shadcn MCP server for Codex, you must manually add the configuration to your ~/.codex/config.toml file, as the shadcn CLI cannot update this file automatically [1][2]. Add the following block to your ~/.codex/config.toml: [mcp_servers.shadcn] command = "npx" args = ["shadcn@latest", "mcp"] After saving the file, restart Codex to load the MCP server [1][2][3]. Alternatively, you can use the Codex CLI to manage your configuration. The codex mcp subcommand provides tools to list, add, remove, and manage your MCP server launchers directly [4][5]. For example, running codex mcp add can help you generate the necessary configuration entries, though you may still need to verify the final TOML structure in your config file [4][6]. Note that Codex uses TOML format for its configuration, not JSON [6].
Citations:
- 1: https://ui.shadcn.com/docs/mcp
- 2: https://ui.shadcn.de/docs/mcp
- 3: https://github.com/shadcn-ui/ui/blob/15ac1be9/packages/shadcn/src/commands/mcp.ts
- 4: https://github.com/openai/codex/blob/d807d44a/codex-rs/cli/src/mcp_cmd.rs
- 5: https://github.com/openai/codex/blob/main/codex-rs/docs/codex_mcp_interface.md
- 6: https://designrevision.com/blog/add-mcp-server-to-codex
State the Codex configuration file path.
The Codex setup instruction currently omits where the TOML block belongs, and the generated registry payload repeats that omission. Add ~/.codex/config.toml to the Codex setup instruction and regenerate the public registry payload.
📍 Affects 2 files
src/registry/sonaui/agent-skill/references/provider-setup.md#L9-L15(this comment)public/r/agent-skill.json#L36-L36
🤖 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/registry/sonaui/agent-skill/references/provider-setup.md` around lines 9
- 15, Update the Codex setup instruction in
src/registry/sonaui/agent-skill/references/provider-setup.md to state that the
TOML block belongs in ~/.codex/config.toml, then regenerate
public/r/agent-skill.json so its corresponding registry payload contains the
same path.
This branch was successfully deployed
1 participant
expanding-actioncomponent and demofluid-tooltipcomponent and demoSummary by CodeRabbit