-
-
Notifications
You must be signed in to change notification settings - Fork 148
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
fix: Prevent Users From Deleting All Vehicles #276
fix: Prevent Users From Deleting All Vehicles #276
Conversation
@mikaelacaron I have also added some localization options for Turkish and Russian languages for the Alert I have created. I know English, Azerbaijani, Russian and Turkish languages. Also have limited knowledge in Japanese and French. If any help is needed in localization part, feel free to ask as well! |
@mikaelacaron hey, just checking up on you 😁 anything you need? |
@windrunner21 Thanks! Sorry I haven't had a chance to look at this yet! I'll get to it this weekend |
Sure, looking forward to your comments! Good luck in the meantime! |
@windrunner21 @mikaelacaron This is a suggestion, but I am wondering if we can improve UX with the alert message text as the user might question why we can't delete the last vehicle. I used ChatGPT to quickly brainstorm some message versions and these seemed like the best. "The last vehicle can't be deleted. Please ensure at least one vehicle remains in your account." Those are just suggestions/ideas! I also don't want to add a bunch of last minute work to the PR, especially considering localization efforts needed. |
@Jakayus hey, I agree with you as I was under the same impression while writing this feature. However, the reason I didn't decide to mention it to @mikaelacaron is that I think this feature is temporary, and this restriction may be removed in the future. If not, then it's not a problem for me, let's decide on the improved prompt and I'll add it! Good catch! |
@windrunner21 Thank you for the background! I would say let's go with I think too if we want to use a fancier/friendlier prompt, perhaps @mikaelacaron can weigh in on specific wording. The above is sort of the quickest fix. |
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.
Great work!!
I think this wording is best, I'm going to manually change it and push it to your branch
The last vehicle can't be deleted. Please add a new vehicle before removing this one.
All good! I'm totally fine with you (and anyone) bringing up any suggestions for how I've written any feature! This issue, the reason for it right now is because if you try to add a maintenance event or odometer reading, without having a vehicle, the row only says "Select a vehicle" without anything else. And because the vehicle doesn't exist, the user would need to figure out that they have to go back to settings to add the vehicle. Adding an onboarding flow #40 prevents this from happening (for this MVP), but the user can still delete vehicles, so the overall issue still exists. #202 prevents the user from deleting all vehicles so this isn't a problem Later on after the 1st version is shipped I'm thinking of a longer term solution to this issue |
Oh also @windrunner21 when adding new features, don't worry about not being able to translate everything into all the different languages this app supports (if you do know a language the app supports, like Russian) you can go ahead and add the translation for the new string If you would like to add a new language that isn't supported yet, you can comment on #7 what language you'd like to add, and then open a PR with only those changes |
@mikaelacaron I think we can display "Add vehicle" button instead of "Select a vehicle" row in this user story, in order to make it clear for the end user to add their vehicle first. If you think that this solutions makes sense, I can work on it if you'd like. |
This doesn't make sense right now because the user will try to tap on it, and still nothing happens If you want to see this, in your simulator do "erase all settings" and then run the app, and you'll see |
What it Does
How I Tested
Notes
Screenshot
Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2023-11-30.at.16.48.08.mp4
n recording: