-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/filter by associations #368
Conversation
268d62e
to
27e3e31
Compare
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.
Thank you for making the changes. Have added some minor suggestions for the code; ready to be merged nonetheless.
!_filtersContainer.value.classChecked || | ||
_semester.value == "" || | ||
event.tags.any { tag -> | ||
tag.name.lowercase() == _semester.value.lowercase() | ||
} | ||
} | ||
.filter { event -> | ||
// filter by association | ||
_selectedAssociation.value == -1 || |
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.
Knitpick: -1 can be defined as a private constant and then used here instead of directly using the value.
app/src/main/java/com/github/swent/echo/compose/components/searchmenu/SearchMenuFilters.kt
Show resolved
Hide resolved
@@ -310,7 +332,8 @@ constructor( | |||
* the default one, the search mode is on. | |||
*/ | |||
private fun updateSearchMode() { | |||
_searchMode.value = _filtersContainer.value != defaultFiltersContainer | |||
_searchMode.value = | |||
_filtersContainer.value != defaultFiltersContainer || _selectedAssociation.value != -1 |
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.
Again, the magic value -1 can be used in as a constant.
6da05cf
to
5ad9f78
Compare
Quality Gate passedIssues Measures |
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.
Nice! Thank you for the changes and explanations!
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.
Ready to be merged
make it possible to filter by an association you follow through the dropdown menu in the search sheet