Basic-Car-Maintenance icon indicating copy to clipboard operation
Basic-Car-Maintenance copied to clipboard

Published 20 hours ago • verified publisher icon mikaelacaron

feature: Use NavigationSplitView rather than TabView for iPad #6

Open windrunner21 opened this issue 1 year ago • 9 comments

What it Does

  • Closes #6
  • I have added NavigationSplitView logic into MainTabView struct. This addition is mutating only iPad version right now (completely works) and should affect MacOS version as well in the future (MacOS isn't building as of now). iPhone proceeds with TabView navigation. I have also refactored the code - added extension to hold @ViewBuilder views for better reusability and more readable code - because at the end of the day TabView and NavigationSplitView use the same content.

How I Tested

  • I have tested this new feature on Previews first and later on Simulators.
  • Tested on iPhone 15 Pro Max - 17.0.1
  • Tested on iPad Pro 12.9 inch - 17.0.1
  • iPad shows NavigationSplitView navigation - can be toggled to be shown/hidden.
  • iPhone shows TabView navigation.

Notes

  • There is a comment I have left in the code that TabSelection enum can be changed to conform to String rather than Int. If needed can be easily swiped out, currently is left as is.

Screenshot

  • Two screen recordings - iPad and iPhone.

https://github.com/mikaelacaron/Basic-Car-Maintenance/assets/18750749/7eda95ac-3913-426c-b3a7-eb210ba943b9

https://github.com/mikaelacaron/Basic-Car-Maintenance/assets/18750749/d78bad15-0f16-486e-bf8c-3a3612f3d982

windrunner21 avatar Dec 18 '23 08:12 windrunner21

@mikaelacaron hi, hope all is well 😄 did you have the chance to look at the PR?

windrunner21 avatar Dec 25 '23 11:12 windrunner21

@windrunner21 No I haven't, and it's the holidays so I'm not going to be looking at this today. You'll see my review when I have time

mikaelacaron avatar Dec 25 '23 13:12 mikaelacaron

@mikaelacaron oh sorry, you are right, I totally forgot about it. Merry Christmas! Have a great holiday 🥳

windrunner21 avatar Dec 25 '23 15:12 windrunner21

hey @mikaelacaron, sorry for the late reply, still on holidays! I'll update PR with requested changes by Sunday latest.

windrunner21 avatar Jan 04 '24 20:01 windrunner21

@windrunner21 No worries at all! Take vacation! 👍

mikaelacaron avatar Jan 05 '24 00:01 mikaelacaron

@windrunner21 After you address all the comments, please click the re-review button

Re-review button

Also if you fix something I mentioned and don't have any questions, go ahead and click the Resolve button, but if you have a question, you can comment on that question directly

mikaelacaron avatar Jan 06 '24 20:01 mikaelacaron

@mikaelacaron I didn't click it yet because I still have to solve the bug with reopening the same tab after relaunching application. I wanted to solve everything and then request review 😄

windrunner21 avatar Jan 06 '24 20:01 windrunner21

Yup! You should resolve everything for sure! I just mentioned this cause I got notified you were working on it haha

And I saw that I didn't mention it in my first review

mikaelacaron avatar Jan 06 '24 20:01 mikaelacaron

@mikaelacaron just finished with the bug fix, implemented it in such a way that TabView / NavigationSplitView are having the same last opened tab.

windrunner21 avatar Jan 07 '24 07:01 windrunner21

@windrunner21 thanks for working on this! haha just now finally getting around to finishing up the final bits of this project to launch it

mikaelacaron avatar Jul 08 '24 21:07 mikaelacaron