Fall back to opening the goal popover below, tracked via a scroll-safe coordinate space - #53
Draft
gsbernstein wants to merge 5 commits into
Draft
Fall back to opening the goal popover below, tracked via a scroll-safe coordinate space#53gsbernstein wants to merge 5 commits into
gsbernstein wants to merge 5 commits into
Conversation
…the screen Co-authored-by: Greg <gsbernstein@users.noreply.github.com>
…iOS 18+ Co-authored-by: Greg <gsbernstein@users.noreply.github.com>
Co-authored-by: Greg <gsbernstein@users.noreply.github.com>
Co-authored-by: Greg <gsbernstein@users.noreply.github.com>
The previous GeometryReader+.global+PreferenceKey approach didn't reliably update during interactive scrolling, so the computed arrow edge was based on a stale position (effectively wherever the button was when the view last re-rendered, not its actual current on-screen position). Using a named coordinate space anchored to the main ScrollView's own (non-scrolling) viewport, read via onGeometryChange, tracks the button's position within that viewport correctly and continuously as the user scrolls. Co-authored-by: Greg <gsbernstein@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Re-implemented the "prefer above, fall back to below when there isn't room" behavior, this time using a technique that actually tracks the button's position correctly while scrolling.
Why the previous attempt failed
The earlier version used the common
GeometryReader+.background+.global+PreferenceKeypattern to read the goal button's on-screen position. That pattern is well known to not reliably update during interactiveScrollViewscrolling —.globalframe values tend to only get recomputed when something else forces a re-render of that subtree, not simply because the user dragged the scroll view. In practice this meant the "space available above the button" was based on a stale position (most likely wherever the button was when the screen first appeared), so the fallback logic was comparing against the wrong numbers and would pick the wrong edge almost regardless of actual current scroll position.The fix
ContentView'sScrollViewnow declares a named coordinate space (Constants.mainScrollCoordinateSpaceName) and publishes its own viewport size via the newscrollViewportSizeenvironment value, measured withonGeometryChange(the ScrollView's own frame doesn't move as its content scrolls, so this value is stable/correct).SleepBankCard's goal button reads its framein: .named(mainScrollCoordinateSpaceName)viaonGeometryChange, which — unlike.global, per the documentedScrollView"frame layer" technique — does update correctly and continuously as the user scrolls, since it's relative to the ScrollView's own fixed viewport rather than the whole screen.goalPopoverArrowEdgecompares the button's live position within that viewport against an estimated popover height to decide whether to open above (arrowEdge: .bottom, preferred) or below (arrowEdge: .top, fallback) — now driven by continuously-correct data instead of stale/guessed data.Files changed
Bedtime/Bedtime/Constants.swift: newmainScrollCoordinateSpaceName.Bedtime/Bedtime/ContentView.swift: declares the coordinate space on the mainScrollViewand publishes its viewport size via environment.Bedtime/Bedtime/Views/SleepBankCard.swift: reads the goal button's frame in that coordinate space and computes the arrow edge from it; preview updated to set up the same coordinate space/environment so it stays interactive.Verification
This is a native iOS/SwiftUI change. Per the repo's
AGENTS.md, this Cloud Agent environment runs Linux and cannot build/run Xcode or the iOS Simulator, so this has not been visually verified. Please confirm on device/Xcode that: