Skip to content

Bugfix: Stabilize NavigationStack Layout + Reduce NavigationBar Spacing on Sheets - #475

Open
fhasse95 wants to merge 4 commits into
skiptools:mainfrom
fhasse95:Navigation-Layout-Bugfix
Open

Bugfix: Stabilize NavigationStack Layout + Reduce NavigationBar Spacing on Sheets#475
fhasse95 wants to merge 4 commits into
skiptools:mainfrom
fhasse95:Navigation-Layout-Bugfix

Conversation

@fhasse95

@fhasse95 fhasse95 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

This PR stabilizes NavigationStack layout behavior so that the navigation bar and the content below it no longer jump when the navigation bar height is measured or updated.

Before this change, the navigation bar could be laid out with an initial height of zero or an incomplete height, and then expand to its final size after the first layout/measurement pass. Visually, this made the navigation bar appear collapsed immediately after a tab switch and then "slide" or jump into place once layout completed.

In release builds with simple views this is often hard to notice because the layout pass completes very quickly. With more complex views, or in debug builds, however, the intermediate state becomes visible. This is also reproducible in the Skip Showcase app in a debug build when taking a closer look.

To reproduce this in the Skip Showcase app:

  1. Install and launch a debug build of the app
  2. Start on the About tab
  3. Switch to the Showcase tab
  4. Observe the navigation bar during the initial layout of the Showcase view

Note: The jump only happens during the first layout of that view. After the Showcase tab has been laid out once, switching away and back again usually does not reproduce the issue since the navigation bar has already been measured.

Below is a before/after comparison of this example in the Skip Showcase app, slowed down to make the issue easier to see:

Before:
Before

After:
After

As shown above, before the change, the navigation bar is initially collapsed after switching tabs and becomes visible only after layout catches up. After the change, the navigation bar is present at the correct size immediately.

This is achieved by reserving an estimated top-bar height before the first real measurement is available. Once the actual top-bar size has been measured, the layout switches to the measured value. This keeps the content and navigation bar stable during initial layout and prevents the visible jump.

I also manually verified additional cases that previously showed navigation-bar jumps during initial layout, including NavigationStack inside .sheet and .fullScreenCover, using my own Skip app with more complex views and navigation flows. Those cases are also fixed with this change.

As part of the same layout improvements, this PR also reduces unnecessary empty space between the navigation bar and the top edge of sheets, making better use of the available sheet area:

Large Title (Before / After):
Before (Sheet Large)After (Sheet Large)

Inline Title (Before / After):
Before (Sheet Inline)After (Sheet Inline)


Thank you for contributing to the Skip project! Please review the contribution guide at https://skip.dev/docs/contributing/ for advice and guidance on making high-quality PRs.

Use this space to describe your change and add any labels (bug, enhancement, documentation, etc.) to help categorize your contribution.

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
  • REQUIRED: I have checked whether this change requires a corresponding update in the Skip Fuse UI repository (link related PR if applicable)
  • OPTIONAL: I have added an example of any UI changes to the Showcase sample app

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

I used Codex to iteratively investigate and refine the fix for the NavigationStack layout issue. To verify the results, I tested multiple playground views in the Skip Showcase app and my own Skip app, which contains several navigation scenarios, including sheets, full-screen covers, tab bars, and nested navigation. All of these scenarios work properly after the bug fix.


@cla-bot cla-bot Bot added the cla-signed label Jul 3, 2026
@dfabulich

Copy link
Copy Markdown
Member

This code is extremely finicky, and has tests all over the showcase. I think it would help for you to list out exactly which playgrounds you tested and how.

One that you didn't explicitly call out is the SafeArea playground --> Geometry padding playground, which has facilities for hiding and showing the Navigation Bar, both with and without a sheet.

@fhasse95

fhasse95 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

This code is extremely finicky, and has tests all over the showcase. I think it would help for you to list out exactly which playgrounds you tested and how.

One that you didn't explicitly call out is the SafeArea playground --> Geometry padding playground, which has facilities for hiding and showing the Navigation Bar, both with and without a sheet.

Yes, I totally agree. Since this code is central and used in many places, I wanted to be extra careful. That's why I tried out the changes in various scenarios.

In Skip Showcase, I validated the changes using the following playgrounds:

  • Sheet Playground

    • Tested all options, including both .sheet and .fullScreenCover, with both inline and large navigation bar title modes.
  • NavigationStack Playground

    • Tested all available options, again with both inline and large navigation bar title modes.
  • Toolbar Playground

    • Tested all available options, including hiding/showing the navigation bar, custom navigation bar colors, etc.
  • SafeArea Playground

    • Tested all available options, with particular focus on .fullScreenCover, .sheet, and the Geometry Padding playground.
    • I also verified that features such as hiding/showing the navigation bar continued to behave correctly there.

