-
Notifications
You must be signed in to change notification settings - Fork 306
Make Poll Results Dialog Shows All Votes with Pagination Support #6043
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?
Conversation
… for displaying poll results. It handles fetching poll votes with pagination. The following new classes have been added to support this controller: - `PollResultsViewState` to represent the different states of the view (Loading, Content, Error). - `PollResultsViewAction` for user actions like requesting to load more votes. - `PollResultsViewEvent` for events from the controller, such as load errors. Unit tests for `PollResultsViewController` have also been added to ensure its correctness.
…g ModalBottomSheet, which provides similar and improved animation, without the padding issue.
The `PollResultsViewState` is simplified from a sealed interface with `Loading`, `Content`, and `Error` states to a single data class. This change streamlines state management by using boolean flags like `isLoading` and `isLoadingMore`. Error handling is now managed through a separate event channel, and the query limit for fetching poll votes has been increased.
The `PollResultsViewState` is simplified from a sealed interface with `Loading`, `Content`, and `Error` states to a single data class. This change streamlines state management by using boolean flags like `isLoading` and `isLoadingMore`. Error handling is now managed through a separate event channel, and the query limit for fetching poll votes has been increased.
… to the PollResultsViewController
…ogic in the remaining previews.
This change refactors how the `Poll` object is passed to the `PollResultsDialogFragment` to prevent crashes during process recreation. The `Poll` object is now stored in a static map within the companion object, and its ID is passed through the fragment's arguments. Reference counting is used to manage the lifecycle of the poll object in the map, ensuring it's cleared when no longer needed. This avoids serializing the large `Poll` object directly into the arguments bundle.
SDK Size Comparison 📏
|
|
...at-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/ViewModelStore.kt
Show resolved
Hide resolved
.../main/kotlin/io/getstream/chat/android/ui/common/state/messages/poll/PollResultsViewState.kt
Outdated
Show resolved
Hide resolved
...e/src/main/java/io/getstream/chat/android/compose/ui/components/poll/PollViewResultDialog.kt
Outdated
Show resolved
Hide resolved
|
|
||
| item { Spacer(modifier = Modifier.height(16.dp)) } | ||
| if (state.isLoading || state.isLoadingMore) { | ||
| LoadingIndicator(modifier = Modifier.fillMaxSize()) |
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.
Wouldn't this be shown over the list of votes? Is that intentional?
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.
Yes. It is shown for the initial load and the pagination loads.
The idea was not to show only the loading UI and block the user, since we already have an initial poll object (which contains 10 max votes).
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.
Bit strange if you ask me... Maybe this is something to bring up for the upcoming design refresh?
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.
Sure.
What UX would you suggest?
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.
I don't really have any good suggestions... Based on how the pagination works in this case, we cannot really know where to show a loading placeholder, so any assumptions could be wrong.
...o/getstream/chat/android/ui/feature/messages/list/internal/poll/PollResultsDialogFragment.kt
Show resolved
Hide resolved
|
I think the wrong ticket is linked here, should be: AND-953 |


🎯 Goal
Fix poll results dialogs showing only 10 votes when polls have more votes. The
/channels/{type}/{id}/queryendpoint returns a limited number of latest votes inlatest_votes_by_option, while the totalvote_countcan be higher.Closes https://linear.app/stream/issue/AND-953
🛠 Implementation details
PollResultsViewControllerinui-commonto manage poll results state and pagination/polls/{poll_id}/votesendpoint (page size: 25, same as iOS)PollResultsViewModelfor both Compose and XML SDKsPollViewResultDialog(Compose) to use ViewModel withModalBottomSheetfor proper window insetsPollResultsDialogFragment(XML) withNestedScrollViewPaginationHelperfor pagination🎨 UI Changes
xml_before.webm
xml_after.webm
compose_before.webm
compose_after.webm
🧪 Testing