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

SwiftUI NavigationLink #101

Closed
brianlombardo opened this issue Aug 10, 2021 · 19 comments · Fixed by #116
Closed

SwiftUI NavigationLink #101

brianlombardo opened this issue Aug 10, 2021 · 19 comments · Fixed by #116
Assignees
Labels
enhancement New feature or request

Comments

@brianlombardo
Copy link
Contributor

brianlombardo commented Aug 10, 2021

  • add support for NavLinks as launchStyle on WorkflowView and WorkflowItem

  • Add 10 screen test, beginning to end and back. don't forget to test wiggle/oscillation between 2 screens in the navigation view. what happens if someone hits back on nav view?

  • test abandoning

@brianlombardo brianlombardo self-assigned this Aug 10, 2021
@brianlombardo brianlombardo added this to the SwiftUI Support milestone Aug 10, 2021
@brianlombardo brianlombardo added the enhancement New feature or request label Aug 10, 2021
@Richard-Gist
Copy link
Collaborator

Richard-Gist commented Aug 10, 2021

Copying text over from another Issue:

I want SwiftUI to have launch styles like UIKit so I can put my stuff in a navigation view.

I'd like something like:

WorkflowLauncher(isLaunched: .constant(true))
   .launchStyle(.navigationStack)
   .thenProceed(with: WorkflowItem(FR1.self).launchStyle(.modal()))

Describe alternatives you've considered

For Modals, I can embed them myself in sheets and share the binding with the WorkflowLauncher or set it on the view.

For navigation stacks, I'm not sure what I can do.

@brianlombardo
Copy link
Contributor Author

brianlombardo commented Aug 11, 2021

We are spiking on ways to handle navigation when the WorkflowLauncher is part of (or contains) a NavigationView. We think that we may be able to create a hidden NavigationLink that can be invoked when our lifecycle methods are called (eg. proceedInWorkflow)

@wiemerm
Copy link
Contributor

wiemerm commented Aug 13, 2021

EOD: The spiking continues. Currently we are spiking it out without the context of a user defined launchStyle and instead with an assumed what would be .navigationStack launch style in order to get that side of it settled. We are working through creating a NavigationLink inside of ModifiedWorkflowView that has an empty label associated with it and uses a Binding<Bool> to determine if the destination of the link should be active or not. As of now the destination is not a subsequent FlowRepresentable but is instead just a Text that we made it to the destination as we figure out what triggering changes to this destination will look like within the context of a workflow.

@brianlombardo
Copy link
Contributor Author

brianlombardo commented Aug 16, 2021

Today we got a little bit closer. The desired behavior can now be observed when setting the WorkflowViewModel body as the destination for the NavigationLink. Currently, I am experimenting with ways to introduce the wrapping NavigationView to the first ModifiedWorkflowView in a sequence.

@morganzellers
Copy link
Contributor

morganzellers commented Aug 17, 2021

  • Learned that NavigationLinks will take action on true AND false values from it's isActive as well as matching & non-matching combinations of tag and selection values.
  • Added pure swiftui implementation to ContentView of example to verify behaviors
    • Showed that the pure SwiftUI and workflow view hierarchies differ
  • SwiftUI:
ContentView {
  NavigationView {
    FirstView {
       NavigationLink {
          SecondView {
             NavigationLink {
                ThirdView
             }
          }
       }
    }
  }
}
  • Potential Workflow hierarchy 1:
ContentView {
  NavigationView {
    ModifiedWorkflowView {
      ~~~FirstView/SecondView/ThirdView~~~
      NavigationLink {
        ~~~SecondView/ThirdView~~~
      }
    }
  }
}
  • Potential Workflow hierarchy 1:
ContentView {
  NavigationView {
    ModifiedWorkflowView<ThirdView>(model:0x001) {
      ModifiedWorkflowView<SecondView>(model:0x001) {
        ModifiedWorkflowView<FirstView>(model:0x001) {
        }
      }
    }
  }
}

We also hit a limitation of 9 FR's in the call chain :(

@morganzellers
Copy link
Contributor

Had spotty connection issues most of the day today. Spent some time looking at Combine types trying to figure out how to filter out false values from isLinkActive.

WorkflowViewModel

final class WorkflowViewModel: ObservableObject {
    @Published var isLinkActive: Bool = false
    
    func proceed(to destination: AnyWorkflow.Element, from source: AnyWorkflow.Element) {
        body = extractView(from: destination).erasedView

        isLinkActive.toggle()
    }
}

ModifiedWorkflowView

NavigationLink(destination: innerBody, isActive: $model.isLinkActive) { }

When the NavigationLink gets a false value from $model.isLinkActive, it navigates backwards. Thus, I'm trying to see what would happen if I could do:

NavigationLink(destination: innerBody, isActive: $model.isLinkActive.filter { $0 ==false }) { }

But still poking around the Combine docs to figure out the how.

@Tyler-Keith-Thompson
Copy link
Collaborator

So flip the concept on its head. In a typical SwiftUI view, when you set up a NavigationLink you have the isActive state as false and you know your destination. So then you flip isActive to true and go along your merry way.

In SwiftCurrent land, it's similar but essentially opposite. You have no clue what will come next, so rather than isActive being false and a concrete destination, you can just rebuild the views you need backward. SwiftCurrent knows about all the things that were proceeded, but not what will be.

PROBLEM: SwiftUI has a bug if isActive is defaulted to true on NavLinks. See details here: https://stackoverflow.com/questions/68365774/nested-navigationlinks-with-isactive-true-are-not-displaying-correctly

There are some .... interesting? workarounds posed in the SO answer that we have validated do in fact, work. Now we're trying to continue this concept of defaulting isActive to true and recursively building the back-stack.

@Richard-Gist
Copy link
Collaborator

We're still spinning on this. The big problem we're trying to solve is the conundrum that when you create a NavigationLink you need its destination to also have a navigation link in order to continue the navigation. We want:

FR1 (with NavigationLink to FR2 (with NavigationLink to FR3)) when you navigate to FR3. But what we have is:
FR1 (with NavigationLink to FR2) when you navigate to FR3.

This happens when you are recursing back from FR3, FR3 told FR2 to have a NavigationLink to itself, but when FR2 told FR1 to make a NavigationLink, it conveniently left off its own NavigationLink to FR3.

This is all grossly simplified but covers the basic issue we're battling.

@Tyler-Keith-Thompson
Copy link
Collaborator

Unfortunate reality: SwiftUI NavigationViews have some odd behavior. Let me start with the good news:

  • We managed to do what we said we'd do, we dynamically build the back-stack every time you proceedInWorkflow

Now the bad

  • SwiftUI NavigationViews will not honor changes to state like that until you navigate back to the first view

What that means:

  • You hit proceed and go forward, you hit proceed and nothing happens, you hit back and it automatically transitions you forward to the 3rd screen, you hit back and are on the 2nd screen, you hit back and are on the 1st screen

Next steps:

  • First, we cry
  • Next, we change our fundamental approach. Idea: We don't build the back-stack we only build forward.

@Richard-Gist
Copy link
Collaborator

There are two spike branches to see the work we did today: tyler-richard-nav-link-spike and tyler-richard-nav-link-spike2

@Richard-Gist
Copy link
Collaborator

We solved it and it is so easy.... We just need to fundamentally change how fluent stuff works so that the typing is reversed. Right now we have: ModifiedWorkflowView<Never, ModifiedWorkflowView<Never, ModifiedWorkflowView<Never, Never, FR1>, FR2> FR3> and what we need is: ModifiedWorkflowView<Never, ModifiedWorkflowView<Never, ModifiedWorkflowView<Never, FR3>, FR2> FR1>

With that one change Modals and navigation links and everything is just stupid simple. See: tyler-richard-nav-link-spike

@Tyler-Keith-Thompson
Copy link
Collaborator

By "solved" he means we conceptually proved it, we have no idea how to fundamentally change how the fluent stuff works.

