-
-
Notifications
You must be signed in to change notification settings - Fork 149
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
feature: Use NavigationSplitView rather than TabView for iPad #6 #279
feature: Use NavigationSplitView rather than TabView for iPad #6 #279
Conversation
@mikaelacaron hi, hope all is well 😄 did you have the chance to look at the PR? |
@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 oh sorry, you are right, I totally forgot about it. Merry Christmas! Have a great holiday 🥳 |
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 have several suggestions and a few things to fix.
Also can you order the functions navigationSplitView
, tabView
, and then selectionContent
There's also a bug, the tab view is saved when you close the app and open it again (for iPhone), it remembers which tab you were on, I'd like that same functionality for the navigation split view
hey @mikaelacaron, sorry for the late reply, still on holidays! I'll update PR with requested changes by Sunday latest. |
@windrunner21 No worries at all! Take vacation! 👍 |
@windrunner21 After you address all the comments, please click the 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 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 😄 |
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 just finished with the bug fix, implemented it in such a way that |
@windrunner21 thanks for working on this! haha just now finally getting around to finishing up the final bits of this project to launch it |
What it Does
NavigationSplitView
rather thanTabView
for iPad #6NavigationSplitView
logic intoMainTabView
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 withTabView
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 dayTabView
andNavigationSplitView
use the same content.How I Tested
NavigationSplitView
navigation - can be toggled to be shown/hidden.TabView
navigation.Notes
Screenshot
Simulator.Screen.Recording.-.iPad.Pro.12.9-inch.6th.generation.-.2023-12-18.at.11.55.01.mp4
Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2023-12-18.at.11.55.50.mp4