-
Notifications
You must be signed in to change notification settings - Fork 121
[Bookings][Part 1] Booking details screen #16168
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
Generated by 🚫 Danger |
|
|
238a3d3 to
0336bd1
Compare
itsmeichigo
left a comment
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 👍 I left some nit-picking in the comments.
| import SwiftUI | ||
|
|
||
| /// periphery: ignore - will be used after Booking list is ready | ||
| final class BookingDetailsViewController: UIHostingController<BookingDetailsView> { |
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.
Can we use the BookingDetailsView directly in BookingTabView? If so I think this hosting controller would be redundant.
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.
Removed host vc in 7b45174
| let appointmentDate = Self.appointmentDateFormatter.string(from: booking.startDate) | ||
| let appointmentTimeFrame = [ | ||
| Self.appointmentTimeFrameFormatter.string(from: booking.startDate), | ||
| Self.appointmentTimeFrameFormatter.string(from: booking.endDate) |
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.
Nit: these can be simplified:
| let appointmentDate = Self.appointmentDateFormatter.string(from: booking.startDate) | |
| let appointmentTimeFrame = [ | |
| Self.appointmentTimeFrameFormatter.string(from: booking.startDate), | |
| Self.appointmentTimeFrameFormatter.string(from: booking.endDate) | |
| let appointmentDate = booking.startDate.formatted(date: .numeric, time: .omitted) | |
| let appointmentTimeFrame = [ | |
| booking.startDate.formatted(date: .omitted, time: .shortened), | |
| booking.endDate.formatted(date: .omitted, time: .shortened) |
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.
Done in 1220ac5
| ) | ||
|
|
||
| let appointmentDetailsSection = Section( | ||
| headerText: "Appointment Details".uppercased(), |
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.
Nit: please ensure to localize this.
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.
Added localizations in 9a19487
| let status: [Status] | ||
|
|
||
| init(_ booking: Booking) { | ||
| bookingDate = Self.dateFormatter.string(from: booking.startDate) |
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.
Nit: similar comment as above about formatting date in a simpler way.
| } | ||
|
|
||
| var body: some View { | ||
| RefreshablePlainList(action: { |
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.
Nit: this type seems outdated. Since we're targeting iOS 17 minimum now, please consider using ScrollView with refreshable for simplicity.
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.
Done in aa85786

Part of: WOOMOB-1236
Description
The PR adds the draft for a "Booking details" screen. The added view and view model is currently a skeleton and doesn't yet have a data binding.
The view and view controller is not yet used in the app. To check out the appearance - use the Xcode, navigate to the
BookingDetailsViewfile and open the canvas with a preview.Current PR only implements the header section and the "Appointment details" section. Other sections and data binding will be implemented in the upcoming PRs.
BookingDetailsViewControllerhas hosting view controller forBookingDetailsViewBookingDetailsViewas SwiftUI view carrying the screen contentBookingDetailsViewModelrepresenting the view state of a booking details screen.TitleAndTextFieldRowto have configurable title and value fonts and color. Reuses it for content rows in "Appointment details" section.Steps to reproduce
N/A
Testing information
To check out the current appearance - use the Xcode, navigate to the
BookingDetailsViewfile and open the canvas with a preview.Screenshots
RELEASE-NOTES.txtif necessary.