@Richard-Gist
Copy link
Collaborator

Tomato/Potato

@Tyler-Keith-Thompson
Copy link
Collaborator

It turns out Richard and I are not smart enough to change the fundamental rules of Swift (honestly, we may be up against just computer science issues regardless of language). SO we came up with the next best thing. Notice that SwiftUI has the desired effect with its views, it does this by composition and wrapping. Example:

VStack {
    VStack {
        Text("First")
    }
 }

Creates a type of VStack<VStack<Text>> or thereabouts, which is really what we want. SO we started with that premise:

WorkflowView(FR1.self) {
    thenProceed(with: FR2.self) {
        thenProceed(with: FR3.self) {
            thenProceed(with: FR4.self)
        }
    }
}

This creates the exact type structure we want. It just has the unfortunate side-effect of looking god-awful and creating a pyramid of doom. Turns out we can be a little bit smarter with autoclosures

WorkflowView(content: FR1.self)
           .thenProceed(with: workflowItem(FR2.self)
           .thenProceed(with: workflowItem(FR3.self)
           .thenProceed(with: workflowItem(FR4.self))))

The advantage is that this is very close to our current API. The disadvantage is the parenthesis hell at the end (LISP, anyone?) and the formatting XCode wants to do forces it right back into pyramid shape.

HOWEVER this gives us extremely important capabilities with both nav stacks and modals. Like they were seriously trivial once we had the type information flowing the correct direction. Ultimately while you may not like the aesthetics as much we care about functionality over aesthetics. If anybody feels brave I encourage them to try to do better, Richard and I have spent at least 15 hours on this so far and this is the best we could do.

@Tyler-Keith-Thompson
Copy link
Collaborator

Crappy Alternatives Considered:

So the premise is that you just need the types in the correct order:

  • Consumers just define their workflows backward!
WorkflowLauncher(isLaunched: .constant(true))
    .thenProceed(with: WorkflowItem(FR4.self))
    .thenProceed(with: WorkflowItem(FR3.self))
    .thenProceed(with: WorkflowItem(FR2.self))
    .thenProceed(with: WorkflowItem(FR1.self))
  • Consumers define their workflow forward but we put something in the DSL that tell US to reverse it. To do this we need code that looks like this:
func reverseTypes<C1, C2, C3>(backwardsWorkflow: ModifiedWorkflowViewOLD<ModifiedWorkflowViewOLD<ModifiedWorkflowViewOLD<Never, C1>, C2>, C3>) -> ModifiedWorkflowViewOLD<ModifiedWorkflowViewOLD<ModifiedWorkflowViewOLD<Never, C3>, C2>, C1> {
    return ModifiedWorkflowViewOLD<ModifiedWorkflowViewOLD<ModifiedWorkflowViewOLD<Never, C3>, C2>, C1>()
}

Note: There'd have to be iterations for as many workflow items as we wanted to support. Currently the swit compiler seems to cry at 9 workflow items so that'd mean we need C1-C9 permutations of reverseType functions. ALSO the consumers could call the DSL reverse thing multiple times and just keep flip-flopping the workflow. This solution would require us to do code-gen to handle all of that nonsense

@Richard-Gist
Copy link
Collaborator

Our efforts to get to the preferred solution can be found in branch nav-101. The refactoring is still ongoing, and view swapping no longer works at this point. More to come, but even with these bumps, we want to continue in this path because we believe this new way of working will be better. No longer will we recurse to the bottom then climb back out. Nav links will actually behave and function correctly. Modals will be just as simple.

@Richard-Gist
Copy link
Collaborator

Then @Tyler-Keith-Thompson came back and now we have mostly working view swapping. The API is still not re-implemented, but we have gotten closer to getting SwiftUIExample back up and running.

@Richard-Gist
Copy link
Collaborator

#111 Is required to enable this issue, but it is a step on the way.

@Richard-Gist
Copy link
Collaborator

Because #111 changed the API to enable this issue, a new discussion was made to track that: Update to the SwiftUI API for the sake of NavigationLinks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants