Potential solution for side jumping issue

[eonist]

Now I have a complete picture. I can see exactly what is wrong and what the fix is. Let me read the InlineJobRowsView and check the SystemStatsViewModel for anything that might fire while the popover is open.

The code is actually architecturally sound, but I can see two critical bugs in the current branch code. Let me compose the definitive fix.

Here is the precise diagnosis and the complete fix:

---

Root Cause Diagnosis

The branch's AppDelegate.swift has the correct architecture — sizingOptions = .preferredContentSize (default), no manual contentSize writes in openPopover() or navigate(). The regression guard comments are correct. However there are two active bugs causing the jump that all 40+ commits have missed:

Bug 1 — SystemStatsViewModel fires while the popover is open

SystemStatsViewModel runs a timer that updates @StateObject private var systemStats inside PopoverMainView.body. This triggers a SwiftUI layout pass → intrinsicContentSize update → preferredContentSize propagates → NSPopover re-anchors on every tick even with the store.reload() guard in place. The isPopoverOpen guard only blocks store.reload() but does not stop SystemStatsViewModel from pushing state changes.

Bug 2 — isPopoverOpen is passed as a let constant, not as a binding

PopoverMainView receives isPopoverOpen: Bool as a var (value-copy). When AppDelegate sets popoverIsOpen = true then calls observable.reload() and then show(), the PopoverMainView that was already rendered still has isPopoverOpen = false until SwiftUI re-renders. The timer fires immediately with the old stale value and calls store.reload() anyway.

Bug 3 — InlineJobRowsView has @State private var cap that mutates on tap

The "+ N more jobs…" button in InlineJobRowsView increments @State cap, which causes a SwiftUI height change while the popover is open.

---

The Fix

The definitive solution has three parts, all in existing files:

Fix 1: Stop SystemStatsViewModel while popover is open

In PopoverMainView.swift, pass isPopoverOpen into SystemStatsViewModel and gate its timer updates:

// In PopoverMainView — add to .onChange of isPopoverOpen:
.onChange(of: isPopoverOpen) { open in
    if open {
        systemStats.pause()   // stop firing layout passes
    } else {
        systemStats.resume()  // resume only when popover is closed
    }
}

Or more simply — stop SystemStatsViewModel entirely while open and only start it when the popover appears and re-read on every open. Since the header stats don't need to update while the popover is open (they were polled once on open), just call systemStats.stop() when isPopoverOpen becomes true and systemStats.start() when it becomes false.

Fix 2: Make isPopoverOpen a @Binding

Change PopoverMainView:

// BEFORE (breaks — timer reads stale value):
var isPopoverOpen: Bool = false

// AFTER (correct — always reads current AppDelegate value):
@Binding var isPopoverOpen: Bool

And in AppDelegate.mainView():

// Pass a binding to popoverIsOpen
return AnyView(PopoverMainView(
    store: observable,
    onSelectJob: ...,
    onSelectAction: ...,
    onSelectSettings: ...,
    isPopoverOpen: Binding(
        get: { self.popoverIsOpen },
        set: { self.popoverIsOpen = $0 }
    )
))

Fix 3: Freeze InlineJobRowsView cap while open

The cap @State mutation resizes the view. Either cap expansion to only happen when !isPopoverOpen, or better: accept isPopoverOpen as a prop and disable the expand button while open.

---

The Correct Minimal Patch

Here are the exact diffs for PopoverMainView.swift — this is the minimal change that resolves all three bugs without touching architecture:

and

Now I have a very clear picture. Let me do one more targeted read to compare the main branch openPopover() vs the fix branch, which I already have. I also need to check if the fix branch's PopoverMainView uses a ScrollView (which is the critical new addition for the redesign):

Now I have everything I need. The analysis is clear. Let me give you the definitive diagnosis and fix.

---

The Root Cause on the Fix Branch

The fix branch (design-branch-3) has correctly implemented the Architecture 1 (fittingSize measurement before show) pattern in AppDelegate.swift — including sizingOptions = [], measuring fittingSize.height before show(), always using Self.idealWidth for width, and never touching contentSize after show(). That part is correct and matches the proven main-branch pattern.

