feat: Add clickable functionality to Picker items and set visibleItem…#19
feat: Add clickable functionality to Picker items and set visibleItem…#19
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughPicker items are now clickable: clicking a visible item computes its target index and, if not centered, launches a coroutine to animate scrolling the list to center that item. The sample screen call to TimePicker adds Changes
Sequence DiagramsequenceDiagram
actor User
participant Item as Picker Item
participant Picker as Picker Composable
participant Coroutine as CoroutineScope
participant ListState as LazyListState
User->>Item: Click
Item->>Picker: onClick(targetIndex)
Picker->>Picker: compute centerIndex
alt targetIndex != centerIndex
Picker->>Coroutine: launch { animateScrollToItem(targetIndex) }
Coroutine->>ListState: animateScrollToItem(targetIndex)
ListState-->>Coroutine: animation progress
Coroutine-->>User: item reaches center
else targetIndex == centerIndex
Picker-->>User: no-op
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 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.
Pull Request Overview
This pull request enhances the Picker component by adding clickable functionality to individual picker items, allowing users to tap on any visible item to animate-scroll it to the selected position. The changes also update the sample screen to demonstrate 5 visible items in the time picker.
Key Changes:
- Added click handling to
Pickeritems with animated scrolling using coroutines - Implemented proper accessibility with
Role.Buttonand interaction sources - Updated sample screen to display 5 visible items in the 24-hour time picker
- Removed unused navigation imports from the sample screen
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
datetimepicker/src/commonMain/kotlin/com/kez/picker/Picker.kt |
Added clickable modifier to picker items with coroutine-based animated scrolling, accessibility role, and necessary imports |
sample/src/commonMain/kotlin/com/kez/picker/sample/ui/screen/TimePickerSampleScreen.kt |
Removed unused navigation imports and set visibleItemsCount = 5 for the 24-hour time picker |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
datetimepicker/src/commonMain/kotlin/com/kez/picker/Picker.kt (1)
214-229: Well-implemented click-to-select feature!The clickable implementation correctly addresses the previous review concern by disabling clicks on null padding items with
enabled = getItem(index) != null. The index calculation logic is sound:
- Calculates the target scroll position to center the clicked item
- Properly coerces bounds for both infinite and bounded modes
- Skips animation when clicking the already-centered item
The accessibility role and coroutine usage are appropriate.
Optional: Consider adding visual click feedback.
The code sets
indication = null, which removes the default ripple effect. While the scroll animation itself provides feedback, adding a subtle indication could improve the user experience, especially on devices where the animation might be quick or subtle:.clickable( enabled = getItem(index) != null, role = Role.Button, - indication = null, + indication = rememberRipple(), interactionSource = remember { MutableInteractionSource() }, onClick = { ... } )If the current design intentionally avoids visual feedback, this can be ignored.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
datetimepicker/src/commonMain/kotlin/com/kez/picker/Picker.kt(5 hunks)
🔇 Additional comments (2)
datetimepicker/src/commonMain/kotlin/com/kez/picker/Picker.kt (2)
4-4: LGTM! Necessary imports for clickable functionality.All imports are appropriate for implementing the click-to-select feature. Each import serves a clear purpose in the implementation.
Also applies to: 6-6, 25-25, 30-30, 49-49
101-101: LGTM! Correct coroutine scope pattern.Using
rememberCoroutineScope()is the appropriate way to launch coroutines from a composable function. The scope will be automatically cancelled when the composition leaves the tree.
close #18
This pull request enhances the
Pickercomponent in thedatetimepickermodule by making picker items clickable, allowing users to jump directly to a selected item. It also updates the sample screen to display more visible items in the time picker. The most important changes are grouped below:Picker component improvements:
Modifier.clickableto each picker item inPicker.kt, enabling users to click on an item to animate-scroll it into the selected position. This uses a coroutine scope to perform the scroll and improves accessibility by setting the role toButton.rememberCoroutineScopeto manage the coroutine for animated scrolling when an item is clicked.Sample screen update:
TimePickerSampleScreento setvisibleItemsCount = 5, making the picker show five items at a time for improved usability and demonstration.Code cleanup:
ScreenShot
Screen_recording_20251112_212042.mp4
Summary by CodeRabbit