-
Notifications
You must be signed in to change notification settings - Fork 1.2k
MBL-2220: Implement parent filter modal for search phase 2 #2399
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
Conversation
75b34cf
to
74a7044
Compare
529c521
to
6e31b02
Compare
9d3a94d
to
92c34eb
Compare
6e31b02
to
7190d55
Compare
@@ -5,8 +5,6 @@ struct FilterCategoryView<T: FilterCategory>: View { | |||
@StateObject var viewModel: FilterCategoryViewModel<T> | |||
|
|||
var onSelectedCategory: ((T?) -> Void)? = nil | |||
var onResults: (() -> Void)? = nil |
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.
The buttons are moving out of FilterCategoryView
and into FilterRootView
, which is why I have all these deletes.
} | ||
} | ||
|
||
private func selectInitialCategory(selectedCategory: T) { |
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.
If you initialized this page with a subcategory, it wasn't reflected in the UI. This fixes it.
struct FilterRootView: View { | ||
@State var filterOptions: SearchFilterOptions |
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.
This was never changed/didn't need to be state.
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.
LGTM! Just a couple of small nits
@ViewBuilder | ||
private func subcategories(for category: T) -> some View { | ||
if self.viewModel.isCategorySelected(category), let subcategories = category.availableSubcategories { | ||
FlowLayout(horizontalSpacing: 8, verticalSpacing: 8, alignment: .leading) { |
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.
Not a blocker at all, but we could consider moving these values into Constants
, same goes for the padding values.
self.selectCategory(category, subcategory: subcategory) | ||
return | ||
} | ||
} |
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.
We could use first(where:)
instead of the manual loop for categories and subcategories lookup:
private func selectInitialCategory(selectedCategory: T) {
if let category = self.categories.first(where: { $0 == selectedCategory }) {
self.selectCategory(category, subcategory: nil)
return
}
for category in self.categories {
if let subcategory = category.availableSubcategories?.first(where: { $0 == selectedCategory }) {
self.selectCategory(category, subcategory: subcategory)
return
}
}
}
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 like that better, thanks!
7190d55
to
f954a8d
Compare
📲 What
Implement the main filter page for search phase 2 🎉
