Conversation
WalkthroughAdds synchronization from TagInput tags to the form via a useEffect in create-graph and refactors TagInput internals to centralize commit/removal logic (delimiters, paste, blur), validation/deduplication, and Backspace behavior; updates placeholder, delimiter semantics, and form description copy. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2468 +/- ##
========================================
- Coverage 1.50% 1.50% -0.01%
========================================
Files 292 292
Lines 46768 46816 +48
Branches 431 431
========================================
Hits 703 703
- Misses 45782 45830 +48
Partials 283 283
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
studio/src/components/create-graph.tsx (1)
264-279: Placeholder example now conflicts with delimiter guidance.The placeholder still shows comma-separated matchers, but the delimiter list is now space/Enter and the comment explicitly says commas are valid inside values. Update the example to match the new UX to avoid confusion.
✅ Suggested fix
- placeholder="key1=value1,key2=value2 ..." + placeholder="key1=value1 key2=value2 ..."
🤖 Fix all issues with AI agents
In `@studio/src/components/ui/tag-input/tag-input.tsx`:
- Around line 134-165: commitInputValue currently reads the render-time tags
variable and calls setTags(nextTags), which can cause lost updates if multiple
commits happen before re-render; change it to use the functional updater form of
setTags so each commit applies to the latest state: inside commitInputValue (and
when computing nextTags from nextTagTexts) call setTags(prevTags => { let t =
prevTags; for (const text of nextTagTexts) { t = tryAddTag(text, t); } if (t !==
prevTags) { setTagCount(t.length); return t; } return prevTags; }); remove
reliance on the outer tags variable and ensure setTagCount is updated from the
resulting tags length.
- Around line 172-197: The new handlers (handleInputBlur, handleKeyDown,
handlePaste) currently mutate tags even when readOnly/disabled is set and the
input element isn't receiving readOnly; update each handler to early-return if
props.readOnly or props.disabled is true, and ensure the input JSX forwards the
readOnly and disabled props to the <input> element so the browser prevents
edits. Specifically, add a guard at the top of handleInputBlur, handleKeyDown,
and handlePaste to skip calling commitInputValue, setTags, setTagCount, or
onTagRemove when readOnly/disabled is true, and pass readOnly={readOnly} and
disabled={disabled} into the input element so the UI reflects the non-editable
state.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@studio/src/components/create-graph.tsx`:
- Line 264: The placeholder "key1=value1,key2=value2 ..." contradicts
FormDescription which says commas denote multiple values for the same key (e.g.,
"team=A,team=B"); update the placeholder(s) used in the input(s) to match that
description (for example "team=A,team=B") so the placeholder aligns with
FormDescription; locate the placeholder prop(s) in create-graph.tsx (the input
element(s) around the current placeholder line and the similar block at lines
~285-288) and replace them consistently.
|
@coderabbitai fullreview |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@studio/src/components/ui/tag-input/tag-input.tsx`:
- Around line 133-152: The paste/blur splitting in commitInputValue only splits
on single-character delimiters (charDelimiters) so configured "Enter" (newline)
delimiters aren't applied to multi-line input; update commitInputValue to also
detect newline delimiters from delimiterList (e.g., '\n' or '\r\n' or any
delimiter containing a newline) and, when splitByDelimiters is true, split the
trimmed text by both the existing charDelimiters regex and a newline-aware regex
(/\\r?\\n/), then merge results, trim and filter empty strings before calling
setInputValue/continuing; reference commitInputValue, delimiterList,
splitByDelimiters, charDelimiters, and escapeForCharClass.
🧹 Nitpick comments (1)
studio/src/components/ui/tag-input/tag-input.tsx (1)
155-163: Consider moving onTagAdd/onTagRemove side effects out of state updaters.
React can invoke updater functions more than once in StrictMode/concurrent rendering, which can duplicate callbacks. AuseEffectdiff between previous and current tags keeps callbacks deterministic.Possible approach (conceptual)
const prevTagsRef = React.useRef<Tag[]>(tags); React.useEffect(() => { const prev = prevTagsRef.current; // diff prev vs tags; call onTagAdd/onTagRemove as needed prevTagsRef.current = tags; }, [tags, onTagAdd, onTagRemove]);Also applies to: 185-193, 205-212
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist