8000 MBL-2220: Implement parent filter modal for search phase 2 by amy-at-kickstarter · Pull Request #2399 · kickstarter/ios-oss · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 2 commits into from
Apr 25, 2025

Conversation

amy-at-kickstarter
Copy link
Contributor
@amy-at-kickstarter amy-at-kickstarter commented Apr 23, 2025

📲 What

Implement the main filter page for search phase 2 🎉
search-filters-phase-2-final

@amy-at-kickstarter amy-at-kickstarter force-pushed the feat/adyer/mbl-2220/modal branch from 75b34cf to 74a7044 Compare April 24, 2025 15:40
@amy-at-kickstarter amy-at-kickstarter changed the base branch from main to tmp/adyer/2220-base April 24, 2025 15:41
@amy-at-kickstarter amy-at-kickstarter force-pushed the feat/adyer/mbl-2220/modal branch 3 times, most recently from 529c521 to 6e31b02 Compare April 25, 2025 14:21
@amy-at-kickstarter amy-at-kickstarter changed the base branch from tmp/adyer/2220-base to stevestreza/search/subcats2 April 25, 2025 14:21
@amy-at-kickstarter amy-at-kickstarter force-pushed the stevestreza/search/subcats2 branch from 9d3a94d to 92c34eb Compare April 25, 2025 15:48
@amy-at-kickstarter amy-at-kickstarter force-pushed the feat/adyer/mbl-2220/modal branch from 6e31b02 to 7190d55 Compare April 25, 2025 15:54
@amy-at-kickstarter amy-at-kickstarter changed the title Feat/adyer/mbl 2220/modal MBL-2220: Implement parent filter modal for search phase 2 Apr 25, 2025
@amy-at-kickstarter amy-at-kickstarter marked this pull request as ready for review April 25, 2025 15:59
@@ -5,8 +5,6 @@ struct FilterCategoryView<T: FilterCategory>: View {
@StateObject var viewModel: FilterCategoryViewModel<T>

var onSelectedCategory: ((T?) -> Void)? = nil
var onResults: (() -> Void)? = nil
Copy link
Contributor Author

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) {
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

Base automatically changed from stevestreza/search/subcats2 to main April 25, 2025 17:55
Copy link
Contributor
@jovaniks jovaniks left a 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) {
Copy link
Contributor

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
}
}
Copy link
Contributor
@jovaniks jovaniks Apr 25, 2025

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
    }
  }
}

Copy link
Contributor Author

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!

@amy-at-kickstarter amy-at-kickstarter force-pushed the feat/adyer/mbl-2220/modal branch from 7190d55 to f954a8d Compare April 25, 2025 19:48
@amy-at-kickstarter amy-at-kickstarter merged commit a217210 into main Apr 25, 2025
5 checks passed
@amy-at-kickstarter amy-at-kickstarter deleted the feat/adyer/mbl-2220/modal branch April 25, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0