During implementation, I first tested the changes in my own app across a variety of scenarios, refining the implementation iteratively based on my observations. Besides fixing the original issue, I specifically looked at things like switching between tabs, verifying the initial layout pass of the navigation bar, and the layout behavior of sheets and full-screen covers. I also made sure that neither the navigation bar nor the underlying content jumped during transitions.

Most of my testing was done on my Google Pixel 9, where I also verified gesture-driven interactions, such as slowly dismissing a sheet using the back gesture, to ensure the navigation bar behaves correctly throughout the interactive transition.

Once everything looked good there, I built Skip Showcase against my local SkipUI branch and went through all of the playgrounds above. To make sure I hadn't introduced any regressions, I repeatedly switched between my bugfix branch and the current main branch of SkipUI and compared the behavior side by side. Other than the initial layout pass bug now being fixed, I wasn't able to observe any side effects.

That said, I obviously can't completely rule out that I may have missed an edge case. So please feel free to try it with either your own app(s) or with Skip Showcase, and let me know if you spot anything that doesn't look right 😊

@fhasse95

Copy link
Copy Markdown
Contributor Author

@dfabulich I spent some more time testing this change extensively and found one additional edge case, which is now fixed by the latest commit.

The issue occurred when the actual toolbar content was taller than the initially reserved navigation bar height. In that situation, the sheet could still jump slightly during the interactive Android back gesture while the sheet or full-screen cover was shrinking.

I encountered this in my own app while testing different localizations. For example, the Arabic version of my Save button resulted in slightly taller toolbar content than the default content used in some other languages:

This caused the measured toolbar height to exceed the initial estimate, making the transition visible during the gesture. The latest commit accounts for this case, and I can no longer reproduce the jump.

To further clarify the main issue addressed by this PR:

Before this change, the navigation bar was initially laid out with a height of zero and would then expand to its final measured size during a later layout pass. With smaller or simpler views, this is often difficult to notice because layout completes very quickly. In larger and more complex applications, particularly in debug builds, however, the intermediate layout state becomes clearly visible.

This PR avoids that intermediate state by reserving an estimated navigation bar height until the actual toolbar height has been been measured. Once the measurement is available, the estimate is seamlessly replaced with the final value.

This behavior can be observed whenever a view containing a NavigationStack is presented for the first time.

For example:

  1. After switching tabs (and even right after app launch), the navigation bar may initially not be rendered at all. It then appears from the top once its height has been measured, causing the content below it to jump.

  2. When presenting a .sheet or .fullScreenCover, the navigation bar may again initially be laid out with a height of zero. During that brief state, the content shifts and the navigation bar background is missing, exposing a temporary white gap underneath the top bar.

Both behaviors can also be reproduced in the Skip Showcase app. Since it is a relatively simple application, the layout issues are only briefly visible and are easiest to spot by recording the screen and stepping through the captured frames. In more complex applications, however, they become much more obvious. The tab-switching issue is visible when opening a tab for the first time, and the presentation issue can be observed in the Sheet Playground.

I have attached relevant frame-by-frame screenshots from Skip Showcase below:

Switching Tabs [Before]

Switching Tabs [After]

As you can see, the navigation bar is already laid out with the correct height before the transition has completed.

Presenting Sheets [Before]

The same issue occurs when presenting sheets. Initially, the navigation bar is laid out with a height of zero before receiving its final height during a later layout pass.

Presenting Sheets [After]

After the changes, the navigation bar already has the correct height even before the sheet presentation has completed.

Presenting Fullscreen Covers [Before]

Presenting Fullscreen Covers [After]

With full-screen covers, the underlying issue was exactly the same. In addition, the navigation bar background was temporarily missing during the transition, resulting in a visible white gap. This PR fixes both issues.

Below are also a few screen recordings from my own Skip app running in debug mode. Since the UI is more complex (icons, gradients, additional drawing work, etc.), the delayed layout becomes much more noticeable than in the Skip Showcase app, as mentioned earlier.

Switching Tabs [Before]

Switching Tabs [After]

Presenting Fullscreen Covers [Before]

Presenting Fullscreen Covers [After]

I hope the additional screenshots and explanations help clarify the issue. If you have any further questions or concerns, please let me know 😊

@marcprux
marcprux requested review from dfabulich and marcprux July 17, 2026 02:24

@marcprux marcprux left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me on the surface, but I do share @dfabulich's concerns over the (lamentable) intricacy of navigation on Android, so I'd like to get his 2nd opinion before I merge.

@fhasse95

Copy link
Copy Markdown
Contributor Author

@marcprux @dfabulich FYI: I merged this branch with the latest main again and did another round of testing.

