5 Edit Vehicle #269
5 Edit Vehicle #269
Conversation
- add EditVehicleEvent
- add Observable to EditVehicleView.
There was a problem hiding this comment.
You're almost there!! I have several comments that need resolved, which you know from our DMs. If you have any questions, you can comment directly on that thread, otherwise click the resolve button once you've addressed my comment. Once you fix everything, please click the re-review button
| @@ -1518,7 +1518,7 @@ | |||
| } | |||
| }, | |||
| "Edit" : { | |||
| "comment" : "Button label to edit this maintenance", | |||
| "comment" : "Button label to edit this vehicle\nButton label to edit this maintenance", | |||
There was a problem hiding this comment.
This is a thing I recently learned, you can't have two comments for a single key
So for this you'd need to make a new key like EditVehicle and then you'd write the commend, and in the actual Localizable.xcstrings file you'd write just Edit
| } | ||
| } | ||
|
|
||
| AnalyticsService.shared.logEvent(.maintenanceUpdate) |
There was a problem hiding this comment.
This is not a maintenanceUpdate this is a vehicleUpdate
| @@ -88,6 +90,30 @@ final class SettingsViewModel { | |||
| } | |||
| } | |||
|
|
|||
| ///Updates users vehicle that is being edited. Fetches selected vehicle from Firestore and saves to that selected vehicle. | |||
| func updateEvent(_ editVehicleEvent: EditVehicleEvent) async { | |||
There was a problem hiding this comment.
You shouldn't be making a new model EditVehicleEvent, you should just be using Vehicle
|
|
||
| AnalyticsService.shared.logEvent(.maintenanceUpdate) | ||
|
|
||
| await self.getVehicles() |
There was a problem hiding this comment.
You don't need the self it's implied
| } label: { | ||
| Text("Update") | ||
| } | ||
| .disabled(name.isEmpty) |
There was a problem hiding this comment.
Follow the same rules as isVehicleValid in the AddVehicleView
| @State private var selectedVehicleEvent: EditVehicleEvent? | ||
| @State private var isEditingVehicle = false | ||
| @State var editViewModel: EditVehicleView? | ||
| @State private var vehicleToEdit: Vehicle? |
There was a problem hiding this comment.
You only need the vehicleToEdit, you shouldn't be using a separate model for editing, so EditVehicleEvent is not needed
Also don't call this editViewModel is not needed
| @@ -124,9 +129,21 @@ struct SettingsView: View { | |||
| } label: { | |||
| Text("Delete", comment: "Label to delete a vehicle") | |||
| } | |||
| Button { | |||
| isEditingVehicle = true | |||
There was a problem hiding this comment.
Here is where you would put vehicleToEdit = vehicle, because this is in the ForEach loop, and it's specifying which vehicle is going to show in the EditVehicleView
| .sheet(isPresented: $isEditingVehicle) { | ||
| EditVehicleView( | ||
| selectedEvent: $selectedVehicleEvent, viewModel: viewModel) |
There was a problem hiding this comment.
Here we'd pass the vehicleToEdit because we are using the Vehicle model, not the new one you created
mikaelacaron
changed the base branch from
dev
to
fix-vehicle-event-connection
There was a problem hiding this comment.
Hey @TratonGossink I'm going to go ahead and merge this into fix-vehicle-event-connection which is something I'm already working on, because I'm redoing the current Firestore schema
Thanks for all your work!
mikaelacaron
merged commit a6f96fa
into
mikaelacaron:fix-vehicle-event-connection
What it Does
How I Tested
*Run the application.
*Tap "Add Vehicle"
*Create new mock-up vehicle
*Swip to view edit option
Notes
Screenshot