Implement export maintenance events #345
Conversation
|
@OmarHegazy93 can you also please add a screenshot for this PR, and add back the heading, be sure not to delete the PR template headings can you also write some more about how this code works, and why it's written the way that it's written, there's several new protocols, what are they for? what do they do? etc? |
There was a problem hiding this comment.
good work @OmarHegazy93 ! I've left a bunch of comments that are mainly around code formatting
If you could update it, that would be great!
If you have any questions, you can comment directly on that thread, as always, otherwise click the resolve button once you've addressed my comment. Once you fix everything, please click the re-review button
| protocol PDFGeneratable { | ||
| func generatePDF() -> URL? | ||
| } |
There was a problem hiding this comment.
what does this protocol do, when it's only implemented in one place?
There was a problem hiding this comment.
I added it so that if we used it in a viewModel, we can mock it, but since it's currently used directly inside ExportOptionsView then yes, no need to have it
Basic-Car-Maintenance/Shared/Dashboard/Views/CarMaintenancePDFGenerator.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/CarMaintenancePDFGenerator.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/DashboardView.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/DashboardView.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/CarMaintenancePDFGenerator.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/CarMaintenancePDFGenerator.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/CarMaintenancePDFGenerator.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/CarMaintenancePDFGenerator.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/CarMaintenancePDFGenerator.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
great work!! thanks for joining my stream for this review!
https://youtube.com/live/I_pXpNRryjI?feature=share
This PR was reviewed live on my YouTube channel
| ShareLink(item: url) { | ||
| VStack { | ||
| Image(uiImage: thumbnail) | ||
| Label("Share", image: SFSymbol.share) |
There was a problem hiding this comment.
| Label("Share", image: SFSymbol.share) | |
| Label("Share", systemImage: SFSymbol.share) |
Should be systemImage, rather than image
| NavigationView { | ||
| VStack(alignment: .leading, spacing: 16) { | ||
| Text("Select the vehicle you want to export the maintenance events for:") |
There was a problem hiding this comment.
this could be a scroll view to be able to accommodate dynamic type sizes
| struct ExportOptionsView: View { | ||
| @Environment(\.dismiss) var dismiss | ||
| @State private var selectedVehicle: Vehicle? | ||
| @State private var isShowingShareSheet = false |
There was a problem hiding this comment.
renamed to isShowingThumbnail because it's not technically showing the share sheet yet, the ShareLink does that
What it Does
How I Tested