While testing, I noticed two additional issues. First, my previous version still contained a bug that caused the bottom safe area inset to be applied incorrectly for sheets. Second, I found another layout issue in SafeAreaPlayground → Sheet → Bottom Toolbar during the interactive Android back dismiss gesture, which is also present on the current main branch:

Before:
Before

After:
After

As you can see, before the fix the safe area was recalculated with a slight delay during the back dismiss gesture, causing the bottom toolbar to jump slightly.

Both issues are fixed in the latest commit.

@dfabulich

Copy link
Copy Markdown
Member

I've put some time and thought into this PR and, paradoxically, I agree that it fixes a bug, but I'm not sure we should go in this direction.

There are two core problems with SkipUI's NavigationStack.

  1. Problem 1: NavigationStack preferences vs. TopAppBar parameters: One core problem is that Compose TopAppBar requires all of its configuration to be passed in as it's composed, in parameters and modifiers, but SwiftUI NavigationStack expects all of its configuration to be set via preference modifiers on the body content. SkipUI renders the NavigationStack, typically painting something "wrong", then, as the body composes, discovers what it should have done, recomposes, and repaints.

    My attempt to address Problem 1 was to add layout hints. That sucks, because it means users have to add some special Skip modifiers to prevent a layout shift. Ideally, SkipUI NavigationStack would "just work" without hints.

    @fhasse95 have you tried layout hints? (You made no mention of hints in this PR.)

  2. Problem 2: The body's layout depends on the layout of the TopAppBar, which we can't reliably predict: Even if we pass in hints, so we can render the TopAppBar armed with the knowledge of the title and the title display mode, we still have no way of knowing for sure how tall the TopAppBar will be. Will it be 62dp collapsed? 112dp? Well, has the user selected an enlarged accessibility font? Has the user chosen an OS override font? We can make some guesses, but the only way to be sure is to actually render the TopAppBar and measure its layout.

    Today, we handle Problem 2 with an onGloballyPositioned modifier, but that guarantees that we'll get one wrong paint before we get a chance to recompose and render the body in the correct layout, even with layout hints.

The core idea of the initial commit of this PR #475 is to make better estimates about how tall the TopAppBar will be, addressing Problem 2 in a different way.

I have a half-baked PR on my machine that did something similar, but I wound up not filing it, partly because Problem 2 is invisible on my test devices as long as I pass in layout hints. (It's still happening … it's still adding to jank. But it's not visible.)

@fhasse95's estimates are pretty decent; when they're wrong, they're less wrong than the current behavior.

But adding better estimates means adding more code to Navigation.swift, making it even more complex. And it still has no automated tests, which IMO is a big part of the reason we kept you waiting almost a month to get this thing reviewed and merged.

I reviewed this PR and thought, "This can't be the right way to handle this. More and more and more code… this can't be right." Navigation.swift already had a "code smell" before you started to work on it; this PR makes that smell more visible. (More… pungent??)

So, I went and looked at how Compose Scaffold handles this. The answer? SubcomposeLayout. Subcomposing allows Scaffold to measure the TopAppBar and use those measurements to position the body content in a single pass. It doesn't attempt to estimate how large the TopAppBar will be, because it doesn't need to.

So, I pulled together a draft PR in that direction.

There are other fixes in the current PR #475, some of which (I think?) don't have anything directly to do with the new estimation code; it's hard to evaluate those in isolation. (I know from experience that fixes to Navigation.swift inherently pile up on one another, so it makes sense that you've added more fixes in here as you found them. Surely someday Github will give us all access to stacked PRs.)

So, we have a couple options:

  1. Merge this PR as-is. It'll make Navigation.swift more complex, and the estimates can't be perfect
  2. Switch to a SubcomposeLayout approach like Navigation safe area subcompose refactor #494. To be honest, Navigation safe area subcompose refactor #494 doesn't look any simpler, but that's because I put it behind a layoutImplementationVersion flag, allowing users to switch back to older implementations at runtime. Maybe someday we'd release a SkipUI 2.0, remove that flag, and simplify.

WDYT?

@fhasse95

fhasse95 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

@dfabulich Thanks for taking the time to review this so thoroughly and for explaining the underlying issues in such detail. I also appreciate you putting together #494.

To answer your question: I haven't tried layout hints yet, so I can't really compare the behavior with and without them. However, if the SubcomposeLayout approach doesn't end up being significantly simpler overall, my preference would be to simply merge this PR as it is and then revisit the Navigation implementation as part of a future SkipUI 2.0 refactor, similar to what you suggested with the layoutImplementationVersion flag.

That way, we'd at least have the current bugs fixed in the existing implementation, even if Navigation.swift remains fairly complex, which, as you pointed out, was already the case before this PR.

If that sounds reasonable to you, I'd merge the latest main branch into this PR first, since another contributor has already created a fix for the sheet top spacing issue that is also included in this PR. After that, however, I think this PR would be ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants