Tokamak icon indicating copy to clipboard operation
Tokamak copied to clipboard

NavigationView with SidebarListStyle contained list is centered on screen before 1st selection

Open foscomputerservices opened this issue 3 years ago • 6 comments

Given the following code we see a list centered in the middle of the screen until the 1st item is chosen. Once an item is chosen, the list moves to the left to the screen.

struct SidebarTest1View: View {
    var body: some View {
        NavigationView {
            List {
                ForEach(["a", "b", "c"], id: \.self) { title in
                    NavigationLink(title, destination: Text("Hello world: \(title)"))
                }
            }
            .listStyle(SidebarListStyle())
        }
    }
}

When the same code is run on iOS/macOS, the List is always at the far left of the screen (sadly, even for RTL languages).

Desktop

  • OS: macOS 11.1
  • Browsers: Safari (14.0.2), Chrome (87.0.4280.141), FireFox (84.0.2)
  • Version of Tokamak: 0.6.1

Additional context

I looked into this quite deeply and think I have found the issue. It has to do with DOMRenderer.mountTarget(before:to:with:) and List.body with an HStack { Spacer() }. Because of this Spacer, the width value on the containing div is set to "100%" instead of auto. This causes the List to take up the entire area of the window until content is selected and placed to the right of the list by the NavigationView.

Here's the code from List:

  public var body: some View {
    if let style = style as? ListStyleDeferredToRenderer {
      style.listBody(ScrollView {
        HStack { Spacer() } // <<- This spacer
        listStack
          .environment(\._outlineGroupStyle, _ListOutlineGroupStyle())
      })
        .frame(minHeight: 0, maxHeight: .infinity)
    } else { ... } }

I am not sure what the solution is, however, as I'm sure that the Spacer is probably there for some other scenario.

Screenshots iPadOS macOS Tokamak

Test Project

TestSidebar.zip/SidebarTest1View

foscomputerservices avatar Jan 16 '21 08:01 foscomputerservices

Most lists are meant to fill their container (We should probably replace that with a flexible frame, I don't think they were working when that was implemented). Sidebar is different though, so maybe we should add a property to ListStyleDeferredToRenderer to specify whether it should fill its parent or not. Wdyt?

carson-katri avatar Jan 16 '21 17:01 carson-katri

Would it make more sense to make it look like iPad and macOS apps by default? At least on wide screens.

MaxDesiatov avatar Jan 16 '21 17:01 MaxDesiatov

Most lists are meant to fill their container (We should probably replace that with a flexible frame, I don't think they were working when that was implemented).

Ah, that makes sense.

Sidebar is different though, so maybe we should add a property to ListStyleDeferredToRenderer to specify whether it should fill its parent or not. Wdyt?

Yes, Sidebar is certainly an interesting application of NavigationView and ListView, for sure. Sounds like a reasonable way to fix it to me, could even default to fill = true.

I can make this change, if it is acceptable to all.

foscomputerservices avatar Jan 17 '21 12:01 foscomputerservices

Would it make more sense to make it look like iPad and macOS apps by default? At least on wide screens.

Ah, interesting! Of course. On very narrow views (e.g. horizontalSizeClass == .compact), the List does consume 100% of the screen when the Sidebar is being displayed. However, I don't think that tokamak supports UserInterfaceSizeClass, does it? Therefore I've been assuming that tokamak would be imitating macOS/iPadOS (e.g. horizontalSizeClass == .regular). But maybe my assumption is flawed.

foscomputerservices avatar Jan 17 '21 12:01 foscomputerservices

No, we do not support UserInterfaceSizeClass yet (see #152), but it should be relatively easy to add to make List and NavigationView adjust based on the current size class.

MaxDesiatov avatar Jan 17 '21 15:01 MaxDesiatov

is there a current solution for this?

aehlke avatar Sep 19 '23 16:09 aehlke