-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Subscriber sort order #21995
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
Subscriber sort order #21995
Conversation
Generated by 🚫 Danger |
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.
Pull Request Overview
This PR adds ascending and descending sort order options to the subscriber list dropdown and wires them through the view models and UI layers.
- Introduces two new string resources for “Ascending” and “Descending.”
- Extends
DataViewViewModel
andSubscribersViewModel
to track and apply sort order. - Updates
SubscribersActivity
andDataViewScreen
composables to render and handle the new sort-order menu items.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
strings.xml |
Added string resources for “ascending” and “descending.” |
SubscribersViewModel.kt |
Implemented getDefaultSort() override. |
SubscribersActivity.kt |
Passed currentSortOrder and onSortOrderClick to the UI. |
DataViewViewModel.kt |
Introduced _sortOrder state, sortOrder flow, and onSortOrderClick . |
DataViewScreen.kt |
Created SortDropdownMenuButton with ascending/descending items and updated previews. |
Comments suppressed due to low confidence (2)
WordPress/src/main/java/org/wordpress/android/ui/dataview/DataViewViewModel.kt:189
- The new
onSortOrderClick
logic and the associatedsortOrder
state changes aren't covered by existing unit tests. Consider adding tests to verify state updates and thatfetchData()
is called.
fun onSortOrderClick(order: WpApiParamOrder) {
WordPress/src/main/java/org/wordpress/android/ui/dataview/DataViewScreen.kt:318
- In the descending
DropdownMenuItem
handler, ensure thatmenuExpanded = false
is executed inside theonClick
lambda so the menu closes after selection, matching the ascending behavior.
onClick = {
* Dropdown menu button for displaying a list of filter [DataViewDropdownItem]s | ||
*/ | ||
@Composable | ||
private fun FilterDropdownMenuButton( | ||
filters: List<DataViewDropdownItem>, | ||
currentFilter: DataViewDropdownItem?, | ||
onFilterClick: (DataViewDropdownItem) -> Unit, | ||
) { | ||
var menuExpanded by remember { mutableStateOf(false) } | ||
IconButton( | ||
onClick = { | ||
menuExpanded = !menuExpanded | ||
}, | ||
modifier = Modifier | ||
.size(48.dp) | ||
) { | ||
Icon( | ||
imageVector = ImageVector.vectorResource(id = R.drawable.ic_filter_list_white_24dp), | ||
contentDescription = stringResource(id = R.string.filter), | ||
tint = MaterialTheme.colorScheme.primary | ||
) | ||
DropdownMenu( | ||
expanded = menuExpanded, | ||
onDismissRequest = { | ||
menuExpanded = false | ||
} | ||
) { | ||
DropdownItems( | ||
titleRes = R.string.filter, | ||
items = filters, | ||
currentItem = currentFilter, | ||
onItemClick = { item -> | ||
onSortClick(item) | ||
onFilterClick(item) | ||
menuExpanded = false | ||
} | ||
) | ||
} | ||
} |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr21995-c437bcf | |
Commit | c437bcf | |
Direct Download | jetpack-prototype-build-pr21995-c437bcf.apk |
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr21995-c437bcf | |
Commit | c437bcf | |
Direct Download | wordpress-prototype-build-pr21995-c437bcf.apk |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #21995 +/- ##
=======================================
Coverage 39.02% 39.02%
=======================================
Files 2153 2153
Lines 101494 101494
Branches 15585 15585
=======================================
Hits 39613 39613
Misses 58384 58384
Partials 3497 3497 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
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.
Looks good!!
Just as a suggestion, what about start adding tests for DataViewViewModel
so we prevent breaking the current stuff while adding all these new changes?
I was planning on making tests one of the last things I do, simply because I expect to make changes to that view model as I use it more. But if you think it's important now, I can create the tests in a separate PR. |
Fair enough. It was just a comment. So, feel free to stick with your plan :) |
This PR updates the subscriber list by adding ascending & descending items to the sort dropdown menu. I also removed "Plan" as a sort field to match iOS.
To test:
sort.mp4