Skip to content
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

Bug-Fix issue: 182 Picker in AddMaintainanceView is not working #196

Merged

Conversation

rramirez-dev
Copy link
Contributor

Updated the vehicle picker to user the vehicle id and not the vehicle object

What it Does

How I Tested

  • Add a list of steps to show the functionality of your feature
    For example:
  • Run the application
  • Tap the (+) button
  • See this screen
    etc...
  1. Launch the App
  2. Load the dashboard by navigating to a different tab and then select the dashboard tab again.
  3. Tap the plus sign to add a new event
  4. Enter a title
  5. Select a vehicle that is not the first one on the list
  6. Tap Add

Notes

  • Anything else that should be noted about how you implemented this feature?
  • I filtered the vehicles array based on the id of the vehicle that the user selected. I then get the first item in the array and pass it to the MaintainanceEvent view.

Screenshot

Simulator Screen Recording - iPhone 15 - 2023-10-20 at 21 49 41

Updated the vehicle picker to user the vehicle id and not the vehicle object
Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this work using the Vehicle object as opposed to the id? if not use the id and change the variable to selectedVehicleID
Also there's some indentation that's incorrect, your Xcode should be set to 4 spaces

…object

-As per the documentation in order to allow the picker to user an object the tag object
 has to match the selectedVehicle type
Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind sorry for the confusion, go ahead and use the id, that's how it's done in the AddOdometerReadingView

Also note you don't need the vehicle as Vehicle anymore because of that, and technically with the current way that you fixed this the if !vehicles.isEmpty check wouldn't be needed, because you made the selectedVehicle optional

@rramirez-dev
Copy link
Contributor Author

rramirez-dev commented Oct 23, 2023

So I should remove "vehicle as Vehicle" from the tag and replace it with "vehicle.id".

@mikaelacaron
Copy link
Owner

So I should remove vehicle as Vehicle from the tag and replace it with vehicle.id

Yes because it's not necessary to cast the vehicle as it's own type, the goal is to use the id as the main point of data for the picker, like you had it before

-Per PR review I have refactor the vehicle picker to use the vehicle id instead of the vehicle object.
-Replaced "vehicle as Vehicle) for the text tag and replaced it with vehicle.id
-Removed !vehicle.isEmpty check
Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! Just a small comment and I can merge this

Comment on lines 78 to 82
let event = MaintenanceEvent(title:
title,
date: date,
notes: notes,
vehicle: vehicle)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to push a change to fix the alignment of this

@mikaelacaron mikaelacaron merged commit 7c49357 into mikaelacaron:dev Oct 24, 2023
2 checks passed
@rramirez-dev rramirez-dev deleted the 182-Add-MaintainanceView-Fix branch October 26, 2023 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG - Picker in AddMaintenanceView is not working
2 participants