Fix judder on scroll with disappearing scrollBehavior

[dfabulich]

Fixes #295

This PR looks small, but it feels like major surgery in the very heart of the NavigationStack, converting it from a Column containing the topBar, content, and bottomBar into a Box where the topBar is in its own Box aligned at the top, the bottomBar is aligned at the bottom, and the content appears in its own Box, filled to max size, relying on safe area insets to avoid clobbering the top and bottom bars.

(We only need to add top/bottom padding if the embedded content ignores the safe area at that edge; in that case, we add padding just so that IgnoresSafeAreaLayout will undo the padding we just added.)

It's quite difficult for me to be confident that I didn't break any layout. I did verify that everything's working in the Safe Area playground in the Showcase. There appear to be no tests in Tests that test ignoresSafeArea. (But, just to be safe, I ran 'em all.)

FWIW, I suspect that this code may improve performance in NavigationStack. In my repro for #295, this PR fixing the layout shift/judder during scroll naturally resulted in fewer recompositions happening during scroll.

(Perhaps I'll take the liberty of calling for @aabewhite's attention on this one!)

Skip Pull Request Checklist:

• REQUIRED: I have signed the Contributor Agreement
• REQUIRED: I have tested my change locally with swift test
• OPTIONAL: I have tested my change on an iOS simulator or device
• OPTIONAL: I have tested my change on an Android emulator or device

[marcprux]

Agreed, we'll need to look pretty closely at this. Did you run through all the navigation playgrounds in Showcase as well?

In the past with risky overhauls like this (like #238), we've offered a backwards-compatibility setting as an escape hatch. I wonder if you could re-use the .layoutImplementationVersion() and bump the default value to 2, and only use the new navigation layout if it is higher than that. It might be a lot of refactoring (or a lot of code duplication), but it would ease our minds about providing a way to avoid large-scale breakage.

skip-ui/Sources/SkipUI/SkipUI/Environment/EnvironmentValues.swift

Lines 700 to 704 in 6377088

	/// Allow users to revert to previous layout behavior.
	var _layoutImplementationVersion: Int {
	get { builtinValue(key: "_layoutImplementationVersion", defaultValue: { 1 }) as! Int }
	set { setBuiltinValue(key: "_layoutImplementationVersion", value: newValue, defaultValue: { 1 }) }
	}

[dfabulich]

The latest version of this PR now adds a layoutImplementationVersion 2. I've tested that .layoutImplementationVersion(1) reverts back to the Column-based layout. (I copied and pasted a bunch of code rather than trying to deduplicate, because I figure anybody who wants to opt in to the old algorithm wants it exactly the way it used to be. If/when we make further changes to the Box-based layout, those can and should diverge from the old way.)

I also added in a layout fix which arose when I tested the navigation playgrounds. 😬 Good on us for finding it! But IMO that's also a bad sign… where else do I still need to check??

[aabewhite]

Really appreciate the patch! See my two comments. So action items are:

1. Restore the safeDrawing insets for cases when the bars are hidden.
2. Simplify the new Box code to pad the content Box based on the heights of the top and bottom bars.

Hopefully I'm correct in how this is all working and not leading you in the wrong direction.

[dfabulich]

OK! I've tested the current version against the NavigationStack playground, the Toolbar playground, and the SafeArea playground, including a new Toolbar subplayground skiptools/skipapp-showcase#56 and a new SafeArea subplayground skiptools/skipapp-showcase#57 that I used to repro the issues that @aabewhite reported.

I think this should be clear to merge now.

[aabewhite]

Hey Dan - in this version of the patch you're no longer padding the content Box when top/bottom bars are visible, so the content ends up under the bars.

I'm not sure if there was a problem with pushing the wrong code... but it's broken everywhere.

It's also possible that I've screwed something up applying the patch, but the code itself looks wrong to me too: it only adds non-zero top/bottom padding to the content Box when the bars are hidden.

[dfabulich]

nope, I screwed it up somehow. I'll try it again.