Implement CSV export of maintenance events #371
Conversation
|
awesome work!! I'll look at this PR over the weekend |
There was a problem hiding this comment.
Great work!!! Thanks for contributing!
You added more complexity in the csv than I would've just for exporting the bool in different ways and the date, I would've only done one option, and made it not as generic, but that's fine! It'll be more flexible in the future how you've written it
One small thing, that I'll make a new issue out of later is that the generateCSVFile function in CSVGeneratorView gets run multiple times, because the function is really called when the view body is evaluated, which can happen many times. A better way to implement this is to make that function trigger from a button tap. But no worries I'll add that as an issue later
|
|
||
| import Foundation | ||
|
|
||
| internal extension BidirectionalCollection where Element == String { |
There was a problem hiding this comment.
this doesn't need to be marked internal cause it is by default
| var newlineDelimited: String { joined(separator: "\r\n") } | ||
| } | ||
|
|
||
| public struct CSVColumn<Record> { |
There was a problem hiding this comment.
this shouldn't be public the "Shared" folder that this is in, is still all in a single project, so it's fine if it stays just internal
Same with like the whole file
| struct ExportOptionsView: View { | ||
| @Environment(\.dismiss) var dismiss | ||
| @State private var selectedVehicle: Vehicle? | ||
| @State private var isShowingThumbnail = false | ||
| @State private var pdfDoc: PDFDocument? | ||
| @State private var showingErrorAlert = false | ||
| @State private var selectedOption: ExportOption? | ||
| @State private var showingExporter = false |
There was a problem hiding this comment.
| @State private var showingExporter = false | |
| @State private var showingCSVGeneratorView = false |
renaming this, and how the variable is used is this file, cause I think it feels more clear cause "showExporter" also feels like it would be used for the PDF, but it isn't
| Button("Export") { | ||
| Menu(content: { | ||
| Picker("Export", selection: $selectedOption) { | ||
| ForEach(ExportOption.allCases, id: \.self) { option in |
There was a problem hiding this comment.
| ForEach(ExportOption.allCases, id: \.self) { option in | |
| ForEach(ExportOption.allCases) { option in |
The id: \.self isn't needed because ExportOption conforms to Identifiable
Also for the Menu I use multiple trailing closure syntax, as a personal preference
| isShowingThumbnail = true | ||
| switch selectedOption { | ||
| case .pdf: | ||
| selectedOption = nil |
There was a problem hiding this comment.
I understand why this is being set to nil, but it's not my favorite way to implement this, maybe we could set it to nil on the dismiss of the view that's shown?
but this works! so I'm not too worried about it
|
|
||
| import Foundation | ||
|
|
||
| internal extension BidirectionalCollection where Element == String { |
There was a problem hiding this comment.
also moving this file to just the Dashboard folder, cause it doesn't fit being in the Dashboard > Views folder
| let events: [MaintenanceEvent] | ||
| let vehicleName: String | ||
|
|
||
| func csvData() -> String { |
There was a problem hiding this comment.
moving the functions below the body, just as a personal preference thing
and they can be private, because they're only used in this view
| private(set) var attribute: (Record) -> CSVEncodable | ||
|
|
||
| init( | ||
| _ header: String, |
There was a problem hiding this comment.
personal preference, but I don't format this on multiple lines unless it's needed because the line is too long, or there's many parameters (which at that point usually it needs to be refactored for passing too many things
What it Does
How I Tested
Screenshot
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-11-21.at.18.13.47.mov