-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feature: improved bullets functionality in message composer #6810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
feature: improved bullets functionality in message composer #6810
Conversation
WalkthroughThe changes introduce text trimming in MessageComposer to strip leading/trailing whitespace from input, add auto-bullet formatting functionality in ComposerInput with state management via Changes
Sequence DiagramsequenceDiagram
participant User
participant ComposerInput
participant handleAutoBullet
participant MessageComposer
participant Backend
User->>ComposerInput: Type text + press Enter
ComposerInput->>ComposerInput: onChangeText triggered
ComposerInput->>handleAutoBullet: newText passed
handleAutoBullet->>handleAutoBullet: Detect previous line type<br/>(numbered/hyphen/none)
handleAutoBullet->>handleAutoBullet: Auto-insert bullet if applicable
handleAutoBullet-->>ComposerInput: Return formatted text
ComposerInput->>ComposerInput: setInput(newText)<br/>setInputValue(newText)
ComposerInput->>ComposerInput: Update TextInput value
ComposerInput->>MessageComposer: Pass final text
MessageComposer->>MessageComposer: Apply .trim()
MessageComposer->>Backend: Send trimmed message
Backend-->>User: Message delivered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/containers/MessageComposer/MessageComposer.tsx(1 hunks)app/containers/MessageComposer/components/ComposerInput.tsx(5 hunks)app/containers/MessageComposer/interfaces.ts(1 hunks)
🔇 Additional comments (5)
app/containers/MessageComposer/interfaces.ts (1)
20-25: LGTM! Clear documentation added.The JSDoc comment effectively documents the
TSetInputparameters and their behavior, improving code maintainability.app/containers/MessageComposer/components/ComposerInput.tsx (3)
171-192: Verify the removal of trim logic.The commented line 172 shows that previously
text.trim()was applied conditionally. Now,setInputuses raw text without trimming. While trimming is now performed inMessageComposer.tsx(line 94) before sending, ensure that intermediate text states (drafts, autocomplete, etc.) work correctly with leading/trailing whitespace.
231-236: LGTM! Auto-bullet integration looks correct.The
onChangeTexthandler properly integrates thehandleAutoBulletfunction and maintains consistency acrosstextRef, debounced callback, andsetInput.
403-403: Controlled TextInput now properly bound to state.The TextInput is now correctly controlled via
value={inputValue}, which synchronizes with the internal state management.app/containers/MessageComposer/MessageComposer.tsx (1)
94-94: No issues found—trim is safe and verified.The
.trim()on line 94 is intentional and correct. Edit action tests explicitly expect trimmed text (expect(editRequest).toHaveBeenCalledWith({ id, msg: 'Message messageId', rid: 'rid' })), confirming this is the expected behavior. For quote actions, passing trimmed text toprepareQuoteMessageis appropriate and prevents sending unnecessary whitespace. The centralized trim at line 94 is cleaner than the previous scattered trim calls and aligns with how slash commands are handled (line 115).
|
|
||
| export const ComposerInput = memo( | ||
| forwardRef<IComposerInput, IComposerInputProps>(({ inputRef }, ref) => { | ||
| const [inputValue, setInputValue] = useState(''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the controlled input behavior.
Converting the TextInput from uncontrolled to controlled by introducing the inputValue state is a significant change. Ensure that this doesn't introduce issues with cursor positioning, text selection, or input performance, especially for longer messages.
Run the following script to check if there are any related issues or tests that might be affected:
🏁 Script executed:
#!/bin/bash
# Description: Search for tests or usage patterns related to ComposerInput text handling
# Search for test files related to ComposerInput
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts | xargs rg -l "ComposerInput" -C 2
# Search for places where inputRef is used with setNativeProps
rg -n "inputRef.*setNativeProps" -A 3 -B 3
# Search for potential issues with controlled inputs
rg -n "value=\{.*\}" app/containers/MessageComposer -A 2 -B 2Length of output: 4333
is NOT appropriate here as we need repository files
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full context of setInput function and surrounding code
sed -n '171,200p' app/containers/MessageComposer/components/ComposerInput.tsx
# Find all setInputValue calls in the file
rg -n "setInputValue" app/containers/MessageComposer/components/ComposerInput.tsx -B 2 -A 2
# Find all setNativeProps calls in the file
rg -n "setNativeProps" app/containers/MessageComposer/components/ComposerInput.tsx -B 3 -A 3
Length of output: 302
🏁 Script executed:
#!/bin/bash
# Find the actual file
fd "ComposerInput.tsx" -type f
# Get content using find + cat
fd "ComposerInput.tsx" -type f -exec cat {} \;Length of output: 341
🏁 Script executed:
#!/bin/bash
# Find the file with correct fd syntax
fd "ComposerInput.tsx"
# List files to see structure
find . -name "ComposerInput.tsx" 2>/dev/null
# Try to get lines around setNativeProps and value binding
rg -n "setNativeProps|value=\{inputValue" --no-heading -C 10Length of output: 8444
Remove setNativeProps call when using controlled input with state.
The conversion to controlled input (via inputValue state on line 403) conflicts with the direct native manipulation via setNativeProps on line 182. This mixed pattern creates inconsistency between React state and native layer, causing the exact issues mentioned: cursor positioning, text selection, and performance problems.
In a controlled input, React state is the single source of truth. Remove the setNativeProps call on line 182 and rely solely on the state-driven value binding.
🤖 Prompt for AI Agents
In app/containers/MessageComposer/components/ComposerInput.tsx around line 50
(inputValue state) and line 182 (setNativeProps call) — you have a controlled
input via inputValue but are also calling setNativeProps directly; remove the
setNativeProps call at line 182 so React state is the single source of truth.
Ensure the TextInput's value prop is bound to inputValue and onChangeText/update
handlers update setInputValue; remove any direct native ref manipulation related
to text content or cursor positioning, and if selection must be controlled
manage selection via state and props rather than setNativeProps.
| const handleAutoBullet = useCallback((text: string, prevText: string): string => { | ||
| if (text.endsWith('\n')) { | ||
| const lines = text.split('\n'); | ||
|
|
||
| // check for deletion | ||
| if (text.length < prevText.length) { | ||
| return text; | ||
| } | ||
| const prevLine = lines[lines.length - 2]; | ||
| console.log('prevline', prevLine); | ||
|
|
||
| const regex = /^((?:\d+\.|-\s)).*/; | ||
|
|
||
| if (prevLine && regex.test(prevLine)) { | ||
| if (prevLine.startsWith('- ')) { | ||
| const newText = `${text}- `; | ||
| console.log('new text', newText); | ||
| return newText; | ||
| } | ||
| const prevNumber = parseInt(prevLine.split('.')[0], 10); | ||
| const nextNumber = prevNumber + 1; | ||
| const newText = `${text}${nextNumber}. `; | ||
| console.log('new text', newText); | ||
| return newText; | ||
| } | ||
| } | ||
| return text; | ||
| }, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug console.log statements and fix regex pattern.
The function has several issues:
- Debug logs (lines 211, 218, 224): Remove
console.logstatements before merging to production. - Regex inconsistency (line 213): The pattern
/^((?:\d+\.|-\s)).*/captures-\s(hyphen + space) but line 216 checks for'- '(hyphen + space). The regex should be/^(\d+\.|-\s)/or/^(\d+\.|- )/for consistency. - Missing exit logic: The feature doesn't handle exiting the bullet list when a user presses enter on an empty bullet line (e.g., "- \n" should not add another "- "). Consider adding logic to detect empty bullet lines and stop auto-insertion.
Apply this diff to fix the regex and remove debug logs:
const handleAutoBullet = useCallback((text: string, prevText: string): string => {
if (text.endsWith('\n')) {
const lines = text.split('\n');
// check for deletion
if (text.length < prevText.length) {
return text;
}
const prevLine = lines[lines.length - 2];
- console.log('prevline', prevLine);
- const regex = /^((?:\d+\.|-\s)).*/;
+ const regex = /^(\d+\.|-)(\s)/;
if (prevLine && regex.test(prevLine)) {
if (prevLine.startsWith('- ')) {
+ // Don't add bullet if previous line is empty bullet
+ if (prevLine.trim() === '-') {
+ return text.slice(0, -1); // Remove the newline, exit bullet mode
+ }
const newText = `${text}- `;
- console.log('new text', newText);
return newText;
}
const prevNumber = parseInt(prevLine.split('.')[0], 10);
const nextNumber = prevNumber + 1;
const newText = `${text}${nextNumber}. `;
- console.log('new text', newText);
return newText;
}
}
return text;
}, []);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/containers/MessageComposer/components/ComposerInput.tsx around lines 202
to 229, remove all debug console.log statements and fix the regex and exit
logic: replace the regex with a consistent pattern like /^(\d+\.|- )/ (or
/^(\d+\.|- )/), remove console.log calls at lines ~211, ~218, ~224, and add
handling to detect empty bullet lines (e.g., if prevLine matches /^- $/ or
/^\d+\. $/ or prevLine.trim() === '-' then do not auto-insert another bullet and
return text unchanged) so pressing Enter on an empty bullet line exits the list;
keep the existing numeric increment logic otherwise.
will update description upon completion
Proposed changes
Issue(s)
#6780
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.