Add option to configure max length for question and option text in polls#1500
Add option to configure max length for question and option text in polls#1500martinmitrevski wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthrough
ChangesPoll Character Limit Enforcement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 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. ✨ 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 |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/StreamChatSwiftUI/ChatComposer/Polls/CreatePollViewModel.swift`:
- Around line 137-143: The asyncAfter block in the updateOption method is
redundant because the value is already clamped and assigned immediately on the
line before the conditional. Remove the entire if maxOptionLength != nil block
containing the DispatchQueue.main.asyncAfter call, as the synchronous assignment
of the clamped value is sufficient and matches the behavior expected by
test_updateOption_truncatedToMaxOptionLength. This also eliminates the potential
bug where the captured index could become stale if options are reordered or
removed before the delayed closure executes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 067cf1db-d583-489d-8af4-e044a4ad11e6
📒 Files selected for processing (4)
CHANGELOG.mdSources/StreamChatSwiftUI/ChatComposer/Polls/CreatePollViewModel.swiftSources/StreamChatSwiftUI/ChatMessageList/Polls/PollsConfig.swiftStreamChatSwiftUITests/Tests/ChatChannel/CreatePollViewModel_Tests.swift
| let value = clamped(value, to: maxOptionLength) | ||
| options[index].text = value | ||
| if maxOptionLength != nil { | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { [weak self] in | ||
| self?.options[index].text = value | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove the redundant asyncAfter assignment.
The value is already clamped and assigned immediately on line 138. The asyncAfter block (lines 139-143) schedules another assignment of the same clamped value, which is unnecessary. Unlike the question property's didSet observer where asyncAfter is needed to handle SwiftUI binding cycles, updateOption is directly called and can clamp synchronously.
The test test_updateOption_truncatedToMaxOptionLength confirms this—it asserts the clamped value immediately without waiting for async completion.
Additionally, the closure captures index by value, which could become stale if options are reordered or removed before the closure executes 0.1 seconds later, potentially writing to the wrong option or crashing.
🔧 Proposed fix
func updateOption(id: UUID, value: String) {
guard let index = options.firstIndex(where: { $0.id == id }) else { return }
let value = clamped(value, to: maxOptionLength)
options[index].text = value
- if maxOptionLength != nil {
- DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { [weak self] in
- self?.options[index].text = value
- }
- }
if index == options.count - 1,
!value.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty {
withAnimation {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let value = clamped(value, to: maxOptionLength) | |
| options[index].text = value | |
| if maxOptionLength != nil { | |
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { [weak self] in | |
| self?.options[index].text = value | |
| } | |
| } | |
| let value = clamped(value, to: maxOptionLength) | |
| options[index].text = value |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/StreamChatSwiftUI/ChatComposer/Polls/CreatePollViewModel.swift`
around lines 137 - 143, The asyncAfter block in the updateOption method is
redundant because the value is already clamped and assigned immediately on the
line before the conditional. Remove the entire if maxOptionLength != nil block
containing the DispatchQueue.main.asyncAfter call, as the synchronous assignment
of the clamped value is sufficient and matches the behavior expected by
test_updateOption_truncatedToMaxOptionLength. This also eliminates the potential
bug where the captured index could become stale if options are reordered or
removed before the delayed closure executes.
SDK Size
|
StreamChatSwiftUI XCSize
|
3 participants
🔗 Issue Links
Resolves https://linear.app/stream/issue/IOS-1698/swiftuistrava-configure-max-length-for-question-and-option-text.
🎯 Goal
Describe why we are making this change.
📝 Summary
Provide bullet points with the most important changes in the codebase.
🛠 Implementation
Provide a detailed description of the implementation and explain your decisions if you find them relevant.
🎨 Showcase
Add relevant screenshots and/or videos/gifs to easily see what this PR changes, if applicable.
🧪 Manual Testing Notes
Explain how this change can be tested manually, if applicable.
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit