docs: inline docs, wiki pages, and repo files for v1.0.0 - #31
Merged
ViewFromTheBox merged 7 commits intoMar 31, 2026
Merged
Conversation
…files for v1.0.0 Add comprehensive documentation across all three packages: - LICENSE, README.md, CONTRIBUTING.md, CHANGELOG.md at repo root - JSDoc/TSDoc comments on all exported components, hooks, providers, and types - 17 wiki-ready markdown pages in docs/ covering getting started, tokens, theming, Laravel/Inertia integration, migration from Livewire, React compatibility, releasing, and full component API reference for all 56+ components - GitHub wiki structure with Home.md, _Sidebar.md, and [[wiki links]] Closes #29 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds repository-level docs (CHANGELOG, README, CONTRIBUTING, LICENSE), a docs site with many component guides, widespread JSDoc/TSDoc across packages, a TypeScript overload for useInertiaForm, exports Chart from data barrel, and five targeted runtime edits (Chart options deep-merge, Diff LCS/normalization, Drawer Escape gating, SpotlightSearch filter, Tabs panel spacing). Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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 (3)
packages/react/src/components/data/Diff/Diff.tsx (2)
124-137: 🧹 Nitpick | 🔵 TrivialConsider
push+reversefor O(k) backtracking.
result.unshift()is O(k) per call, making the overall backtracking O(k²) where k is the LCS length. For large files, replacing withpush+reverseat the end achieves O(k):♻️ Suggested optimization
const result: string[] = []; let i = m; let j = n; while (i > 0 && j > 0) { if (a[i - 1] === b[j - 1]) { - result.unshift(a[i - 1]); + result.push(a[i - 1]); i--; j--; } else if (dp[i - 1][j] > dp[i][j - 1]) { i--; } else { j--; } } - return result; + return result.reverse();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/data/Diff/Diff.tsx` around lines 124 - 137, The backtracking loop in Diff.tsx uses result.unshift(...) inside the while loop (variables result, i, j, m, n, a, b, dp), which makes backtracking O(k²); change to result.push(a[i - 1]) when a[i - 1] === b[j - 1] and after the loop call result.reverse() to restore order, converting the backtrack to O(k) and preserving the same output.
197-217: 🧹 Nitpick | 🔵 TrivialInconsistent text color styling between display modes.
In side-by-side mode, only background colors are applied (
bg-error/20,bg-success/20), while the inline mode useslineTypeClasseswhich includes both background and text colors (text-success-content,text-error-content). This creates a visual inconsistency where inline mode has colored text but side-by-side mode doesn't.Consider reusing
lineTypeClassesor applying text colors consistently across both modes:♻️ Suggested fix to apply consistent text colors
{/* Left pane */} - <div className={cn('flex', line.type === 'removed' && 'bg-error/20')}> + <div className={cn('flex', line.type === 'removed' && lineTypeClasses.removed)}> {line.type !== 'added' ? ( ... {/* Right pane */} - <div className={cn('flex', line.type === 'added' && 'bg-success/20')}> + <div className={cn('flex', line.type === 'added' && lineTypeClasses.added)}> {line.type !== 'removed' ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/data/Diff/Diff.tsx` around lines 197 - 217, The side-by-side panes apply only background classes (e.g., 'bg-error/20', 'bg-success/20') causing text color to differ from inline mode; update the left and right pane container className to include the same mapping used by inline mode (lineTypeClasses[line.type]) instead of or in addition to the hardcoded bg- classes so both background and text color are applied consistently; use the existing cn helper to merge classes (e.g., cn('flex', lineTypeClasses[line.type], /* any other classes like shrink/spacing */)) and remove or replace the explicit bg-error/20 / bg-success/20 strings so lookups via lineTypeClasses control styling for both modes.packages/react/src/components/layout/Drawer/Drawer.tsx (1)
130-134:⚠️ Potential issue | 🟡 MinorInconsistent Escape key handling compared to Sidebar.
Unlike
Sidebar.tsx(lines 109-119), this Drawer closes on any Escape press when open, regardless of focus location, and doesn't checke.defaultPrevented. This creates inconsistent behavior—Sidebar only closes if focus is within the drawer panel or body, but Drawer closes unconditionally.If an inner component (e.g., a nested dropdown or modal) handles Escape and prevents default, Drawer would still close, causing unexpected behavior. Consider aligning with Sidebar's approach by checking
e.defaultPreventedand verifying focus is within the panel before closing.🔧 Suggested alignment with Sidebar's behavior
const handleKeyDown = (e: globalThis.KeyboardEvent) => { if (e.key === 'Escape' && !persistent) { + if (e.defaultPrevented) return; + // Only close if focus is within the side panel (not main content) + const inPanel = panelRef.current?.contains(document.activeElement); + const inBody = document.body === document.activeElement || document.activeElement === null; + if (!inPanel && !inBody) return; onClose(); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/layout/Drawer/Drawer.tsx` around lines 130 - 134, The Drawer’s handleKeyDown currently closes on any Escape press; modify handleKeyDown to first ignore events with e.defaultPrevented and only trigger onClose when persistent is false AND the currently focused element is inside the drawer panel (e.g., use panelRef.current?.contains(document.activeElement) or similar). In short, update handleKeyDown to mirror Sidebar’s logic: check e.defaultPrevented, verify focus is within the drawer panel before calling onClose(), and leave persistent behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 52-57: The fenced code blocks in CONTRIBUTING.md (e.g., the
triple-backtick block showing the packages tree) lack a fence language and are
missing the required blank line before nested fenced blocks under numbered
lists, triggering MD031/MD040; fix by adding a language tag (use "text" for
plain blocks) to each triple-backtick fence and ensure there is a blank line
immediately before each fenced block that is nested under a list item. Apply the
same changes to the other affected ranges mentioned (blocks around 100-129 and
156-174), updating the fences that contain the package tree and any nested code
examples so they include ```text and a preceding blank line.
- Around line 182-184: Update the "Releasing" section to point first to the
in-repo checked-in guide (docs/releasing.md) instead of the external wiki; edit
the heading "Releasing" in CONTRIBUTING.md so the link targets docs/releasing.md
and optionally keep the wiki URL as a secondary/fallback link or note that the
wiki mirror is pending, ensuring the in-repo guide is the primary reference.
In `@docs/_Sidebar.md`:
- Around line 1-21: Replace the non-standard first line "**[[Home]]**" with a
top-level heading like "# [[Home]]" and add a single blank line before and after
each section heading ("### Guides", "### Components", "### Development") so the
Markdown linter stops flagging missing heading spacing; update the header and
spacing around those symbols in this file accordingly to preserve the same link
targets while satisfying markdownlint.
In `@docs/Component-API-Reference.md`:
- Around line 21-25: The example imports Badge from the wrong subpath: update
the import that currently reads "import { Table, Badge } from
'@artisanpack-ui/react/data';" so that Badge is imported from the display
package (e.g., "import { Badge } from '@artisanpack-ui/react/display';") and
keep Table coming from the data package (or split into two import lines) to
ensure the Badge symbol is imported from the correct module.
In `@docs/Design-Tokens.md`:
- Around line 202-209: The Tailwind plugin example imports the wrong symbol;
update the import in the example so it imports createArtisanPackPlugin instead
of artisanpackTokens from the '@artisanpack-ui/tokens/tailwind' subpath (replace
the import statement in the Tailwind Plugin code block to reference
createArtisanPackPlugin).
In `@docs/Laravel-Inertia-Integration.md`:
- Around line 50-79: The example's import list for CreateUserPage is missing the
Checkbox component used in the JSX; update the top import that currently reads
"import { Input, Button, Select } from '@artisanpack-ui/react';" to also include
Checkbox so it becomes "Input, Button, Select, Checkbox" (or add a separate
import for Checkbox) ensuring the Checkbox symbol used in the JSX and the
useInertiaForm usage remain consistent.
In `@docs/Migration-from-Livewire.md`:
- Line 149: Update the documentation line about "Inertia's usePage()" to
explicitly state the import source: instruct readers to import usePage from
`@inertiajs/react` (e.g., import { usePage } from '@inertiajs/react') and clarify
that `@artisanpack-ui/react-laravel` does not export usePage; reference the symbol
usePage and the module names `@inertiajs/react` and `@artisanpack-ui/react-laravel`
so readers know where to import it from.
In `@docs/Navigation-Components.md`:
- Around line 146-157: The docs table for SpotlightItem lists properties that
don't exist in the actual SpotlightItem interface; update the docs to match the
interface by removing the `keywords` and `renderLink` rows from the
SpotlightItem property table in Navigation-Components.md so it only documents
`key`, `label`, `description`, `icon`, and `group`; verify against the
SpotlightItem interface in the SpotlightSearch component to ensure parity.
- Around line 37-46: The MenuItemType docs table is missing the renderLink
property; update the Markdown table under "MenuItemType" to include a row for
`renderLink` with type `(props) => ReactElement` and a brief description like
"Custom link renderer (for routing libraries)"; reference the actual interface
symbol `MenuItemType` and the implementation in `Menu.tsx` (the `renderLink`
property) to ensure the table matches the component's interface.
In `@packages/react/src/components/data/Chart/Chart.tsx`:
- Around line 16-25: ChartDataPoint.color is declared but per-point colors are
only used in the pie/donut branch; for axis charts (single-series bar/line) the
rendering uses a single resolved color so data[].color is ignored — either
narrow ChartDataPoint.color to only apply to pie/donut or update the axis-chart
rendering to respect per-point colors. Fix: in ChartDataPoint keep the color
optional, and modify the axis chart rendering code paths (the single-series
bar/line render functions and the code that computes the single resolved color)
to prefer data[i].color for each bar/point when present (falling back to the
series/global color), ensuring the same color resolution logic used by the
pie-like branch is reused for functions that draw axis charts.
In `@packages/react/src/components/data/index.ts`:
- Around line 1-7: The module index is missing exports for the Chart component
and its related types; add exports for the default Chart component and the types
ChartProps, ChartSeries, ChartDataPoint, and ChartType so the chart API is part
of the public module surface (export the Chart component and named type exports
from the Chart implementation, e.g., export { default as Chart } and export type
{ ChartProps, ChartSeries, ChartDataPoint, ChartType } from the Chart module).
In `@packages/react/src/components/data/Stat/Stat.tsx`:
- Around line 32-33: The JSDoc for the Stat component's prop changeLabel is
inaccurate: it says "appended after the change percentage" but the
implementation appends it after the whole description block (change indicator +
description). Update the comment for the changeLabel prop in Stat (prop name:
changeLabel in component Stat) to read something like "Additional label appended
after the change indicator and description" and ensure any related docs or prop
tables that reference the old wording are updated to match the actual rendering
behavior.
In `@packages/react/src/components/feedback/Skeleton/Skeleton.tsx`:
- Around line 22-23: Update the JSDoc for the Skeleton component's circle prop
to precisely describe the fallback logic: when circle is true and only one of
width or height is provided, that single dimension is used for both width and
height; when both width and height are provided, each will be applied (which may
not yield a perfect circle). Reference the circle prop and the Skeleton
component to ensure readers can correlate the doc with the implementation.
In `@packages/react/src/components/form/Button/Button.tsx`:
- Around line 24-25: Update the JSDoc for the loading prop on the Button
component to accurately reflect runtime behavior: state that loading shows a
spinner in place of the left icon and disables interaction when rendering a
native button element, but when the component is rendered as a link (link prop)
it only sets aria-disabled and does not prevent navigation; reference the
loading prop and link prop in the comment so consumers understand both cases.
In `@packages/react/src/components/layout/Collapse/Collapse.tsx`:
- Around line 29-30: Update the prop documentation to clarify that the name prop
does NOT implement cross-instance exclusive-open behavior: currently Collapse
maintains per-instance internalOpen state and emits onOpenChange, so grouping
multiple Collapse components by name will NOT auto-close siblings; if
exclusive-open behavior is required it must be implemented by a
shared/controlled parent. Specifically, edit the JSDoc/comments for the name
prop and any related docs referencing lines about native radio-group behavior to
state that name only sets an HTML name attribute for form semantics, that
internalOpen is instance-local, and that consumers must coordinate via
onOpenChange (or supply a shared controlled state) to achieve one-open-at-a-time
grouping. Include references to the name prop, internalOpen state, and
onOpenChange callback in the updated documentation.
In `@packages/react/src/components/layout/Tabs/Tabs.tsx`:
- Around line 56-59: The panel spacing is always using left padding (pl-4) even
when verticalRight is true, so when the panel is rendered before the tab list
the gap ends up on the outer edge; update the Tabs component render logic that
adds the panel container padding (the code handling vertical/verticalRight in
Tabs.tsx and the duplicate logic around the other occurrence at lines ~292-297)
to apply side-specific spacing: use pl-4 when vertical is true and use pr-4 when
verticalRight is true (or compute the padding class based on verticalRight),
ensuring the panel receives the correct inner gap between content and the tab
list.
In `@packages/react/src/components/navigation/Pagination/Pagination.tsx`:
- Around line 27-30: The JSDoc `@defaultValue` for props previousLabel and
nextLabel in Pagination.tsx is incorrect (documents '<<' and '>>' while the
component actually uses '«' and '»'); fix by making them consistent—either
update the JSDoc comments for previousLabel and nextLabel to '@defaultValue '«''
and '@defaultValue '»'' to match the defaults used in the Pagination component,
or change the component defaults (where previousLabel/nextLabel are set) to '<<'
and '>>' to match the docs; reference the previousLabel and nextLabel prop
comments and the Pagination component default assignments to apply the change.
In
`@packages/react/src/components/navigation/SpotlightSearch/SpotlightSearch.tsx`:
- Around line 1-8: Update the module-level JSDoc to accurately describe the
matching behavior implemented by defaultFilter: replace "Provides fuzzy
filtering" with wording that reflects the actual algorithm (e.g., "Provides
word-based substring filtering and grouping") so the top-level comment matches
the defaultFilter function's documented behavior; reference the module header
and the defaultFilter function to ensure the description aligns with the
function's "word-based substring filter" implementation.
- Around line 65-66: The JSDoc for the prop filterFn is misleading — it mentions
"fuzzy matching" while the component uses a different defaultFilter; update the
comment on filterFn in SpotlightSearch.tsx to accurately describe that filterFn
overrides the component's defaultFilter behavior (used to determine whether a
SpotlightItem matches a query) and specify the signature (item: SpotlightItem,
query: string) => boolean so it matches the actual prop and defaultFilter
implementation.
In `@README.md`:
- Around line 66-82: The example is missing the createLayout import used in
LoginPage.layout; update the top import to include createLayout from
'@artisanpack-ui/react-laravel' alongside useInertiaForm and AppLayout so
createLayout is defined when setting LoginPage.layout, ensuring the component
example runs as shown.
---
Outside diff comments:
In `@packages/react/src/components/data/Diff/Diff.tsx`:
- Around line 124-137: The backtracking loop in Diff.tsx uses
result.unshift(...) inside the while loop (variables result, i, j, m, n, a, b,
dp), which makes backtracking O(k²); change to result.push(a[i - 1]) when a[i -
1] === b[j - 1] and after the loop call result.reverse() to restore order,
converting the backtrack to O(k) and preserving the same output.
- Around line 197-217: The side-by-side panes apply only background classes
(e.g., 'bg-error/20', 'bg-success/20') causing text color to differ from inline
mode; update the left and right pane container className to include the same
mapping used by inline mode (lineTypeClasses[line.type]) instead of or in
addition to the hardcoded bg- classes so both background and text color are
applied consistently; use the existing cn helper to merge classes (e.g.,
cn('flex', lineTypeClasses[line.type], /* any other classes like shrink/spacing
*/)) and remove or replace the explicit bg-error/20 / bg-success/20 strings so
lookups via lineTypeClasses control styling for both modes.
In `@packages/react/src/components/layout/Drawer/Drawer.tsx`:
- Around line 130-134: The Drawer’s handleKeyDown currently closes on any Escape
press; modify handleKeyDown to first ignore events with e.defaultPrevented and
only trigger onClose when persistent is false AND the currently focused element
is inside the drawer panel (e.g., use
panelRef.current?.contains(document.activeElement) or similar). In short, update
handleKeyDown to mirror Sidebar’s logic: check e.defaultPrevented, verify focus
is within the drawer panel before calling onClose(), and leave persistent
behavior unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: df90668f-855d-4266-90d5-6145a15e04cb
📒 Files selected for processing (112)
CHANGELOG.mdCONTRIBUTING.mdLICENSEREADME.mddocs/Component-API-Reference.mddocs/Data-Display-Components.mddocs/Design-Tokens.mddocs/Feedback-Components.mddocs/Form-Components.mddocs/Getting-Started.mddocs/Home.mddocs/Laravel-Inertia-Integration.mddocs/Layout-Components.mddocs/Migration-from-Livewire.mddocs/Navigation-Components.mddocs/React-18-vs-19-Compatibility.mddocs/Theming.mddocs/Utility-Components.mddocs/_Sidebar.mdpackages/react-laravel/src/auth/index.tspackages/react-laravel/src/feedback/InertiaToastProvider.tsxpackages/react-laravel/src/feedback/index.tspackages/react-laravel/src/form/FormContext.tsxpackages/react-laravel/src/form/InertiaForm.tsxpackages/react-laravel/src/form/index.tspackages/react-laravel/src/form/useInertiaForm.tspackages/react-laravel/src/index.tspackages/react-laravel/src/layout/index.tspackages/react-laravel/src/navigation/InertiaBreadcrumbs.tsxpackages/react-laravel/src/navigation/InertiaLink.tsxpackages/react-laravel/src/navigation/InertiaMenu.tsxpackages/react-laravel/src/navigation/InertiaPagination.tsxpackages/react-laravel/src/navigation/index.tspackages/react-laravel/src/types.tspackages/react/src/components/data/Avatar/Avatar.tsxpackages/react/src/components/data/Badge/Badge.tsxpackages/react/src/components/data/Calendar/Calendar.tsxpackages/react/src/components/data/Carousel/Carousel.tsxpackages/react/src/components/data/Chart/Chart.tsxpackages/react/src/components/data/Code/Code.tsxpackages/react/src/components/data/Diff/Diff.tsxpackages/react/src/components/data/Progress/Progress.tsxpackages/react/src/components/data/Sparkline/Sparkline.tsxpackages/react/src/components/data/Stat/Stat.tsxpackages/react/src/components/data/Table/Table.tsxpackages/react/src/components/data/Timeline/Timeline.tsxpackages/react/src/components/data/index.tspackages/react/src/components/display/index.tspackages/react/src/components/feedback/Alert/Alert.tsxpackages/react/src/components/feedback/EmptyState/EmptyState.tsxpackages/react/src/components/feedback/Error/Error.tsxpackages/react/src/components/feedback/Loading/Loading.tsxpackages/react/src/components/feedback/Skeleton/Skeleton.tsxpackages/react/src/components/feedback/Toast/Toast.tsxpackages/react/src/components/feedback/index.tspackages/react/src/components/form/Button/Button.tsxpackages/react/src/components/form/Checkbox/Checkbox.tsxpackages/react/src/components/form/ColorPicker/ColorPicker.tsxpackages/react/src/components/form/DatePicker/DatePicker.tsxpackages/react/src/components/form/Editor/Editor.tsxpackages/react/src/components/form/File/File.tsxpackages/react/src/components/form/Input/Input.tsxpackages/react/src/components/form/Password/Password.tsxpackages/react/src/components/form/Pin/Pin.tsxpackages/react/src/components/form/Radio/Radio.tsxpackages/react/src/components/form/Range/Range.tsxpackages/react/src/components/form/RichTextEditor/RichTextEditor.tsxpackages/react/src/components/form/Select/Select.tsxpackages/react/src/components/form/Textarea/Textarea.tsxpackages/react/src/components/form/Toggle/Toggle.tsxpackages/react/src/components/form/index.tspackages/react/src/components/layout/Accordion/Accordion.tsxpackages/react/src/components/layout/Card/Card.tsxpackages/react/src/components/layout/Collapse/Collapse.tsxpackages/react/src/components/layout/Divider/Divider.tsxpackages/react/src/components/layout/Drawer/Drawer.tsxpackages/react/src/components/layout/Dropdown/Dropdown.tsxpackages/react/src/components/layout/Grid/Grid.tsxpackages/react/src/components/layout/Modal/Modal.tsxpackages/react/src/components/layout/Popover/Popover.tsxpackages/react/src/components/layout/Stack/Stack.tsxpackages/react/src/components/layout/Tabs/Tabs.tsxpackages/react/src/components/layout/index.tspackages/react/src/components/navigation/Breadcrumbs/Breadcrumbs.tsxpackages/react/src/components/navigation/Menu/Menu.tsxpackages/react/src/components/navigation/Navbar/Navbar.tsxpackages/react/src/components/navigation/Pagination/Pagination.tsxpackages/react/src/components/navigation/Sidebar/Sidebar.tsxpackages/react/src/components/navigation/SpotlightSearch/SpotlightSearch.tsxpackages/react/src/components/navigation/Steps/Steps.tsxpackages/react/src/components/navigation/index.tspackages/react/src/components/utility/Clipboard/Clipboard.tsxpackages/react/src/components/utility/Icon/Icon.tsxpackages/react/src/components/utility/Markdown/Markdown.tsxpackages/react/src/components/utility/ThemeToggle/ThemeToggle.tsxpackages/react/src/components/utility/Tooltip/Tooltip.tsxpackages/react/src/components/utility/index.tspackages/react/src/hooks/use-theme.tsxpackages/react/src/index.tspackages/react/src/types/common.tspackages/react/src/types/form.tspackages/tokens/src/animation.tspackages/tokens/src/borders.tspackages/tokens/src/color-resolver.tspackages/tokens/src/colors.tspackages/tokens/src/glass.tspackages/tokens/src/index.tspackages/tokens/src/shadows.tspackages/tokens/src/spacing.tspackages/tokens/src/types.tspackages/tokens/src/typography.tspackages/tokens/src/utils/cn.ts
Comment on lines
+21
to
+25
| import { Button, Input } from '@artisanpack-ui/react/form'; | ||
| import { Card, Modal } from '@artisanpack-ui/react/layout'; | ||
| import { Menu } from '@artisanpack-ui/react/navigation'; | ||
| import { Table, Badge } from '@artisanpack-ui/react/data'; | ||
| import { Alert, Toast } from '@artisanpack-ui/react/feedback'; |
There was a problem hiding this comment.
Fix likely incorrect subpath import in example.
At Line 24, Badge is imported from @artisanpack-ui/react/data, but Badge belongs to display components in this package structure. This example can cause broken imports for users.
Proposed doc fix
-import { Table, Badge } from '@artisanpack-ui/react/data';
+import { Table } from '@artisanpack-ui/react/data';
+import { Badge } from '@artisanpack-ui/react/display';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/Component-API-Reference.md` around lines 21 - 25, The example imports
Badge from the wrong subpath: update the import that currently reads "import {
Table, Badge } from '@artisanpack-ui/react/data';" so that Badge is imported
from the display package (e.g., "import { Badge } from
'@artisanpack-ui/react/display';") and keep Table coming from the data package
(or split into two import lines) to ensure the Badge symbol is imported from the
correct module.
- Fix fenced code block language tags and spacing in CONTRIBUTING.md - Point Releasing link to in-repo docs/Releasing.md - Fix _Sidebar.md heading format and spacing - Fix Tailwind plugin import symbol in Design-Tokens.md - Add missing Checkbox import in Laravel-Inertia example - Add usePage import source in Migration-from-Livewire.md - Add missing createLayout import in README.md example - Export Chart and types from data/index.ts - Fix changeLabel, circle, loading, name, filterFn JSDoc accuracy - Fix Pagination previousLabel/nextLabel @DefaultValue to match actual defaults - Fix SpotlightSearch module/filterFn docs to say "word-based substring" not "fuzzy" - Fix Tabs vertical padding: use pr-4 for verticalRight, pl-4 for vertical - Optimize Diff LCS backtracking: push+reverse instead of unshift - Guard Drawer Escape handler with defaultPrevented and focus check Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/react/src/components/form/Button/Button.tsx (1)
136-145:⚠️ Potential issue | 🟠 MajorDisabled link variant is still interactive.
aria-disabledalone does not block click/keyboard activation on<a>. Withdisabledorloading, this branch should prevent navigation (and ideally remove from tab order) to match button semantics and avoid accidental actions.Proposed fix
const element = link ? ( <a href={link} className={buttonClasses} target={external ? '_blank' : undefined} rel={external ? 'noopener noreferrer' : undefined} role="button" aria-busy={loading || undefined} aria-disabled={disabled || loading || undefined} + tabIndex={disabled || loading ? -1 : undefined} + onClick={(e) => { + if (disabled || loading) { + e.preventDefault(); + e.stopPropagation(); + } + }} > {content} </a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/form/Button/Button.tsx` around lines 136 - 145, The anchor branch for the JSX element (the const element when link is truthy) currently only sets aria-disabled but remains interactive; update the link branch that renders the <a> so that when disabled || loading is true it prevents activation by removing or omitting href, setting tabIndex={-1}, adding an onClick/onKeyDown handler that calls event.preventDefault() when disabled/loading, and avoid setting target/rel in that state; keep aria-busy and aria-disabled for accessibility and ensure the same buttonClasses are applied so the visual styling stays consistent (refer to the const element, link, loading, disabled, buttonClasses, and external symbols).packages/react/src/components/data/Diff/Diff.tsx (2)
143-153: 🧹 Nitpick | 🔵 TrivialTighten map typings to the
DiffLineunion.Using
Record<string, string>weakens type safety. Constrain keys toDiffLine['type']to catch invalid lookups at compile time.🔧 Proposed refactor
-const lineTypeClasses: Record<string, string> = { +const lineTypeClasses: Record<DiffLine['type'], string> = { added: 'bg-success/20 text-success-content', removed: 'bg-error/20 text-error-content', same: '', }; -const linePrefix: Record<string, string> = { +const linePrefix: Record<DiffLine['type'], string> = { added: '+', removed: '-', same: ' ', };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/data/Diff/Diff.tsx` around lines 143 - 153, The maps lineTypeClasses and linePrefix are typed as Record<string,string>, which is too loose; change their types to Record<DiffLine['type'], string> (or a Partial<Record<DiffLine['type'], string>> if some variants are optional) and update the declarations for lineTypeClasses and linePrefix accordingly so TypeScript enforces that keys exactly match the DiffLine union; ensure all DiffLine types (added, removed, same) are present or handle missing keys as appropriate.
50-53:⚠️ Potential issue | 🟡 MinorHandle truly empty inputs without emitting a phantom line.
split('\n')on''produces[''], so empty-vs-empty currently renders one blank diff row (line 1/1). Treat empty content as zero lines to avoid this UI artifact.💡 Proposed fix
function computeDiff(oldText: string, newText: string): DiffLine[] { - const oldLines = oldText.split('\n'); - const newLines = newText.split('\n'); + const oldLines = oldText === '' ? [] : oldText.split('\n'); + const newLines = newText === '' ? [] : newText.split('\n'); const result: DiffLine[] = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/data/Diff/Diff.tsx` around lines 50 - 53, computeDiff currently treats an empty string as one blank line because oldText.split('\n')/newText.split('\n') yields ['']; update the oldLines/newLines initialization in computeDiff to treat '' as zero lines (e.g. const oldLines = oldText === '' ? [] : oldText.split('\n'); and similarly for newLines) so truly empty inputs produce no diff rows and avoid the phantom blank line.packages/react/src/components/navigation/Pagination/Pagination.tsx (1)
42-78: 🧹 Nitpick | 🔵 TrivialLGTM: Function documentation is accurate.
The JSDoc correctly describes the page range generation logic. The
@paramand@returnstags are helpful and match the implementation.Optional: Redundant safety checks.
Lines 65 and 73 contain defensive checks (
!pages.includes(i)and!pages.includes(total)) that are technically unnecessary since:
rangeStartis always ≥ 2 (line 55), soiin the loop can never equal 1.rangeEndis always ≤total - 1(line 56), sototalcan never already be in thepagesarray before line 73.These checks don't cause bugs and can be kept as defensive programming, but they could be removed for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/navigation/Pagination/Pagination.tsx` around lines 42 - 78, The getPageRange function contains redundant existence checks: remove the defensive checks inside the loop and before pushing total — specifically delete the if (!pages.includes(i)) guard around the loop body that pushes i, and delete the if (!pages.includes(total)) guard before pages.push(total); rely on the established rangeStart >= 2 and rangeEnd <= total - 1 invariants so the loop can push i directly and pages.push(total) can be unconditional.packages/react/src/components/layout/Collapse/Collapse.tsx (1)
113-115: 🧹 Nitpick | 🔵 TrivialConsider: ARIA attributes semantically belong on the interactive element.
Lines 113-115 (and 123-125 for checkbox) place
aria-expanded,aria-controls, andaria-labelledbyon the hidden<input>element. Semantically,aria-expandedis more appropriate for the visible interactive trigger—in this case, the.collapse-titlediv (line 129).However, DaisyUI's CSS framework may rely on the input having these attributes for styling behavior. If that's the case, the current implementation is acceptable for framework compatibility.
If DaisyUI doesn't require ARIA on the input, consider moving these attributes to the title div and making it a
<button>or addingrole="button" tabindex="0"for proper keyboard interaction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/layout/Collapse/Collapse.tsx` around lines 113 - 115, The ARIA attributes (aria-expanded, aria-controls, aria-labelledby) are currently applied to the hidden input in Collapse; move them to the visible trigger element (.collapse-title) inside the Collapse component so the interactive control (use the .collapse-title div or better convert it to a <button>) exposes aria-expanded={isOpen}, aria-controls={`collapse-content-${autoId}`} and aria-labelledby={`collapse-title-${autoId}`} and ensure keyboard accessibility by using a <button> or adding role="button" and tabindex="0"; also apply the same change for the checkbox variant and only keep the attributes on the hidden <input> if DaisyUI styling requires them.
♻️ Duplicate comments (3)
CONTRIBUTING.md (1)
187-187:⚠️ Potential issue | 🟡 MinorFix case-sensitive path for release guide link.
The checked-in file is
docs/releasing.md;./docs/Releasing.mdmay break on case-sensitive environments.Proposed doc fix
-See [docs/Releasing.md](./docs/Releasing.md) for the full release process, including stable releases, snapshots, and troubleshooting. +See [docs/releasing.md](./docs/releasing.md) for the full release process, including stable releases, snapshots, and troubleshooting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` at line 187, Update the case of the release guide link in the CONTRIBUTING.md line that reads "See [docs/Releasing.md](./docs/Releasing.md)..." to point to the actual checked-in filename by changing the link target (and optionally the link text) to use "releasing.md" — e.g., replace "./docs/Releasing.md" with "./docs/releasing.md" so the link works on case-sensitive filesystems.packages/react/src/components/layout/Collapse/Collapse.tsx (2)
29-29:⚠️ Potential issue | 🟠 MajorDocumentation still incorrectly claims browser enforcement of radio behavior.
The phrase "the browser enforces one-open-at-a-time within the same name group" is incorrect with the current controlled-input implementation. Because each Collapse instance renders
checked={isOpen}(line 111) driven by its owninternalOpenstate, React overrides the browser's radio semantics—multiple radios with the samenamecan appear checked simultaneously.The added note about internal state is helpful, but the core claim about browser enforcement should be corrected.
Suggested wording:
-/** HTML `name` attribute — switches to a radio input so the browser enforces one-open-at-a-time within the same name group. Note: the component's internal state may not reflect sibling closures; use controlled `open` + `onOpenChange` if you need to track state precisely. */ +/** HTML `name` attribute — switches to a radio input for grouped behavior. Note: one-open-at-a-time semantics require controlled `open` + `onOpenChange` with parent coordination; the component's per-instance internal state does not enforce sibling closures. */This issue was previously flagged in past reviews.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/layout/Collapse/Collapse.tsx` at line 29, The doc comment on the Collapse component incorrectly states that the browser enforces one-open-at-a-time via the HTML name attribute; update the comment (referencing Collapse, the checked={isOpen} usage and internalOpen state) to remove that claim and instead note that because each instance is controlled (checked driven by internalOpen/prop), React can render multiple checked radios even with the same name and you should use a controlled open + onOpenChange to coordinate exclusive-open behavior across siblings.
107-128:⚠️ Potential issue | 🔴 CriticalCritical: Controlled radio inputs break one-open-at-a-time semantics.
The current implementation uses controlled radio inputs (
checked={isOpen}at line 111) whereisOpenis derived from per-instance state (internalOpenat line 78). This creates a fundamental conflict:Reproduction scenario:
- Render two Collapse components with
name="group"and both uncontrolled- Open Collapse A → its
internalOpenbecomestrue- Open Collapse B → its
internalOpenbecomestrue- Browser attempts to uncheck Collapse A's radio (native radio semantics)
- But A's
onChangedoes NOT fire, so A'sinternalOpenstaystrue- Both re-render with
checked={true}→ both radios appear checkedReact's controlled
checkedprop overrides the browser's native radio group coordination, violating the documented one-open-at-a-time behavior.Possible solutions:
- Make radio inputs uncontrolled (remove
checkedprop, adddefaultChecked)- Require parent-level controlled state for all grouped Collapse components
- Implement a context provider that coordinates siblings with the same
nameWithout one of these fixes, the
nameprop doesn't provide functional accordion behavior.🔍 Verification: confirm radio conflict
Run this script to check if there's test coverage validating one-open-at-a-time behavior:
#!/bin/bash # Search for tests covering radio/name group accordion behavior rg -n -C3 'name.*=.*["\'].*["\']' --glob '*Collapse*.test.*' --glob '*Collapse*.spec.*' rg -n -C3 'accordion|radio.*group|one.*open' --glob '*Collapse*.test.*' --glob '*Collapse*.spec.*'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/layout/Collapse/Collapse.tsx` around lines 107 - 128, The radio inputs break native group behavior because they are rendered as controlled (checked={isOpen}) while isOpen is driven by per-instance internalOpen; change the radio branch to render an uncontrolled input when a name is provided by replacing checked={isOpen} with defaultChecked={internalOpen} (keep onChange={handleToggle}, aria attributes and disabled), so browser radio coordination can uncheck siblings; preserve the current controlled behavior for the checkbox branch and ensure symbols referenced are internalOpen, isOpen, handleToggle and the name prop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/_Sidebar.md`:
- Around line 3-23: Change the section headings that currently use "###" to "##"
so levels don't skip from the H1 "# [[Home]]" to H3; specifically update the
"### Guides", "### Components", and "### Development" headers to "## Guides",
"## Components", and "## Development" (and any other top-level sections in the
same file using "###") to satisfy MD001 and maintain proper heading hierarchy.
In `@docs/Laravel-Inertia-Integration.md`:
- Around line 20-28: The example Layout component passes non-existent props to
AppLayout; update the snippet so Layout uses AppLayout's actual props: replace
navbar={<Navbar .../>} and sidebar={<Sidebar .../>} with the correct header and
footer props (e.g., header={<Navbar brand="My App" />} and footer={<Sidebar
items={[...]} />}), and add any required props like title or className if
needed; update references to Navbar, Sidebar, and the children prop accordingly
so AppLayout receives header/footer/title/className instead of navbar/sidebar.
In `@packages/react/src/components/feedback/Skeleton/Skeleton.tsx`:
- Around line 29-30: The documentation is too absolute about aria-hidden; update
the Skeleton component docs to state that aria-hidden is applied by default but
can be overridden by consumer props passed via the ...rest spread in the
Skeleton component (or alternatively enforce a non-overridable aria-hidden by
removing aria-hidden from the ...rest merge in the Skeleton implementation if
you want to prevent overrides). Locate the Skeleton component and its prop
spread usage (the ...rest/props handling and the aria-hidden attribute) and
either reword the comment to "aria-hidden is applied by default and may be
overridden via props" or change the implementation so aria-hidden is set after
spreading props to make it non-overridable.
In `@packages/react/src/components/layout/Collapse/Collapse.tsx`:
- Around line 1-10: The module comment incorrectly claims "native radio-group
accordion behaviour"; update the top-of-file docstring in the Collapse component
to remove "native" and instead say it uses "accordion behaviour via radio inputs
and the `name` prop (requires controlled state or parent coordination for true
one-open-at-a-time behavior)" to accurately reflect that checked is controlled
via isOpen/internalOpen and multiple instances won't auto-coordinate; reference
the Collapse component and the checked={isOpen} / internalOpen behavior when
making the wording change.
In `@packages/react/src/components/layout/Drawer/Drawer.tsx`:
- Around line 1-10: Prettier flagged formatting issues in the Drawer component
file; run the project's Prettier formatter against this file (e.g., run prettier
--write on packages/react/src/components/layout/Drawer/Drawer.tsx or use the
repo's formatting script) to auto-fix style problems, then commit the formatted
file so the Drawer module/header and any React component code within Drawer.tsx
conform to the repo's Prettier rules.
- Around line 131-136: The current keydown handler returns early on
e.defaultPrevented which inadvertently blocks the Tab focus-trap logic; update
the handler so the e.defaultPrevented guard is moved inside the Escape branch
only: check e.key === 'Escape' and then if e.defaultPrevented return, and only
then consider persistent, panelRef.current?.contains(document.activeElement) and
call onClose(); leave the Tab-handling/focus-trap logic (the code that handles
'Tab' key) unconditional so focus trapping always runs. Ensure you reference the
existing symbols panelRef, onClose, persistent, and the
e.defaultPrevented/Escape/Tab checks when making the change.
In `@packages/react/src/components/layout/Tabs/Tabs.tsx`:
- Around line 1-10: Prettier flagged formatting issues in the Tabs component;
run the formatter (prettier --write) on the Tabs module (the Tabs component) to
auto-fix styling inconsistencies, then re-run lint/CI; ensure the top-level
JSDoc and the Tabs component export follow the project's Prettier/ESLint rules
so the file passes prettier --check and CI.
In
`@packages/react/src/components/navigation/SpotlightSearch/SpotlightSearch.tsx`:
- Around line 64-65: The JSDoc for the SpotlightSearch component's placeholder
prop mismatches the actual default string: update the `@defaultValue` in the
comment for the placeholder property to use the Unicode ellipsis ('Search…') so
it matches the default used in SpotlightSearch (placeholder prop / default
value), ensuring the doc and implementation are consistent.
---
Outside diff comments:
In `@packages/react/src/components/data/Diff/Diff.tsx`:
- Around line 143-153: The maps lineTypeClasses and linePrefix are typed as
Record<string,string>, which is too loose; change their types to
Record<DiffLine['type'], string> (or a Partial<Record<DiffLine['type'], string>>
if some variants are optional) and update the declarations for lineTypeClasses
and linePrefix accordingly so TypeScript enforces that keys exactly match the
DiffLine union; ensure all DiffLine types (added, removed, same) are present or
handle missing keys as appropriate.
- Around line 50-53: computeDiff currently treats an empty string as one blank
line because oldText.split('\n')/newText.split('\n') yields ['']; update the
oldLines/newLines initialization in computeDiff to treat '' as zero lines (e.g.
const oldLines = oldText === '' ? [] : oldText.split('\n'); and similarly for
newLines) so truly empty inputs produce no diff rows and avoid the phantom blank
line.
In `@packages/react/src/components/form/Button/Button.tsx`:
- Around line 136-145: The anchor branch for the JSX element (the const element
when link is truthy) currently only sets aria-disabled but remains interactive;
update the link branch that renders the <a> so that when disabled || loading is
true it prevents activation by removing or omitting href, setting tabIndex={-1},
adding an onClick/onKeyDown handler that calls event.preventDefault() when
disabled/loading, and avoid setting target/rel in that state; keep aria-busy and
aria-disabled for accessibility and ensure the same buttonClasses are applied so
the visual styling stays consistent (refer to the const element, link, loading,
disabled, buttonClasses, and external symbols).
In `@packages/react/src/components/layout/Collapse/Collapse.tsx`:
- Around line 113-115: The ARIA attributes (aria-expanded, aria-controls,
aria-labelledby) are currently applied to the hidden input in Collapse; move
them to the visible trigger element (.collapse-title) inside the Collapse
component so the interactive control (use the .collapse-title div or better
convert it to a <button>) exposes aria-expanded={isOpen},
aria-controls={`collapse-content-${autoId}`} and
aria-labelledby={`collapse-title-${autoId}`} and ensure keyboard accessibility
by using a <button> or adding role="button" and tabindex="0"; also apply the
same change for the checkbox variant and only keep the attributes on the hidden
<input> if DaisyUI styling requires them.
In `@packages/react/src/components/navigation/Pagination/Pagination.tsx`:
- Around line 42-78: The getPageRange function contains redundant existence
checks: remove the defensive checks inside the loop and before pushing total —
specifically delete the if (!pages.includes(i)) guard around the loop body that
pushes i, and delete the if (!pages.includes(total)) guard before
pages.push(total); rely on the established rangeStart >= 2 and rangeEnd <= total
- 1 invariants so the loop can push i directly and pages.push(total) can be
unconditional.
---
Duplicate comments:
In `@CONTRIBUTING.md`:
- Line 187: Update the case of the release guide link in the CONTRIBUTING.md
line that reads "See [docs/Releasing.md](./docs/Releasing.md)..." to point to
the actual checked-in filename by changing the link target (and optionally the
link text) to use "releasing.md" — e.g., replace "./docs/Releasing.md" with
"./docs/releasing.md" so the link works on case-sensitive filesystems.
In `@packages/react/src/components/layout/Collapse/Collapse.tsx`:
- Line 29: The doc comment on the Collapse component incorrectly states that the
browser enforces one-open-at-a-time via the HTML name attribute; update the
comment (referencing Collapse, the checked={isOpen} usage and internalOpen
state) to remove that claim and instead note that because each instance is
controlled (checked driven by internalOpen/prop), React can render multiple
checked radios even with the same name and you should use a controlled open +
onOpenChange to coordinate exclusive-open behavior across siblings.
- Around line 107-128: The radio inputs break native group behavior because they
are rendered as controlled (checked={isOpen}) while isOpen is driven by
per-instance internalOpen; change the radio branch to render an uncontrolled
input when a name is provided by replacing checked={isOpen} with
defaultChecked={internalOpen} (keep onChange={handleToggle}, aria attributes and
disabled), so browser radio coordination can uncheck siblings; preserve the
current controlled behavior for the checkbox branch and ensure symbols
referenced are internalOpen, isOpen, handleToggle and the name prop.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31c5d4c5-d713-413d-a610-0b86ae68b222
📒 Files selected for processing (17)
.gitignoreCONTRIBUTING.mdREADME.mddocs/Design-Tokens.mddocs/Laravel-Inertia-Integration.mddocs/Migration-from-Livewire.mddocs/_Sidebar.mdpackages/react/src/components/data/Diff/Diff.tsxpackages/react/src/components/data/Stat/Stat.tsxpackages/react/src/components/data/index.tspackages/react/src/components/feedback/Skeleton/Skeleton.tsxpackages/react/src/components/form/Button/Button.tsxpackages/react/src/components/layout/Collapse/Collapse.tsxpackages/react/src/components/layout/Drawer/Drawer.tsxpackages/react/src/components/layout/Tabs/Tabs.tsxpackages/react/src/components/navigation/Pagination/Pagination.tsxpackages/react/src/components/navigation/SpotlightSearch/SpotlightSearch.tsx
- Fix Prettier formatting in Drawer.tsx and Tabs.tsx (CI failure) - Fix _Sidebar.md heading hierarchy (### → ##) - Fix AppLayout example to use actual header/footer props - Clarify Skeleton aria-hidden is overridable via props - Fix Collapse module doc and switch radio input to defaultChecked for proper browser radio-group coordination - Move defaultPrevented guard inside Escape branch in Drawer so Tab focus-trap still works - Fix SpotlightSearch placeholder @DefaultValue to use ellipsis - Fix Diff empty string handling and narrow Record types - Tabs verticalRight padding already fixed (Prettier reformatted) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/react/src/components/layout/Collapse/Collapse.tsx (1)
7-8:⚠️ Potential issue | 🟠 Major
defaultCheckedde-syncs radio mode from controlledopenstate.At Line 112, switching to
defaultChecked={isOpen}makes the radio input uncontrolled after mount, butAccordionstill drivesCollapsevia controlledopen. This can desynchronize DOM checked state from React state and makes the one-open guarantee in Line 7-8 / Line 30 inaccurate.Suggested fix
- * `name` prop. When `name` is set, uses `defaultChecked` so the browser - * can coordinate one-open-at-a-time across siblings sharing the same name. + * `name` prop. For true one-open-at-a-time behavior, coordinate siblings + * via shared controlled state (`open` + `onOpenChange`) in a parent. - /** HTML `name` attribute — switches to an uncontrolled radio input so the browser can coordinate one-open-at-a-time within the same name group. Use controlled `open` + `onOpenChange` if you need to track state precisely. */ + /** HTML `name` attribute for radio semantics. For one-open-at-a-time groups, coordinate sibling `Collapse` state in a parent using `open` + `onOpenChange`. */ name?: string; - defaultChecked={isOpen} + checked={isOpen}#!/bin/bash # Verify the controlled/uncontrolled mismatch between Collapse and Accordion. rg -n -C3 'defaultChecked=\{isOpen\}|checked=\{isOpen\}|name=\{name\}' packages/react/src/components/layout/Collapse/Collapse.tsx rg -n -C8 'cloneElement\(child, \{|open:\s*isOpen|onOpenChange' packages/react/src/components/layout/Accordion/Accordion.tsxExpected verification result:
Collapsecurrently usesdefaultChecked={isOpen}in the radio branch whileAccordionprovides controlledopenupdates.Also applies to: 30-30, 112-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/layout/Collapse/Collapse.tsx` around lines 7 - 8, Collapse uses defaultChecked={isOpen} for the radio input which makes the DOM uncontrolled after mount and can desync from the controlled open state driven by Accordion; change the radio branch in the Collapse component to use a controlled input (checked={isOpen}) and wire its onChange/onClick to the existing open toggle handler (the same handler used for the non-radio branch or the passed onOpenChange) so the input stays in sync with the isOpen prop, while preserving the name prop for browser-level one-at-a-time grouping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/Laravel-Inertia-Integration.md`:
- Line 71: The example references an undefined roles variable; add a local roles
definition near the snippet (e.g., a constant array of options) so the line
Select label="Role" options={roles} {...field('role')} is runnable—define roles
as an array of option objects (matching your Select component's expected shape)
above the JSX and ensure the options' keys/labels align with Select and
field('role').
- Around line 90-96: The docs incorrectly show methods as
form.post/form.put/form.patch/form.destroy; update the table to reference the
top-level InertiaFormHelpers methods (post, put, patch, destroy) instead of
using the form. prefix so they match the InertiaFormHelpers interface; change
each entry for `form.post`, `form.put`, `form.patch`, `form.destroy` to `post`,
`put`, `patch`, `destroy` and ensure any surrounding text refers to using the
InertiaFormHelpers helpers rather than methods on a `form` object.
In `@packages/react/src/components/data/Diff/Diff.tsx`:
- Around line 51-52: Normalize CRLF vs LF before splitting to avoid spurious
diffs: in the Diff component, ensure you normalize both oldText and newText
(e.g., replace carriage returns or convert \r\n to \n) before computing oldLines
and newLines so the existing lines comparison isn't polluted by leftover '\r'
characters; update the code that produces oldLines/newLines (referencing
oldText, newText, oldLines, newLines) to perform this normalization prior to
calling split('\n').
In `@packages/react/src/components/layout/Tabs/Tabs.tsx`:
- Around line 297-300: Add regression assertions in the Tabs unit test to
validate the panel spacing classes when the component is vertical and
verticalRight toggles; specifically, update the Tabs.test to render the Tabs
(component Tabs) with isVertical true and check that the panel element (the
element receiving panelClassName / className) contains 'pr-4' when verticalRight
is true and 'pl-4' when verticalRight is false, in addition to the existing
wrapper 'flex' assertion, so any regression changing the pr-4/pl-4 spacing is
caught.
---
Duplicate comments:
In `@packages/react/src/components/layout/Collapse/Collapse.tsx`:
- Around line 7-8: Collapse uses defaultChecked={isOpen} for the radio input
which makes the DOM uncontrolled after mount and can desync from the controlled
open state driven by Accordion; change the radio branch in the Collapse
component to use a controlled input (checked={isOpen}) and wire its
onChange/onClick to the existing open toggle handler (the same handler used for
the non-radio branch or the passed onOpenChange) so the input stays in sync with
the isOpen prop, while preserving the name prop for browser-level one-at-a-time
grouping.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ac5a349e-06fe-4d13-a419-e7684cf5ca8a
📒 Files selected for processing (8)
docs/Laravel-Inertia-Integration.mddocs/_Sidebar.mdpackages/react/src/components/data/Diff/Diff.tsxpackages/react/src/components/feedback/Skeleton/Skeleton.tsxpackages/react/src/components/layout/Collapse/Collapse.tsxpackages/react/src/components/layout/Drawer/Drawer.tsxpackages/react/src/components/layout/Tabs/Tabs.tsxpackages/react/src/components/navigation/SpotlightSearch/SpotlightSearch.tsx
Comment on lines
+297
to
+300
| className={cn( | ||
| isVertical ? (verticalRight ? 'flex-1 pr-4' : 'flex-1 pl-4') : 'p-4', | ||
| panelClassName, | ||
| )} |
There was a problem hiding this comment.
Add regression assertions for verticalRight panel spacing.
The runtime fix is good, but current coverage (see packages/react/src/__tests__/layout/Tabs.test.tsx Line 110-120) only checks the wrapper flex class and won’t catch pr-4/pl-4 regressions.
✅ Suggested test additions
+it('applies right-side panel gap in verticalRight layout', () => {
+ const { container } = render(<Tabs tabs={sampleTabs} verticalRight />);
+ const tabPanel = container.querySelector('[role="tabpanel"]');
+ expect(tabPanel).toHaveClass('flex-1', 'pr-4');
+ expect(tabPanel).not.toHaveClass('pl-4');
+});
+
+it('applies left-side panel gap in vertical layout', () => {
+ const { container } = render(<Tabs tabs={sampleTabs} vertical />);
+ const tabPanel = container.querySelector('[role="tabpanel"]');
+ expect(tabPanel).toHaveClass('flex-1', 'pl-4');
+ expect(tabPanel).not.toHaveClass('pr-4');
+});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react/src/components/layout/Tabs/Tabs.tsx` around lines 297 - 300,
Add regression assertions in the Tabs unit test to validate the panel spacing
classes when the component is vertical and verticalRight toggles; specifically,
update the Tabs.test to render the Tabs (component Tabs) with isVertical true
and check that the panel element (the element receiving panelClassName /
className) contains 'pr-4' when verticalRight is true and 'pl-4' when
verticalRight is false, in addition to the existing wrapper 'flex' assertion, so
any regression changing the pr-4/pl-4 spacing is caught.
- Add roles variable to useInertiaForm example in Laravel docs
- Document both top-level and form.method() submit patterns
- Revert Collapse radio input to checked={isOpen} — Accordion passes
controlled `open` prop, so defaultChecked would desync after mount
- Update Collapse module doc and name prop JSDoc accordingly
- Normalize CRLF line endings in Diff before splitting
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/react/src/components/layout/Collapse/Collapse.tsx (1)
6-8:⚠️ Potential issue | 🟠 Major
nameprop docs still overstate exclusive accordion behaviorLine 30 still implies guaranteed one-open-at-a-time behavior from
name, but this component keeps instance-localinternalOpenand does not coordinate sibling state by itself. Please reword to state thatnameselects radio input semantics only, and true exclusivity requires shared controlled state (e.g., parent viaopen/onOpenChange).Suggested doc wording
- * plus indicator icons, and radio-input accordion behaviour via the - * `name` prop. When used inside an {`@link` Accordion}, state is managed + * plus indicator icons, and radio-input semantics via the + * `name` prop. For true one-open-at-a-time accordion behavior across + * multiple instances, coordinate state in a parent. When used inside + * an {`@link` Accordion}, state is managed * by the parent via the controlled `open` prop.- /** HTML `name` attribute — switches to a radio input for one-open-at-a-time grouping. When used inside an Accordion, state is coordinated by the parent via `open` + `onOpenChange`. */ + /** HTML `name` attribute for radio input semantics. For guaranteed one-open-at-a-time grouping across multiple Collapse instances, coordinate via parent-controlled `open` + `onOpenChange` (e.g., Accordion). */ name?: string;Also applies to: 30-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/layout/Collapse/Collapse.tsx` around lines 6 - 8, Update the Collapse component docs so the description of the name prop no longer claims it enforces one-open-at-a-time behavior; instead state that name only enables radio-input semantics for keyboard/ARIA and that true exclusivity must be implemented via shared controlled state (e.g., parent-managed open/onOpenChange in Accordion). Edit the text near the `name` prop and anywhere referencing `internalOpen`/`open` to clarify the component maintains instance-local `internalOpen` and does not coordinate sibling Collapse instances, and add a short note suggesting using a parent (like `Accordion`) to manage exclusive open state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/Laravel-Inertia-Integration.md`:
- Around line 88-91: The documentation omits the returned name property from the
field helpers; update the table for `field(name)` and `checkbox(name)` to
include `name` in their return shapes to match the InertiaFormHelpers
implementation in useInertiaForm.ts (i.e., document that `field(name)` returns
`{ name, value, onChange, onBlur, error }` and `checkbox(name)` returns `{ name,
checked, onChange, error }` so components can spread the `name` attribute).
---
Duplicate comments:
In `@packages/react/src/components/layout/Collapse/Collapse.tsx`:
- Around line 6-8: Update the Collapse component docs so the description of the
name prop no longer claims it enforces one-open-at-a-time behavior; instead
state that name only enables radio-input semantics for keyboard/ARIA and that
true exclusivity must be implemented via shared controlled state (e.g.,
parent-managed open/onOpenChange in Accordion). Edit the text near the `name`
prop and anywhere referencing `internalOpen`/`open` to clarify the component
maintains instance-local `internalOpen` and does not coordinate sibling Collapse
instances, and add a short note suggesting using a parent (like `Accordion`) to
manage exclusive open state.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f6a17bbf-d726-4cda-a0f9-4604f24da58b
📒 Files selected for processing (3)
docs/Laravel-Inertia-Integration.mdpackages/react/src/components/data/Diff/Diff.tsxpackages/react/src/components/layout/Collapse/Collapse.tsx
- Add name to field/checkbox helper return shapes in Laravel docs - Collapse: clarify that name prop does not coordinate siblings; recommend Accordion or controlled state for exclusive open behaviour Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/Laravel-Inertia-Integration.md`:
- Around line 116-127: The InertiaForm example uses a non-existent API surface;
replace the function-children + initialValues/action usage with the actual API:
create the form state with useInertiaForm(...) and pass that form object into
the InertiaForm via the form prop, rename action to url, keep method, and render
children as normal JSX (not a function) — update references to InertiaForm,
useInertiaForm, and props form/url/method/children accordingly so the snippet
matches the real component signature.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 532cf200-30e1-42db-b1e7-9101c42ae1c0
📒 Files selected for processing (2)
docs/Laravel-Inertia-Integration.mdpackages/react/src/components/layout/Collapse/Collapse.tsx
5 tasks
…e Releasing.md - Replace NPM_TOKEN with NPM_CONFIG_PROVENANCE in ci.yml and snapshot.yml for OIDC-based trusted publisher auth (no long-lived token needed) - Update Releasing docs: prerequisites and troubleshooting now reference trusted publishers instead of NPM_TOKEN - Fix InertiaForm example to use actual API (form/url/method props, regular children) instead of non-existent initialValues/action/function-children - Rename docs/releasing.md → docs/Releasing.md for wiki filename consistency Relates to #26 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
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
tokens,react,react-laravel)docs/withHome.md,_Sidebar.md, and[[wiki links]]— ready to copy directly intoreact.wiki.gitChanges
@moduleheaders, prop descriptions,@exampleblocks,@defaultValuetagsTest plan
npm run buildpassesnpm run type-checkpasses (tsc --noEmit)](./grep returns 0 matches)docs/to wiki repo and verify renderingCloses #29
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores