-
Notifications
You must be signed in to change notification settings - Fork 119
Bookings: Update selector for service/event and customer name filters #16278
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
|
|
|
Hi @itsmeichigo, There might be something I'm missing, but I could not get the selections dismiss working. |
|
Thanks @adborbas for the feedback!
I just updated all the filters for bookings in this PR to not dismiss the selector immediately upon selection. This will keep the behavior consistent with the static options as well (attendance status, booking status). Also we'll need to support multiple selections, but that will be handled later. So for now, one would need to either tap < arrow to add more filter or tap Show bookings to apply the filters.
I updated the sort descriptor in fb76687 to fix this.
I asked Wagner in lxixW6aBBNkCd9ooBK70Z1-fi-1421_3698#1478520395 - I'll update this later when I get a response from him. |
@itsmeichigo Ok, thanks for the clarification! This makes sense now. In this case I think the "Testing steps" needs some adjustment cause it says:
I'd expect them to say not dismiss the view. I'll check the code now. |
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 work on this 👍 Had one small comment.
| /// | ||
| public struct BookingProductFilter: Codable, Hashable { | ||
| /// The underlying product | ||
| public let product: Product |
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.
One small concern — I wonder if we really need to store the entire Product here. Having both product, id, and name feels a bit redundant and might make it easier for other parts of the code to start depending on BookingProductFilter for full product data instead of just treating it as a lightweight filter value.
Would it make sense for this type to only keep the minimal info needed for filtering (e.g. id and name) and drop the product property? That would keep the model focused on its purpose and avoid any unintended coupling later on.
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 is a valid point! I added an update in b6ed021 with the following changes:
- Removed the full product model from
BookingProductFilter. - Updated
SyncableListSelectorViewto accept a closure for checking initial selection rather than comparing full models. - Updated
FilterListViewControllerfollowing the selection logic change.
…kingProductFilter
My bad, I updated the code but forgot to update the PR description. I updated it to avoid confusion in case someone else visits the PR. I just addressed the issue you found above, will enable automerge now to unblock my other PR. Please feel free to leave comments if any, I'll address them in subsequent PRs. Thanks! |




Part of WOOMOB-1240
Description
This PR updates the selector for Service/Event and Customer name filters for bookings:
ListSyncableto support showing selector for bookable products (named as Service/Event on the UI).Testing steps
Testing information
Screenshots
Simulator.Screen.Recording.-.iPhone.17.-.2025-10-27.at.14.34.02.mov
RELEASE-NOTES.txtif necessary.