1

This question is coming on the heels of this question that I asked (and had answered by @Asperi) yesterday, but it introduces a new unexpected element.

The basic setup is a 3 column macOS SwiftUI app. If you run the code below and scroll the list to an item further down the list (say item 80) and click, the List will re-render and occasionally "jump" to a place (like item 40), leaving the actual selected item out of frame. This issue was solved in the previous question by encapsulating SidebarRowView into its own view.

However, that solution works if the active binding (activeItem) is stored as a @State variable on the SidebarList view (see where I've marked //#1). If the active item is stored on an ObservableObject view model (see //#2), the scrolling behavior is affected.

I assume this is because the diffing algorithm somehow works differently with the @Published value and the @State value. I'd like to figure out a way to use the @Published value since the active item needs to be manipulated by the state of the app and used in the NavigationLink via isActive: (say if a push notification comes in that affects it).

Is there a way to use the @Published value and not have it re-render the whole List and thus not affect the scrolled position?

Reproducible code follows -- see the commented line for what to change to see the behavior with @Published vs @State

struct Item : Identifiable, Hashable {
    let id = UUID()
    var name : String
}

class SidebarListViewModel : ObservableObject {
    @Published var items = Array(0...300).map { Item(name: "Item \($0)") }
    @Published var activeItem : Item? //#2
}

struct SidebarList : View {
    @StateObject private var viewModel = SidebarListViewModel()

    @State private var activeItem : Item? //#1
    
    var body: some View {
        List(viewModel.items) {
            SidebarRowView(item: $0, activeItem: $viewModel.activeItem) //change this to $activeItem and the scrolling works as expected
        }.listStyle(SidebarListStyle())
    }
}

struct SidebarRowView: View {
    let item: Item
    @Binding var activeItem: Item?

    func navigationBindingForItem(item: Item) -> Binding<Bool> {
        .init {
            activeItem == item
        } set: { newValue in
            if newValue {
                activeItem = item
            }
        }
    }

    var body: some View {
        NavigationLink(destination: Text(item.name),
                            isActive: navigationBindingForItem(item: item)) {
            Text(item.name)
        }
    }
}

struct ContentView : View {
    var body: some View {
        NavigationView {
            SidebarList()
            Text("No selection")
            Text("No selection")
                .frame(minWidth: 300)
        }
    }
}

(Built and tested with Xcode 13.0 on macOS 11.3)

jnpdx
  • 45,847
  • 6
  • 64
  • 94
  • This seems to be an inordinately complex solution for minimal, reproducible example. It feels like it is going down a rabbit hole. I would like to gain a better perspective. With your app, you select an item from the first column. Does that cause the second column to populate with choices based on the first column choice? And then to a detail on the third column? or are the first two columns independent, and the combination gives you the detail in the third? – Yrb Oct 09 '21 at 19:06
  • 2
    @Yrb I respectfully disagree that it's "inordinately complex" -- it's fewer than 50 lines of code with a *one line* change that dramatically affects the behavior. Yes, in my actual app, the second column is based on choices from the first and the detail is on the third -- ala the Mail app. All of that code is removed as I tested it and it had no bearing on on the behavior in the first column. It's possible that I could remove the third column in the example, but it seems trivial since it's just a `Text` item here. – jnpdx Oct 09 '21 at 19:36
  • @Cristik but without 3 children in the `NavigationView` root, one doesn't get the 3-column layout. Is there another way to get it? If not, I don't think I'm "not properly using the navigation view" -- in fact, this seems to be the standard practice for achieving this layout. – jnpdx Oct 12 '21 at 15:11
  • Tested more on this, the problem goes away if you remove the `isActive` argument passed to the `NavigationLink`. Do you need that, BTW? Architecturally speaking, it's strange for a list row to be aware of the surrounding context, like the selected item in the parent list. – Cristik Oct 14 '21 at 05:41
  • I do need isActive — that’s how I would affect navigation if a push notification came in that would warrant showing a specific view, for example. – jnpdx Oct 14 '21 at 06:07
  • But do you need it in the row view also? – Cristik Oct 14 '21 at 06:08
  • Yes — I need to be able to push all the way to the 3rd column and have the selections be correct. Like in Mail, for example. – jnpdx Oct 14 '21 at 06:28
  • Can you edit the question with a sample of how you plan to propagate the selection? Maybe we can find an alternative that doesn’t have the scrolling issue – Cristik Oct 14 '21 at 08:00
  • @Cristik https://gist.github.com/jnpdx/7cd8f2c01359361dc0c17389332197d8 I'm hesitant to change the original example because there was already an issue with someone thinking it was too long and adding the code to propagate selection state definitely adds more complexity. – jnpdx Oct 15 '21 at 18:32
  • Looked at the gist, the problem goes away if you replace the bindings by regular properties, and remove the `isActive` argument sent to the navigation link. You should not need them anyway. – Cristik Oct 16 '21 at 08:43
  • @jnpdx Played a little with the "navigate to" button, my conclusion is that there are SwiftUI bugs at play here, but you are also incorrectly using the view model. I stand by my original comment, you're leaking too much information to the child views. Try fixing that, and you'll see that you app will increasingly work better and better (until hitting the SwiftUI bugs, ofcourse) – Cristik Oct 17 '21 at 07:02
  • Okay. I appreciate the advice, but I’m afraid I don’t have any ideas about how to fix it. How else can I propagate the active state to the child views (and of course back up when the links are activiated)? I’m not trying to “leak information” — only as much as I need to for the functionality. I’d love to fix it, but I’m at a loss here — thus the question. – jnpdx Oct 17 '21 at 07:12
  • You could inject the selection to the three columns, so instead of circulating both `activeFolder` and `activeItem`, you'd inject each column only the values it needs: first column the folders list, and a binding for the selected folder, second the items and a binding for the selected item, third, just the item. – Cristik Oct 17 '21 at 08:13
  • In the original question (not the gist), the only thing propagated is the active item to the first sidebar and the issue is still encountered. I’m not sure how to fix that and still be able to use isActive on that first link. – jnpdx Oct 17 '21 at 08:29
  • Also, it might help not to thinking in navigation view terms, you don't need a navigation view there, you already have a predefined number of columns, a navigation view is not the best choice. – Cristik Oct 17 '21 at 08:45

1 Answers1

1

Update. I still think that the original answer identified the problem, however seems that there's an even easier workaround to this: push the view model one level upstream, to the root ContentView, and inject the items array to the SidebarList view.

Thus, the following changes should fix the "jumping" issue:


struct SidebarList : View {
    let items: [Item]
    @Binding var activeItemId: UUID?
    // ...
}

// ...

struct ContentView : View {
    @StateObject private var viewModel = SidebarListViewModel()

    var body: some View {
        NavigationView {
            SidebarList(items: viewModel.items,
                        activeItemId: $viewModel.activeItemId)
        // ...
}

For some reason, this works, I don't have an explanation why. However, there's one problem left, that's caused by SwiftUI: programatically changing the selection won't make the list scroll to the new selection. Scroll SwiftUI List to new selection might help fixing this too.

Also, warmly recommending to move the NavigationLink from the body of SidebarRowView to the List part of SidebarList, this will help you limit the amount of details that get leaked to the row view.

Another recommendation I would make, would be to use the tag:selection: alternative to isActive. This works better when you have a pool of possible navigation links from which only one can be active at a certain time. This involves of course changing the view model from var activeItem: Item? to var activeItemId: UUID?, this will avoid the need of the hacky navigationBindingForItem function:

class SidebarListViewModel : ObservableObject {
    @Published var items = // ...
    @Published var activeItemId : UUID?
}

// ...
NavigationLink(destination: ...,
               tag: item.id, 
               selection: $activeItemId) {

Original Answer

This is most likely what's causing the problematic behaviour:

func navigationBindingForItem(item: Item) -> Binding<Bool> {
        .init {
            activeItem == item
        } set: { newValue in
            if newValue {
                activeItem = item
            }
        }
    }

If you put a breakpoint on the binding setter, you'll see that the setter gets called every time you select something, and if you also print the item name, you'll see that when the problematic scrolling happens, it always scroll to the previous selected item.

Seems this "manual" binding interferes with the SwiftUI update cycle, causing the framework to malfunction.

The solution here is simple: remove the @Binding declaration from the activeItem property, and keep it as a "regular" one. You also can safely remove the isActive argument passed to the navigation link.

Bindings are needed only when you need to update values in parent components, most of the time simple values are enough. This also makes your views simpler, and more in line with the Swift/SwiftUI principles of using immutable values as much as possible.

Cristik
  • 30,989
  • 25
  • 91
  • 127
  • But without isActive how would I control navigation programmatically as discussed in the comments? – jnpdx Oct 16 '21 at 16:02
  • @jnpdx the navigation link doesn't need the `isActive` to do its job. The presence of the element is enough to trigger the appropriate pushes. – Cristik Oct 17 '21 at 05:51
  • Sorry, I must be missing something -- in the gist version, how does the "Navigate to 25-0" Button that is meant to simulate a push notification propagate the state without `isActive` and select the right answer? Can you provide a code sample showing this? Like I said, I must be missing something... – jnpdx Oct 17 '21 at 05:54
  • @jnpdx OK, I get now what you asked, don't think you mentioned about this before. I haven't played with the button, maybe update your question to include this too, otherwise other people stumbling upon the question might not fully understand the answer(s). – Cristik Oct 17 '21 at 06:04
  • Sorry it wasn't clear. I did mention it in the original question and in this one ("I'd like to figure out a way to use the `@Published` value since the active item needs to be manipulated by the state of the app"). I didn't explicitly mention what had been changed in the gist, and that's definitely my fault. I will try to make it more clear in the original question as well. Yeah, without that, there's no need for bindings at all, which would be convenient... Thank you for your attention to this, though! – jnpdx Oct 17 '21 at 06:07
  • @jnpdx lol, I just found a simpler solution that works with little changes to both the code from the question, and to the gist code. One big problem that surfaced now, is that the programatic change of the selection doesn't make the list scroll to the new selected item, and also the two detail views don't get refreshed until the list is manually scrolled to the new selected item. This is clearly a(nother) SwiftUI bug, added a link to a possible solution to this. – Cristik Oct 17 '21 at 18:36
  • Excellent work! I had avoided the `NavigationLink` with `selection` because in SwiftUI 1.0 it was completely broken on 10.15. But, it appears to work on Big Sur just fine. Thanks for pointing me towards that! In regards to the `NavigationLink`s in the `SidebarListRow` with the binding, that was a result of the answer to the original question (linked at the top of this one), but, obviously it lead to compromises. You've seemed to hit the nail on the head with the update though. I wish I understood the fundamental change in moving the view model up, but happy to accept somewhat blindly for now. – jnpdx Oct 17 '21 at 19:48