fix(#736): updates for doc issues found in apsara audit#756
fix(#736): updates for doc issues found in apsara audit#756
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a site-level 404 page and a ChipInput demo component. Splits datatable demo into non-virtualized and virtualized named exports and updates datatable docs to include a virtualized preview. Demo plumbing (Preview, DemoPreview, DemoPlayground) and demo types now accept an optional inline Sequence Diagram(s)sequenceDiagram
participant Page as Demo Page
participant Playground as DemoPlayground
participant PreviewWrap as DemoPreview
participant Preview as Preview/LivePreview
participant DemoComp as Demo Component (e.g., DataTableDemo / ChipInputDemo)
Page->>Playground: render <DemoPlayground style={...} scope={...} />
Playground->>PreviewWrap: render <DemoPreview style={style} ... />
PreviewWrap->>Preview: render <Preview style={style} />
Preview->>Preview: apply className + style -> LivePreview
Preview->>DemoComp: render demo from scope (e.g., DataTableDemo / ChipInputDemo)
Note right of DemoComp: Demo receives scope props and renders inside LivePreview
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/www/src/content/docs/components/input-field/demo.ts (1)
131-159: Consider deduplicating/guarding chip entries.Minor UX polish for the demo: pressing Enter on an already-present label will add a duplicate chip (e.g.,
Tag1twice), and very long input produces a chip that may push the layout. Not a bug, but since this demo is meant to showcase realistic usage, a small guard would make it a better reference implementation for consumers.♻️ Suggested tweak
onKeyDown={(e) => { - if (e.key === "Enter" && input.trim()) { - setChips([...chips, { label: input.trim() }]); - setInput(""); + if (e.key === "Enter" && input.trim()) { + e.preventDefault(); + const label = input.trim(); + if (!chips.some((c) => c.label === label)) { + setChips([...chips, { label }]); + } + setInput(""); } }}The
e.preventDefault()also avoids any accidental form submission if this demo is ever embedded inside a<form>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/src/content/docs/components/input-field/demo.ts` around lines 131 - 159, In ChipInputDemo, prevent adding duplicate or excessively long chips by enhancing the onKeyDown handler on the InputField: call e.preventDefault(), compute const value = input.trim(), check that value is non-empty, not already present in chips (compare against chips.map(c=>c.label)), and optionally limit length (e.g., value.length <= MAX_LABEL_LENGTH) before calling setChips([...chips, {label: value}]) and clearing setInput(""); update the chips onRemove logic remains the same.apps/www/src/app/not-found.tsx (1)
13-17: Render the 404 copy with semantic elements.
Textdefaults tospan, so this page lacks heading and paragraph semantics for assistive-tech navigation. Use therenderprop to render the status ash1and message aspwhile preserving visual styling.Proposed semantic markup update
- <Text size={10} weight='bold'> + <Text render={<h1 />} size={10} weight='bold' style={{ margin: 0 }}> 404 </Text> <div className={styles.divider} /> - <Text size={7}>This page could not be found.</Text> + <Text render={<p />} size={7} style={{ margin: 0 }}> + This page could not be found. + </Text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/src/app/not-found.tsx` around lines 13 - 17, The Text component is currently rendering as a span, so update the two Text usages to use the render prop to provide semantic elements: render the status Text (the one with size={10} weight='bold') as an h1 and render the message Text (size={7}) as a p, preserving their size/weight props and keeping the existing divider (styles.divider) in place; locate the Text usages in not-found.tsx and add render="h1" and render="p" respectively.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/src/content/docs/components/search/demo.ts`:
- Around line 46-60: The ControlledSearch component is defined but never
rendered; after the ControlledSearch function declaration add an instance of it
(i.e., render <ControlledSearch />) so the controlled demo appears in the live
preview — locate the ControlledSearch function in the diff and append a JSX
mount of ControlledSearch right after its definition.
In `@packages/raystack/components/search/search.tsx`:
- Around line 35-36: The uncontrolled Search initializes internalValue to '' so
a passed defaultValue is ignored and both value and defaultValue get forwarded;
change the uncontrolled initialization in the Search component so internalValue
is initialized from props.defaultValue (e.g., const [internalValue,
setInternalValue] = useState(() => props.defaultValue ?? '')), keep currentValue
= isControlled ? controlledValue : internalValue, and when rendering the
underlying input only pass value when isControlled (so the uncontrolled case
only forwards defaultValue in ...props); update references to internalValue,
setInternalValue, currentValue, isControlled and controlledValue accordingly.
---
Nitpick comments:
In `@apps/www/src/app/not-found.tsx`:
- Around line 13-17: The Text component is currently rendering as a span, so
update the two Text usages to use the render prop to provide semantic elements:
render the status Text (the one with size={10} weight='bold') as an h1 and
render the message Text (size={7}) as a p, preserving their size/weight props
and keeping the existing divider (styles.divider) in place; locate the Text
usages in not-found.tsx and add render="h1" and render="p" respectively.
In `@apps/www/src/content/docs/components/input-field/demo.ts`:
- Around line 131-159: In ChipInputDemo, prevent adding duplicate or excessively
long chips by enhancing the onKeyDown handler on the InputField: call
e.preventDefault(), compute const value = input.trim(), check that value is
non-empty, not already present in chips (compare against chips.map(c=>c.label)),
and optionally limit length (e.g., value.length <= MAX_LABEL_LENGTH) before
calling setChips([...chips, {label: value}]) and clearing setInput(""); update
the chips onRemove logic remains the same.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 64524df2-bf9e-4881-9147-e69994eb7b89
📒 Files selected for processing (15)
apps/www/src/app/not-found.module.cssapps/www/src/app/not-found.tsxapps/www/src/components/datatable-demo.tsxapps/www/src/components/demo/demo-preview.tsxapps/www/src/components/demo/demo.tsxapps/www/src/components/demo/styles.module.cssapps/www/src/components/preview/preview.module.cssapps/www/src/content/docs/components/breadcrumb/demo.tsapps/www/src/content/docs/components/calendar/demo.tsapps/www/src/content/docs/components/datatable/demo.tsapps/www/src/content/docs/components/input-field/demo.tsapps/www/src/content/docs/components/input-field/index.mdxapps/www/src/content/docs/components/search/demo.tspackages/raystack/components/accordion/accordion-root.tsxpackages/raystack/components/search/search.tsx
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 `@apps/www/src/components/demo/demo-preview.tsx`:
- Around line 47-74: The UI branches assume codePreview is a non-empty array and
directly index into codePreview[0] and codePreview[activeCodePreviewTab]; guard
against empty arrays by checking Array.isArray(codePreview) &&
codePreview.length > 0 before indexing, and render a sensible fallback (e.g.,
null/placeholder) when empty. Update the two branches that render Editor (where
code is taken from codePreview[activeTab] / codePreview[0] /
codePreview[activeCodePreviewTab]) to use a safe expression or early return when
codePreview.length === 0, ensuring Editor only receives a defined code string
and buttons map over the array only when it has items.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c9a66f9-898f-4f5a-b18e-9cb302b191e7
📒 Files selected for processing (13)
apps/www/src/app/not-found.tsxapps/www/src/components/demo/demo-playground.tsxapps/www/src/components/demo/demo-preview.tsxapps/www/src/components/demo/styles.module.cssapps/www/src/components/demo/types.tsapps/www/src/components/preview/preview.module.cssapps/www/src/components/preview/preview.tsxapps/www/src/content/docs/components/accordion/demo.tsapps/www/src/content/docs/components/announcement-bar/demo.tsapps/www/src/content/docs/components/breadcrumb/demo.tsapps/www/src/content/docs/components/datatable/demo.tsapps/www/src/content/docs/components/navbar/demo.tsapps/www/src/content/docs/components/sidebar/demo.ts
✅ Files skipped from review due to trivial changes (6)
- apps/www/src/components/demo/styles.module.css
- apps/www/src/content/docs/components/announcement-bar/demo.ts
- apps/www/src/content/docs/components/accordion/demo.ts
- apps/www/src/app/not-found.tsx
- apps/www/src/components/demo/types.ts
- apps/www/src/content/docs/components/breadcrumb/demo.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/www/src/components/preview/preview.module.css
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/www/src/components/datatable-demo.tsx (1)
113-119: Extract duplicated literals (height,defaultSort,rowHeight) into named constants.This avoids drift between demo variants and makes tuning easier.
♻️ Proposed refactor
+const TABLE_HEIGHT = 400; +const DEFAULT_SORT = { name: 'email', order: 'asc' } as const; +const VIRTUALIZED_ROW_HEIGHT = 44.5; + const DataTableDemo = () => { const data = useMemo(() => generateData(100), []); return ( <Flex direction='column' gap={4} width='full'> - <div style={{ height: 400 }}> + <div style={{ height: TABLE_HEIGHT }}> <DataTable data={data} mode='client' columns={columns} - defaultSort={{ name: 'email', order: 'asc' }} + defaultSort={DEFAULT_SORT} > <DataTable.Toolbar /> <DataTable.Content /> @@ const DataTableVirtualizedDemo = () => { const data = useMemo(() => generateData(1000), []); @@ <Flex direction='column' gap={4} width='full'> - <div style={{ height: 400 }}> + <div style={{ height: TABLE_HEIGHT }}> <DataTable data={data} mode='client' columns={columns} - defaultSort={{ name: 'email', order: 'asc' }} + defaultSort={DEFAULT_SORT} > <DataTable.Toolbar /> - <DataTable.VirtualizedContent rowHeight={44.5} /> + <DataTable.VirtualizedContent rowHeight={VIRTUALIZED_ROW_HEIGHT} /> </DataTable>Also applies to: 133-139, 142-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/src/components/datatable-demo.tsx` around lines 113 - 119, The DataTable demo duplicates literals for the container height, defaultSort and rowHeight across variants; create named constants (e.g., TABLE_HEIGHT, DEFAULT_SORT, ROW_HEIGHT) at the top of the component and replace inline literals in the DataTable usage and surrounding div style (references: the <DataTable ... defaultSort=... rowHeight=... /> usage and the enclosing div style={{ height: ... }}), then reuse those constants for the other demo blocks mentioned to keep values consistent and easy to tune.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/src/components/inputfield-chip-demo.tsx`:
- Around line 20-29: The add/remove handlers for chips capture the stale chips
array from render; change both setChips usages to the functional updater form to
base changes on the latest state: in the onKeyDown handler that currently does
setChips([...chips, { label: input.trim() }]) use setChips(prev => [...prev, {
label: input.trim() }]) and in the chips mapping where onRemove calls
setChips(chips.filter((_, j) => j !== i)) use setChips(prev => prev.filter((_,
j) => j !== i)); keep clearing input with setInput('') as before and ensure you
still reference input.trim() for the new label.
---
Nitpick comments:
In `@apps/www/src/components/datatable-demo.tsx`:
- Around line 113-119: The DataTable demo duplicates literals for the container
height, defaultSort and rowHeight across variants; create named constants (e.g.,
TABLE_HEIGHT, DEFAULT_SORT, ROW_HEIGHT) at the top of the component and replace
inline literals in the DataTable usage and surrounding div style (references:
the <DataTable ... defaultSort=... rowHeight=... /> usage and the enclosing div
style={{ height: ... }}), then reuse those constants for the other demo blocks
mentioned to keep values consistent and easy to tune.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42769879-0019-4ceb-8087-3388aaca0d1e
📒 Files selected for processing (5)
apps/www/src/components/datatable-demo.tsxapps/www/src/components/demo/demo.tsxapps/www/src/components/inputfield-chip-demo.tsxapps/www/src/content/docs/components/input-field/demo.tsapps/www/src/content/docs/components/input-field/index.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/www/src/content/docs/components/input-field/index.mdx
- apps/www/src/components/demo/demo.tsx
- apps/www/src/content/docs/components/input-field/demo.ts
Description
Resolves multiple documentation issues identified in the Apsara UI audit. Fixes include:
Type of Change
How Has This Been Tested?
Checklist:
Related Issues
#736