search

apptekstudios / ASNavigationView

6 stars 1 forks source link

Feature Request: Support Splitview #1

Open tarasis opened 4 years ago

tarasis commented 4 years ago

ASNavigationView works really well but it doesn't support SwiftUI's SplitView.

App looks great on the iPhone, but on the iPad, the detail view appears at the bottom on the main view. Is it possible to add support for split view?

apptekstudios commented 4 years ago

Done 😄 The master branch now uses a split view by default. I'm sure it can use more customisation settings, but won't have time this week to look at it. PR's welcome if you end up adding anything 👍

tarasis commented 4 years ago

Cheers, that's great.

If I get a chance I'll try and add things but my skills are rather rusty right now.

tarasis commented 4 years ago

I have found a couple of issues with it, that I hope I can look into before you get back to it.

a) Doesn't support a default view for the split view like NavigationView, where you can just do NavigationView { MainView() InstructionsView() } Then in MainView you can have the List generating NavigationLink.

At present ASNavigationView sticks the initial DetailView (InstructionsView) below MainView. (text at bottom, should be on the right hand side.

Screenshot 2019-11-28 at 01 50 57

b) Selecting a detail view when in portrait breaks the split view when you rotate back to landscape. The Master view remains off the side of the screen until you tap the < in the navigation bar, then it just overlays the Master and disappears when you tap on a ASNavigationButton.

If you rotate to Portrait and then back to landscape without selecting a detail view then it works perfectly.

apptekstudios commented 4 years ago

I had some spare time so went ahead and fixed those issues 😄

The splitView appearance wasn't getting updated because the size class doesn't change for iPad in landscape... I'm now observing for changes in the view bounds instead to support the expected landscape behaviour 👍

You can now specify a placeholderView like this:

ASNavigationView(placeholderDetailView: Text("Select a view to start"))
{ ... }
apptekstudios commented 4 years ago

Also fixed an issue where pressing a link in the master view again was re-presenting the detail view - it will now do nothing if already open

tarasis commented 4 years ago

laugh That was unexpectedly quick. Thank you!

Had just finished a fix, using NSNotificationCenter to detect the orientation change (per this answer), and then triggering the call to configure(animated).

Was just tidying it up for committing when I noticed your reply :)

Will look in the morning, its 3:45am here. 😨

apptekstudios commented 4 years ago

Was just tidying it up for committing when I noticed your reply :)

Whoops! Sorry to double up on the fix, let me know if the changes have resolved those problems for you 👍

tarasis commented 4 years ago

OMG Apologies, I thought I had responded to this 2 weeks ago.

Yes, your changes appear to have worked well. I did test it out quite heavily, but haven't touched Xcode in 2 weeks.

There was something I wanted to ask but I don't recall just yet what it was.