feat(platform): extract ReorderList and LocaleTabs components#967
Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
067b55b to
6f0df49
Compare
📝 WalkthroughWalkthroughThis PR introduces two new UI components— Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The review requires understanding two new generic component APIs and their integration points, verifying Storybook story completeness, and validating the refactoring of conversation-starters to correctly wire state and callbacks through the new components, including the override-slot padding/trimming logic. Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/routes/dashboard/$id/agents/$agentId/conversation-starters.tsx (1)
187-190:⚠️ Potential issue | 🔴 CriticalFix return type to resolve typecheck failure.
hasLocaleContentreturnsboolean | undefinedwhenstartersisundefined, butLocaleTabs.hasContentexpects(locale: string) => boolean. This causes the pipeline failure at line 244.🐛 Proposed fix
function hasLocaleContent(locale: string) { const starters = config.i18n?.[locale]?.conversationStarters; - return starters && starters.length > 0; + return Boolean(starters && starters.length > 0); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/routes/dashboard/`$id/agents/$agentId/conversation-starters.tsx around lines 187 - 190, The function hasLocaleContent currently can return undefined; change it to always return a boolean so it matches LocaleTabs.hasContent: inside hasLocaleContent (referencing config.i18n and conversationStarters) coerce the result to a boolean (e.g., check starters?.length and return a boolean expression such as (starters?.length ?? 0) > 0 or use !! to cast) so the function signature effectively becomes (locale: string) => boolean and the typecheck will pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/components/ui/forms/reorder-list.tsx`:
- Around line 38-52: Remove the unnecessary React reserved "key" property from
the ReorderListRowProps<T extends ReorderItem> interface: delete the key?:
string declaration in ReorderListRowProps, and update any code that attempted to
pass or type-check a "key" through this interface (e.g., places using
ReorderListRowProps or components that accept those props); rely on React's
external key handling when rendering list items and keep the rest of the props
(item, index, total, readonlyOrder, onMoveUp, onMoveDown, onRemove, renderItem,
moveUpLabel, moveDownLabel, dragHandleLabel, removeLabel) unchanged.
- Around line 63-66: The default aria-label strings (moveUpLabel, moveDownLabel,
dragHandleLabel, removeLabel) in the ReorderList component are hardcoded
English; remove those literal defaults from the parameter destructuring in the
ReorderList function, update the ReorderListProps type to make these four props
required (non-optional), and fix all call sites to pass translated labels (or
alternatively move to use the app's translation hook inside ReorderList to
derive fallback values); ensure references to moveUpLabel, moveDownLabel,
dragHandleLabel, and removeLabel are only used from props and that TS types
reflect they must be provided.
In `@services/platform/app/components/ui/navigation/locale-tabs.tsx`:
- Around line 45-81: The tab UI rendered in tabs.map should expose ARIA roles:
add role="tablist" to the container element that wraps the mapped buttons, and
add role="tab" plus aria-selected={active} on each button returned by the map
(the button that uses onLocaleChange and checks active via
activeLocale/isDefault). Also ensure the button remains focusable and that any
visual active state (the active span) corresponds to aria-selected for the
button; optionally keep disabled handling as-is (disabled buttons should not
have aria-selected=true).
---
Outside diff comments:
In
`@services/platform/app/routes/dashboard/`$id/agents/$agentId/conversation-starters.tsx:
- Around line 187-190: The function hasLocaleContent currently can return
undefined; change it to always return a boolean so it matches
LocaleTabs.hasContent: inside hasLocaleContent (referencing config.i18n and
conversationStarters) coerce the result to a boolean (e.g., check
starters?.length and return a boolean expression such as (starters?.length ?? 0)
> 0 or use !! to cast) so the function signature effectively becomes (locale:
string) => boolean and the typecheck will pass.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e0916da2-b0c0-46b8-8a09-bb3cda75e47d
📒 Files selected for processing (5)
services/platform/app/components/ui/forms/reorder-list.stories.tsxservices/platform/app/components/ui/forms/reorder-list.tsxservices/platform/app/components/ui/navigation/locale-tabs.stories.tsxservices/platform/app/components/ui/navigation/locale-tabs.tsxservices/platform/app/routes/dashboard/$id/agents/$agentId/conversation-starters.tsx
…cale sync Extract inline reorder and locale tab UI from conversation-starters into general-purpose components with Storybook stories. ReorderList: uses useDragControls for smoother drag, larger arrow buttons (IconButton size-7), readonlyOrder prop, and generic render prop API. LocaleTabs: underline-style locale switcher with default/untranslated badges and actions slot. Fix conversation starters locale sync: override locales now pad/trim slots to match default locale count, preserving index alignment. Structure controls (add/remove/reorder) are hidden on override locales.
6f0df49 to
810ea3c
Compare
Extract inline reorder and locale tab UI from conversation-starters into general-purpose components with Storybook stories.
ReorderList: uses useDragControls for smoother drag, larger arrow buttons (IconButton size-7), readonlyOrder prop, and generic render prop API.
LocaleTabs: underline-style locale switcher with default/untranslated badges and actions slot.
Fix conversation starters locale sync: override locales now pad/trim slots to match default locale count, preserving index alignment. Structure controls (add/remove/reorder) are hidden on override locales.
Summary by CodeRabbit
Release Notes