The problem is in PopoverMainView.swift on the fix branch . The redesign wraps the actions list in a ScrollView, and Rule 6 in the file's own regression guard explains exactly what happens :

"Without this cap, fittingSize.height reports the full unbounded content height (can be 1000pt+ with 30 cached groups), making the popover grow off-screen."

But there are three compounding bugs that together cause the jump and/or wrong height:

---

The Three Bugs

Bug 1 — sizingOptions is [] but ScrollView gives fittingSize.height = 0 on first open

When sizingOptions = [] (correct, prevents re-anchor), NSHostingController does not auto-size; AppDelegate.openPopover() must measure fittingSize.height itself. But ScrollView reports fittingSize.height = 0 before its content has laid out at the correct width — unless you explicitly set the frame to idealWidth × screenHeight and call layoutSubtreeIfNeeded() first .

The fix branch does perform this measurement step :

hostingController.view.setFrameSize(measureFrame)  // idealWidth × screenHeight
hostingController.view.layoutSubtreeIfNeeded()
let naturalHeight = hostingController.view.fittingSize.height

But there is a subtle issue: the ScrollView has a dynamic maxScrollHeight cap computed as (NSScreen.main?.visibleFrame.height ?? 700) - 120 . This cap is a SwiftUI state value computed at render time — it doesn't get picked up by layoutSubtreeIfNeeded() until the view has gone through a full SwiftUI render pass. On the first open, the view hasn't rendered yet so fittingSize.height can return 0 or an incorrect value, falling back to the naturalHeight > 0 ? naturalHeight : 300 guard which hardcodes 300pt — causing wrong height.

Bug 2 — openPopover() does NOT reload mainView before measuring

Look at openPopover() on the fix branch :

popoverIsOpen = true
observable.reload()
// Step 1: ensure the hosting view is showing mainView with live data.
// navigate() to mainView was already called by popoverDidClose() or this is first open.
// Do NOT call mainView() again here — it resets savedNavState prematurely.

After observable.reload(), SwiftUI schedules a layout pass on the next run loop tick — it does NOT layout synchronously. So when you immediately call layoutSubtreeIfNeeded(), the view has stale data (empty or previous state) and fittingSize.height reflects that stale layout, not the newly loaded content. This means the popover opens at the wrong height for the current content count.

Bug 3 — mainView() passes isPopoverOpen: popoverIsOpen but popoverIsOpen is set to true after mainView() was last created

In popoverDidClose() , mainView() is called with isPopoverOpen: false (correct), and that view instance is stored in hostingController.rootView. Then in openPopover(), popoverIsOpen = true is set but the existing mainView() instance in the hosting controller still has isPopoverOpen: false — meaning store.reload() inside the timer is not gated correctly during this window. The isPopoverOpen prop needs to be updated on the view before the popover shows.

---

The Fix

Here are the exact changes needed on the fix branch:

Fix to AppDelegate.swift — openPopover()

The measurement timing is the core issue. You need to:

1. Call mainView() fresh with isPopoverOpen: true so the hosting controller has live props
2. Force a synchronous SwiftUI layout pass using CATransaction.flush() after setting the frame, not just layoutSubtreeIfNeeded() (which only flushes AppKit layout, not SwiftUI's render pass)
3. Fall back gracefully when fittingSize.height is 0

private func openPopover() {
    guard let button = statusItem?.button,
          button.window != nil,
          let popover,
          let hostingController
    else { return }

    popoverIsOpen = true
    observable.reload()

    // ✅ FIX: Swap to a fresh mainView with isPopoverOpen:true so the timer
    // guard is correct from this moment forward.
    hostingController.rootView = AnyView(PopoverMainView(
        store: observable,
        onSelectJob: /* ... existing closure ... */,
        onSelectAction: /* ... existing closure ... */,
        onSelectSettings: /* ... existing closure ... */,
        isPopoverOpen: true  // ← CRITICAL: was false in the popoverDidClose() copy
    ))

    // ✅ FIX: Give SwiftUI a full render pass at the measurement frame.
    // CATransaction.flush() forces SwiftUI's rendering pipeline to complete,
    // so fittingSize.height reflects the actual laid-out content — not 0.
    let screenHeight = NSScreen.main?.visibleFrame.height ?? 900
    hostingController.view.setFrameSize(NSSize(width: Self.idealWidth, height: screenHeight))
    hostingController.view.layoutSubtreeIfNeeded()
    CATransaction.flush()   // ← THIS IS THE KEY LINE — forces SwiftUI render

    let naturalHeight = hostingController.view.fittingSize.height
    let height = min(max(naturalHeight > 0 ? naturalHeight : 300, Self.minHeight), Self.maxHeight)

    let finalSize = NSSize(width: Self.idealWidth, height: height)
    hostingController.view.setFrameSize(finalSize)
    popover.contentSize = finalSize

    popover.show(relativeTo: button.bounds, of: button, preferredEdge: .maxY)
    popover.contentViewController?.view.window?.makeKey()

    if let saved = savedNavState,
       let restored = validatedView(for: saved) {
        navigate(to: restored)
    }
}

The critical addition is CATransaction.flush() . Without it, layoutSubtreeIfNeeded() flushes AppKit's layout tree but SwiftUI's internal render pass (which produces preferredContentSize and therefore fittingSize) hasn't completed. This is exactly the industry pattern described in the status-bar-app-position-warning.md guide :

"2. hc.view.layoutSubtreeIfNeeded() — force one layout pass at that width
3. Read hc.view.fittingSize.height — this is now the natural content height"

CATransaction.flush() ensures that step 2 is truly complete before step 3 reads.

Fix to PopoverMainView.swift — isPopoverOpen propagation

The mainView() factory needs to pass isPopoverOpen correctly when called from openPopover(). Currently the code avoids calling mainView() in openPopover() to prevent resetting savedNavState, but the side effect is the view gets stale isPopoverOpen: false. The solution: extract the PopoverMainView construction into the hosting controller rootView assignment with the correct flag, separately from the savedNavState logic.

Also, update popoverDidClose() to explicitly pass isPopoverOpen: false:

func popoverDidClose(_ notification: Notification) {
    popoverIsOpen = false
    DispatchQueue.main.async { [weak self] in
        guard let self else { return }\
        // ✅ CORRECT: isPopoverOpen:false here — popover is closed.
        self.hostingController?.rootView = self.mainView()  // mainView() reads self.popoverIsOpen which is now false
    }
}

The mainView() factory already reads self.popoverIsOpen at call time :

isPopoverOpen: popoverIsOpen

So the fix is ensuring openPopover() calls mainView() after setting popoverIsOpen = true, rather than relying on the stale view from popoverDidClose(). The revised openPopover() above does this correctly.

---

Why Main Branch Works

Main branch AppDelegate.swift uses fittingSize.width (which is flagged as dangerous in the guide ) but it works because PopoverMainView on main has no ScrollView — it's a flat VStack with ForEach rows. Without a ScrollView, fittingSize.height is correctly calculated after just layoutSubtreeIfNeeded(). The fix branch added ScrollView for the redesign, which is what breaks the measurement .

The fix branch AppDelegate is actually superior to main (always uses Self.idealWidth, never fittingSize.width) — it just needs the CATransaction.flush() to force the SwiftUI render pipeline to complete before reading fittingSize.height through a ScrollView.

---

Summary of Changes Needed

File	Change	Why
AppDelegate.swift → openPopover()	Add CATransaction.flush() after layoutSubtreeIfNeeded()	Forces SwiftUI render through ScrollView before reading fittingSize.height
AppDelegate.swift → openPopover()	Re-assign hostingController.rootView to a new mainView() with isPopoverOpen: true at start of openPopover()	Ensures the timer guard is active and the view has live data when measured
AppDelegate.swift → mainView()	Ensure isPopoverOpen: popoverIsOpen is passed (already correct — just must be called after popoverIsOpen = true)	Timer guard correctness
PopoverMainView.swift	Keep ScrollView + maxScrollHeight cap as-is	This is correct — the cap prevents fittingSize explosion with many action groups

The sizingOptions = [], never-touch-contentSize-after-show, always-use-idealWidth-for-width architecture on the fix branch is 100% correct . The single missing piece is CATransaction.flush() to synchronize SwiftUI's rendering pipeline before the height measurement.

and

Now I have a complete picture. Let me analyze the critical differences. The main branch works because:

1. It uses idealWidth: 420 (not 480)
2. It has no ScrollView — just a flat VStack with .prefix(5) on actions and .prefix(3) on jobs — so the height is always bounded naturally by the short fixed lists
3. No isPopoverOpen guard needed because there's no timer in the main branch's PopoverMainView

The redesign branch (fix/issue-294-popover-redesign-design-branch-3) introduces a ScrollView with a dynamic maxScrollHeight cap — but here's the real problem: ScrollView with maxHeight does NOT correctly report its preferredContentSize to NSHostingController. The ScrollView collapses its preferredContentSize.height to zero (or a non-deterministic value) because ScrollView is designed to fill whatever space it's given and doesn't report a natural preferred height.

Let me verify by checking if there's a specific issue with how the branch's PopoverMainView computes height with ScrollView:

The core insight from the status-bar-app-position-warning.md is precisely: "Wrapping the dynamic list view in ScrollView — ScrollView reports infinite preferred height to SwiftUI's layout system, making the popover enormous or unpredictable" .

The branch uses ScrollView containing the entire actions section with .frame(maxHeight: maxScrollHeight). This fights with sizingOptions = .preferredContentSize — ScrollView doesn't contribute a sensible preferred height, so preferredContentSize.height becomes either 0, enormous, or the full maxScrollHeight, not the actual content height.

The fix is to not use ScrollView for the main list — instead use a LazyVStack or simple VStack with a capped ForEach, OR if you must scroll, use a GeometryReader-measured content height approach. But the cleanest fix matching the proven architecture is to use a VStack with a fixedSize(horizontal: false, vertical: true) + .frame(maxHeight: maxScrollHeight) on the inner content, not on a ScrollView.

Here is the corrected PopoverMainView.swift body section. The key is replacing the ScrollView { VStack { actionsSection } }.frame(maxHeight: maxScrollHeight) with a structure that lets SwiftUI measure natural content height while still capping it:

// ✅ CORRECT — replaces the ScrollView block
VStack(alignment: .leading, spacing: 0) {
    actionsSection
}
.fixedSize(horizontal: false, vertical: true)   // measure natural content height
.frame(maxHeight: maxScrollHeight, alignment: .top)  // cap; DO NOT use ScrollView here

This tells SwiftUI to measure the actual content height (fixedSize vertical: true) and then cap it at maxScrollHeight. Because there's no ScrollView, preferredContentSize.height reflects actual content, not infinity. With sizingOptions = .preferredContentSize, NSPopover height tracks this correctly — no jump, no empty space.

Let me now push this fix to the branch:

and

Now I have a complete picture. I can see the fundamental problem clearly. Let me compose the definitive answer with precise code fixes:

The root cause is now crystal clear from reading all the code. The branch (fix/issue-294-popover-redesign-design-branch-3) uses sizingOptions = .preferredContentSize (the default — it's not overridden to []), which means every SwiftUI layout pass triggers preferredContentSize propagation to the popover → NSPopover re-anchors → side jump. But the branch also has a ScrollView wrapping the actionsSection with a maxScrollHeight cap, which fights against preferredContentSize reporting.

There are three concrete bugs in the branch versus main:

1. Branch uses sizingOptions = .preferredContentSize (default) but also has a ScrollView inside — ScrollView reports unbounded preferred height to preferredContentSize, and the maxScrollHeight cap via .frame(maxHeight:) doesn't constrain the preferred height, only the layout height. This creates a mismatch.

2. Branch sets contentSize only once (initial placeholder 300pt) but relies on preferredContentSize auto-propagation — except the isPopoverOpen guard prevents store.reload() while open, but systemStats (a @StateObject) still ticks every second and triggers layout passes while open, which propagate new preferredContentSize values → re-anchor.

3. StepLogView on the branch explicitly says ❌ NEVER add .idealWidth here and uses .frame(maxWidth: .infinity, maxHeight: .infinity) with no idealWidth — meaning when navigated to StepLogView, preferredContentSize.width becomes non-deterministic → side jump.

The fix is: adopt main's architecture exactly (measure fittingSize once before show(), sizingOptions = [], no auto-propagation) but adapt it for the new wider 480pt design with a ScrollView that has a capped maxHeight. Here is the precise fix:

---

The Diagnosis: 3 Bugs on the Branch

The branch's AppDelegate comment describes "Architecture 1" (preferredContentSize-driven) but the implementation has critical flaws that cause the jump. Here's exactly what's wrong and what to do:

Bug 1 — ScrollView + preferredContentSize = Infinite Height Reports

The branch uses sizingOptions = .preferredContentSize (default, never set to []) combined with a ScrollView in PopoverMainView. This is the forbidden combination documented in your own status-bar-app-position-warning.md :

"ScrollView reports infinite preferred height to SwiftUI's layout system, making the popover enormous or unpredictable."

NSHostingController with sizingOptions = .preferredContentSize asks SwiftUI for the view's ideal size. A ScrollView with unbounded content reports an enormous ideal height (the full unclipped content), regardless of your .frame(maxHeight: maxScrollHeight) cap — because maxHeight constrains layout height, not ideal/preferred height. Result: preferredContentSize.height jumps to 900+pt → NSPopover re-anchor → side jump.

Bug 2 — SystemStatsViewModel Ticks While Open

SystemStatsViewModel is a @StateObject that updates CPU/MEM stats. Even with the if !isPopoverOpen { store.reload() } guard correctly in place, systemStats updates are not guarded — they fire every ~1 second, mutate @ObservedObject/@StateObject state, trigger a SwiftUI layout pass, which propagates a new preferredContentSize to the popover → re-anchor → side jump every second.

Bug 3 — StepLogView Has No idealWidth

StepLogView on the branch explicitly says ❌ NEVER add .idealWidth here and uses .frame(maxWidth: .infinity, maxHeight: .infinity). But with sizingOptions = .preferredContentSize, navigating to this view makes preferredContentSize.width non-deterministic (can be 0, screen-width, or anything). Width change → re-anchor → side jump on navigation.

---

The Correct Fix: Adopt Main's Architecture for the New Design

main branch uses Architecture 2 (fittingSize-before-show, sizingOptions left as default []). The branch needs to do the same but with fixedWidth = 480 and a ScrollView with a proper height cap.

Fix 1 — AppDelegate.swift: Revert to fittingSize-before-show pattern

The branch's openPopover() is currently minimal and relies on auto-propagation. It needs to be changed to measure fittingSize before show(), exactly like main:

// ❌ BRANCH — openPopover() (BROKEN)
private func openPopover() {
    guard let button = statusItem?.button,
          button.window != nil,
          let popover
    else { return }
    popoverIsOpen = true
    observable.reload()
    popover.show(relativeTo: button.bounds, of: button, preferredEdge: .maxY)
    // ... no sizing at all — relies on preferredContentSize auto-propagation
}

// ✅ CORRECT — openPopover() (matches main's architecture, adapted for 480pt)
private func openPopover() {
    guard let button = statusItem?.button,
          button.window != nil,
          let popover,
          let hostingController
    else { return }
    popoverIsOpen = true
    observable.reload()
    // Measure BEFORE show(). fittingSize reflects the content AFTER reload().
    // Width: always Self.idealWidth (480). NEVER fittingSize.width — non-deterministic.
    let h = hostingController.view.fittingSize.height
    let size = NSSize(width: Self.idealWidth, height: max(h, 200))
    hostingController.view.setFrameSize(size)
    popover.contentSize = size
    popover.show(relativeTo: button.bounds, of: button, preferredEdge: .maxY)
    popover.contentViewController?.view.window?.makeKey()
    if let saved = savedNavState,
       let restored = validatedView(for: saved) {
        navigate(to: restored)
    }
}

The branch comment says sizingOptions = [] is wrong — this is incorrect for the redesigned view. The comment's reasoning (that fittingSize is stale before SwiftUI renders) is only valid if observable.reload() triggers async @ObservedObject updates. But RunnerStoreObservable.reload() uses withAnimation(nil) and is synchronous — by the time fittingSize is called, the layout is already up to date.

Fix 2 — PopoverMainView.swift: Replace ScrollView with fixedSize-capped list

The ScrollView is incompatible with preferredContentSize. If you keep sizingOptions = .preferredContentSize, you must remove the ScrollView. If you keep the ScrollView, you must switch to sizingOptions = [] with manual fittingSize measurement. Use main's architecture (no auto-propagation, measure once):

// ❌ BRANCH — ScrollView is incompatible with preferredContentSize
ScrollView {
    VStack(alignment: .leading, spacing: 0) {
        actionsSection
    }
}
.frame(maxHeight: maxScrollHeight)   // constrains layout height but NOT preferred height

// ✅ CORRECT — use fixedSize + frame(maxHeight:) which IS compatible with fittingSize
actionsSection
    .fixedSize(horizontal: false, vertical: true)   // measure natural content height
    .frame(maxHeight: maxScrollHeight, alignment: .top)  // cap it
// ⛔ DO NOT wrap in ScrollView — ScrollView reports infinite ideal height
// ✅ Overflow beyond maxScrollHeight will simply be clipped (acceptable trade-off,
//    use "Load 10 more" pagination button to keep content within cap)

If you must have scrolling for actions, the alternative is to keep the ScrollView but add .frame(idealHeight: fixedCapHeight) on it to give preferredContentSize a stable height to report — but this means empty-state will have empty space at the bottom. The cleanest solution is: no ScrollView on the main list + pagination button keeps items ≤ 10 visible.

Fix 3 — StepLogView.swift: Add idealWidth back

Remove the ❌ NEVER add .idealWidth here instruction in StepLogView. With sizingOptions = [] and fittingSize-before-show, StepLogView doesn't need idealWidth at all (since the frame is already set and won't auto-propagate). But if you're keeping sizingOptions = .preferredContentSize, every view in the nav tree must declare .frame(idealWidth: 480).

// ✅ For the fittingSize architecture (recommended): no change needed in StepLogView
// The popover frame is already set before show(), navigate() is a pure rootView swap,
// and StepLogView fills the existing frame with .frame(maxWidth: .infinity, maxHeight: .infinity)

// ✅ For the preferredContentSize architecture (NOT recommended for this view):
// .frame(idealWidth: 480, maxWidth: .infinity, maxHeight: .infinity, alignment: .top)

Fix 4 — PopoverMainView.swift: Gate systemStats updates while open

// ❌ BRANCH — systemStats ticks unconditionally, triggering layout passes while open
@StateObject private var systemStats = SystemStatsViewModel()

// ✅ CORRECT — stop systemStats while popover is open (or pause its @Published updates)
// In startRunnerRefreshTimer(), also gate systemStats:
runnerRefreshTimer = Timer.scheduledTimer(withTimeInterval: 5, repeats: true) { _ in
    LocalRunnerStore.shared.refresh()
    if !isPopoverOpen {
        store.reload()
        // Also refresh systemStats only when closed, OR use a separate stats source
        // that does NOT mutate @ObservedObject while popover.isShown
    }
}

---

Summary: Exactly What to Change

File	Change
AppDelegate.swift	In openPopover(): add fittingSize measurement + setFrameSize + contentSize = size before show(). Keep sizingOptions unset (defaults to []). Remove the branch's assertion that sizingOptions = [] is wrong.
PopoverMainView.swift	Replace ScrollView { actionsSection }.frame(maxHeight:) with actionsSection.fixedSize(horizontal:false, vertical:true).frame(maxHeight:, alignment:.top). Remove ScrollView. The pagination button already caps content.
StepLogView.swift	Remove the ❌ NEVER add .idealWidth here instruction. With sizingOptions = [], StepLogView is fine as-is (.frame(maxWidth:.infinity, maxHeight:.infinity)). No idealWidth needed.
All detail views	Verify they all have .frame(idealWidth: 480, maxWidth:.infinity) — or at minimum .frame(maxWidth:.infinity, maxHeight:.infinity) so they fill the fixed frame set by openPopover().
systemStats timer	Gate systemStats updates behind !isPopoverOpen to prevent layout passes while open.

The main branch proves this architecture works: measure fittingSize once before show(), width is always Self.fixedWidth, navigate() is a pure rootView swap with zero sizing. The redesign branch just needs to adopt the same contract at 480pt width instead of 340pt.