[ios] pull down InstructionView handle to show remaining steps

[michaelkirk]

FIXES #32

cue-sheet-notched.mov.mp4

cue-sheet-non-notched.mov.mp4

[michaelkirk]

The layout changes here are non-trivial.

We want to be able to expand the instruction view over top of the other content when it's expanded. But when it's not expanded, we want it to behave as if it's in a vstack with the NavigationInnerGridView.

My solution was to put the InstructionView in a ZStack above the grid, and then plumb out the InstructionView's "unexpanded height" to push down the NavigatingInnerGridView.

I struggled a bit with implementing this layout in SwiftUI. It's something I think would be natural to express in old school iOS layout constraints. Maybe there's a better way?

[ianthetechie]

Yeah, I'm not aware of a better way offhand either. It's one of the areas where SwiftUI and others struggle compared to springs-and-struts or auto layout.

[ianthetechie]

There are a few conflicts unfortunately with the main branch since @Archdoog just cleaned up a bunch of stuff to clean up the pattern matching mess 😅

[michaelkirk]

At the end of the day I don't think it's dangerous to include this change, but it's curious.

Here's what it looks like with this change reverted:

icon-size.mov.mp4

[michaelkirk]

Converting to a draft, I'd like to tweak the handle styling slightly and add a snapshot test for the expanded maneuver list.

[michaelkirk]

Ok! I applied a couple last style tweaks, and updated the demo videos in the description.

[ianthetechie]

I wish SwiftUI had a better model for this sort of thing. It's technically a binding, but really its sole purpose is publishing values back out to observers. Maybe we can clarify this a bit in the comment (if I'm understanding it correctly)?

[michaelkirk]

Yes, you are understanding correctly — this is a Binding because we want the child component to communicate something to its parent. I think your additional documentation makes this clearer. 👍

[ianthetechie]

Ok, this is really close now :) I've cleaned up a few things and left some final comments for discussion!

[ianthetechie]

There are a few conflicts unfortunately with the main branch since @Archdoog just cleaned up a bunch of stuff to clean up the pattern matching mess 😅

[michaelkirk]

I think that we can sidestep the problem by making this a binding instead of a boolean value. This way developers can still do what you intended, but with less potential confusion and more flexibility:

Funny enough, I actually had it as a binding initially, but got annoyed at not being able to fiddle with the canvas Previews, and decided to change it.

But really, exposing the "isExpanded" parameter was all just a last minute thing I added to enable snapshot tests, so I'm pretty happy to go whatever way you prefer with it.

(Side note: it's clear that you understand what's happening since your comment explains that this is only the initial value, but I think that puts you in the top like 2% of devs by SwiftUI knowledge :D)

Thank you, but this is a miscalculation on your part. 😆 I'm new to SwiftUI and any apparent competence is purely coincidental. I'm almost constantly confounded by it.

[michaelkirk]

There are a few conflicts unfortunately with the main branch since @Archdoog just cleaned up a bunch of stuff to clean up the pattern matching mess 😅

I have rebased and pushed.

If this is not a rebase and force push kind of town, let me know, and I will merge with main next time.

[ianthetechie]

Thanks! Giving it a final check now!

I'm almost constantly confounded by it.

ha! Yeah, SwiftUI is... pretty confounding. I found Thinking in SwiftUI helpful for making it SLIGHTLY less confounding... (No joke, I read the sections on state management like 25 times over while doing the initial sketches of the MapLibre SwiftUI DSL.)

If this is not a rebase and force push kind of town, let me know, and I will merge with main next time.

All good; thanks for checking 👍

[michaelkirk]

I prefer your change because it's clearer, but I don't see how the previous version was a bug.

If remaining steps had one thing in it, then remainingSteps[1...] == [] so the result is the same, right?

Or am I missing something?

[ianthetechie]

So I went off to code up a Swift Fiddle, because OBVIOUSLY Swift is zero-indexed, and accessing index 1 of an array of length 1 is out of bounds access...

You can subscript an array with any integer from zero up to, but not including, the count of the array. Using a negative number or an index equal to or greater than count triggers a runtime error.

But' here's a fiddle that proves your original code actually returns empty array as you claim: https://swiftfiddle.com/aw3i6znzvjcjzi3pdoefmu23sy. I've been writing Swift since the 0.x days and I have to say, this is the most surprised I've been in a very long time! 🤯

Scouring the Apple docs, I couldn't figure out why this works as it does, but your code was indeed correct 😂 If you've got any links explaining why this is the way it is, I'd be grateful to learn!

[michaelkirk]

let foo = ["a"]
print("foo[0] => \(foo[0])") // "a"
// print("foo[1] => \(foo[1])") // Fatal error: Index out of range

print("foo[0...] => \(foo[0...])") // ["a"]
print("foo[1...] => \(foo[1...])") // []

// And my favorite - while this works (and seems like it should):
print("foo[0...] => \(foo[0...])") // ["a"]
print("foo[0..0] => \(foo[0...0])") // ["a"]

// This blows up
print("foo[1...] => \(foo[1...])") // []
// print("foo[1..1] => \(foo[1...1])") // Fatal error: Array index is out of range

// I think this is a hint to the riddle
// print("foo[2...] => \(foo[2...])") // Swift/Range.swift:754: Fatal error: Range requires lowerBound <= upperBound

// Note the error! 
// It's obvious where the `lowerBound` is coming from (2), 
// but where is the `upperBound` coming from which is greater than 2?

func example() {
  // My guess is that accessing a RandomAccessCollection with PartialRangeFrom, 
  // converts the PartialRangeFrom to a Range (not a ClosedRange!), using the Array.length as the `upperBound` of the Range. That would explain all the behavior we're seeing.

  do {
    let rangeFrom: PartialRangeFrom = 2...
    let arrayLength = 1
    // this will panic when rangeFrom.start is greater than arrayLength - e.g. 2..<1
    let range: Range = rangeFrom.lowerBound..<arrayLength 
  }

  do {
    // but this will not panic, it'll just be empty:
    let rangeFrom: PartialRangeFrom = 1...
    let arrayLength = 1
    let range: Range = rangeFrom.lowerBound..<arrayLength 
    assert(range == 1..<1)
  }
}

Link to fiddle: https://swiftfiddle.com/l4jhe6atjjgkdnih6ot6rsyzgy