Add configurable character limits for poll creation#6429
Add configurable character limits for poll creation#6429ryanhurststrava wants to merge 3 commits into
Conversation
Add optional titleCharLimit and answerCharLimit parameters to CreatePollDialogFragment.newInstance() to allow developers to enforce character limits on poll titles and answer options during poll creation. The limits are passed via Bundle arguments and applied using InputFilter.LengthFilter. Both parameters default to null, meaning no character limit is enforced by default, maintaining backward compatibility. Usage: CreatePollDialogFragment.newInstance( listener, titleCharLimit = 100, answerCharLimit = 50 )
WalkthroughThis PR adds optional character limit constraints to poll creation dialogs. The ChangesPoll Creation UI Character Limits
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
VelikovPetar
left a comment
VelikovPetar
left a comment
There was a problem hiding this comment.
Hi @ryanhurststrava,
Thank you for the contribution!
I've left several remarks concerning breaking changes in the public API, which we are not allowed to make, unless it is a major release.
I have an additional question: The changes that you made, indicate that you directly call CreatePollDialogFragment.newInstance from your code - can you confirm that my assumption is correct?
Otherwise, there is no other public way to inject these values, which means that if you are using the XML SDK as-is (without intercepting the poll click), it will not be possible to customise these values.
| .setCreatePollDialogListener(createPollDialogListener) | ||
| public fun newInstance( | ||
| createPollDialogListener: CreatePollDialogListener, | ||
| titleCharLimit: Int? = null, |
There was a problem hiding this comment.
Adding the titleChatLimit/answerChatLimit here would also be a breaking change for Java integrations. We should add a JvmOverloads annotation to this method, so that the corresponding overloads are generated for the java code.
I would also rename the parameters to:
questionTextLimitoptionTextLimit
To be better aligned with the class naming.
| import io.getstream.chat.android.ui.utils.extensions.streamThemeInflater | ||
|
|
||
| public class OptionsAdapter( | ||
| private val answerCharLimit: Int?, |
There was a problem hiding this comment.
As the OptionsAdapter is a public facing API, adding a mandatory argument at the start of the constructor would be a breaking change as customers who potentially instantiate the OptionsAdapter manually, would encounter a compilation error when updating the SDK.
In order to prevent a breaking both for Java and Kotlin consumers of the API, we would need to introduce a secondary constructor:
// Extend the default constructor with the new parameter
public class OptionsAdapter(
private val optionTextLimit: Int?,
private val onOptionChange: (id: Int, text: String) -> Unit,
) : ListAdapter<PollAnswer, OptionsAdapter.OptionViewHolder>(OptionDiffCallback) {
// Introduce the new constructor, mimicking the original, primary constructor signature
/**
* Builds an [OptionsAdapter] instance without providing option text limit.
*
* @param onOptionChange Callback invoked when the option text changes.
*/
public constructor(onOptionChange: (id: Int, text: String) -> Unit) : this(null, onOptionChange)
// ....
}I would also rename the answerCharLimit to optionTextLimit, to be a bit more aligned with the class name.
yeah this is correct 👍 |
Improvements: - Rename parameters for better alignment with class naming: * titleCharLimit -> questionTextLimit * answerCharLimit -> optionTextLimit - Add @jvmoverloads annotation to CreatePollDialogFragment.newInstance() to prevent breaking changes for Java integrations - Add secondary constructor to OptionsAdapter for backward compatibility, maintaining original signature without optionTextLimit parameter This ensures the API remains backward compatible for both Kotlin and Java consumers while adding the new character limit functionality.
ryanhurststrava
force-pushed
the
feature/poll_character_limits
branch
from
0816c63 to
16f339e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollDialogFragment.kt (2)
97-99: ⚡ Quick winPreserve existing question-field filters when adding the length filter
At Line 98, replacing
binding.question.filterscan drop any existing filters configured on the view. Appending is safer.Proposed change
arguments?.getInt(ARG_QUESTION_TEXT_LIMIT)?.takeIf { it > 0 }?.let { limit -> - binding.question.filters = arrayOf(InputFilter.LengthFilter(limit)) + binding.question.filters = binding.question.filters + InputFilter.LengthFilter(limit) }🤖 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 `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollDialogFragment.kt` around lines 97 - 99, The current assignment to binding.question.filters replaces any existing InputFilters and may drop previously configured behavior; instead, read the existing filters from binding.question.filters (nullable), create a new array that contains all existing filters plus the new InputFilter.LengthFilter(limit) constructed from ARG_QUESTION_TEXT_LIMIT, and set that combined array back on binding.question.filters so existing filters are preserved while adding the length limit.
195-198: ⚡ Quick winNormalize invalid limits in the factory to match runtime behavior
At Lines 196-197, any non-null value is persisted, but only values
> 0are actually used later (Lines 62, 97). Centralizing the> 0normalization innewInstanceavoids silently carrying ineffective argument values.Proposed change
public fun newInstance( createPollDialogListener: CreatePollDialogListener, questionTextLimit: Int? = null, optionTextLimit: Int? = null, ): CreatePollDialogFragment { + val normalizedQuestionTextLimit = questionTextLimit?.takeIf { it > 0 } + val normalizedOptionTextLimit = optionTextLimit?.takeIf { it > 0 } return CreatePollDialogFragment().apply { arguments = Bundle().apply { - questionTextLimit?.let { putInt(ARG_QUESTION_TEXT_LIMIT, it) } - optionTextLimit?.let { putInt(ARG_OPTION_TEXT_LIMIT, it) } + normalizedQuestionTextLimit?.let { putInt(ARG_QUESTION_TEXT_LIMIT, it) } + normalizedOptionTextLimit?.let { putInt(ARG_OPTION_TEXT_LIMIT, it) } } }.setCreatePollDialogListener(createPollDialogListener) }🤖 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 `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollDialogFragment.kt` around lines 195 - 198, The factory should normalize invalid (non-positive) limits so only values actually used at runtime are stored; update CreatePollDialogFragment.newInstance to only putInt(ARG_QUESTION_TEXT_LIMIT, it) and putInt(ARG_OPTION_TEXT_LIMIT, it) when the provided questionTextLimit and optionTextLimit are > 0 (currently any non-null is stored), ensuring the arguments match the runtime checks performed in CreatePollDialogFragment (see usages around lines where question/option limits are checked, e.g., the checks at lines 62 and 97).stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/OptionsAdapter.kt (1)
72-75: ⚡ Quick winPreserve existing
EditTextfilters when applying the new limitAt Line 74, assigning a new array replaces any pre-existing filters on
binding.option. Appending the newLengthFilteravoids dropping other constraints.Proposed change
init { optionTextLimit?.let { limit -> - binding.option.filters = arrayOf(InputFilter.LengthFilter(limit)) + binding.option.filters = binding.option.filters + InputFilter.LengthFilter(limit) } }🤖 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 `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/OptionsAdapter.kt` around lines 72 - 75, The init block in OptionsAdapter replaces any existing InputFilters on binding.option by assigning a new array; instead, retrieve the current filters from binding.option.filters (handle null/empty), create a new array that appends the new InputFilter.LengthFilter(limit) to the existing filters, and assign that combined array back to binding.option.filters so existing constraints are preserved.
🤖 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.
Nitpick comments:
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollDialogFragment.kt`:
- Around line 97-99: The current assignment to binding.question.filters replaces
any existing InputFilters and may drop previously configured behavior; instead,
read the existing filters from binding.question.filters (nullable), create a new
array that contains all existing filters plus the new
InputFilter.LengthFilter(limit) constructed from ARG_QUESTION_TEXT_LIMIT, and
set that combined array back on binding.question.filters so existing filters are
preserved while adding the length limit.
- Around line 195-198: The factory should normalize invalid (non-positive)
limits so only values actually used at runtime are stored; update
CreatePollDialogFragment.newInstance to only putInt(ARG_QUESTION_TEXT_LIMIT, it)
and putInt(ARG_OPTION_TEXT_LIMIT, it) when the provided questionTextLimit and
optionTextLimit are > 0 (currently any non-null is stored), ensuring the
arguments match the runtime checks performed in CreatePollDialogFragment (see
usages around lines where question/option limits are checked, e.g., the checks
at lines 62 and 97).
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/OptionsAdapter.kt`:
- Around line 72-75: The init block in OptionsAdapter replaces any existing
InputFilters on binding.option by assigning a new array; instead, retrieve the
current filters from binding.option.filters (handle null/empty), create a new
array that appends the new InputFilter.LengthFilter(limit) to the existing
filters, and assign that combined array back to binding.option.filters so
existing constraints are preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a9faa812-8381-4995-a1f7-630eddbf4750
📒 Files selected for processing (2)
stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollDialogFragment.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/OptionsAdapter.kt
|
update: I need some additional configuration here to support hiding of the comment and suggestion fields when creating a poll. I opened a new pr #6435 with some more customization |
2 participants
🎯 Goal
Add configurable character limits for poll titles and answer options to provide developers with flexibility in controlling input length during poll creation. This addresses use cases where different character limits may be needed based on UI constraints, localization requirements, or business rules.
🛠 Implementation
New Files Created
CreatePollDialogFragment.ktnewInstance()factory method with optionaltitleCharLimitandanswerCharLimitparametersBundlearguments using keysARG_TITLE_CHAR_LIMITandARG_ANSWER_CHAR_LIMITInputFilter.LengthFilteron the question/title EditText fieldCreatePollDialogListenerinterface for handling poll creation and dismissal eventsCreatePollViewModelfor state managementOptionsAdapter.ktanswerCharLimitparameter that's applied to each option EditTextInputFilter.LengthFilterfor answer options when limit is specifiedDiffUtilfor efficient list updatesKey Design Decisions
titleCharLimitandanswerCharLimitdefault tonull, meaning no character limit is enforced unless explicitly specifiedInputFilter.LengthFilterfor enforcing limits at the input levelUsage Example
📱 UI Changes
Character limits are enforced at the input level using InputFilter, preventing users from exceeding the specified length.
1778192411.mp4
✅ Testing
Manual Testing Steps
titleCharLimit = 100- verify title input stops at 100 charactersanswerCharLimit = 50- verify each answer option stops at 50 charactersExpected Behavior
InputFilter.LengthFilterprevents input beyond the limitContributor Checklist
developbranchReviewer Checklist
🎉 GIF
Screenshots and demo to be added
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
Summary by CodeRabbit