-
Notifications
You must be signed in to change notification settings - Fork 0
Fix layout-pass timing race in ScrollExpansion.updateHeight #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
78128f5
2135cf9
c632838
f3c93b5
4f7ce81
d5abaf4
5194478
79608ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -115,6 +115,22 @@ final class AppKitContainer: NSHostingController<EmergeModifierView>, ScrollExpa | |
| } | ||
| var heightAnchor: NSLayoutConstraint? | ||
| var previousHeight: CGFloat? | ||
| var pendingContentSizeRetries: Int = 0 | ||
| var lastObservedContentHeight: CGFloat? | ||
| var pendingIntrinsicSizeRetries: Int = 0 | ||
| var lastObservedIntrinsicSize: CGSize? | ||
|
|
||
| var hostFittingSize: CGSize? { | ||
| // NSView's `fittingSize` is the AppKit equivalent of UIKit's | ||
| // systemLayoutSizeFitting(compressedSize) and honors active layout | ||
| // constraints — same role in the settle loop. | ||
| view.fittingSize | ||
| } | ||
|
|
||
| func setNeedsAnotherLayoutPass() { | ||
| view.needsLayout = true | ||
| updateScrollViewHeight() | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
Outdated
Comment on lines
+130
to
+133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
|
|
||
| public var rendered: ((EmergeRenderingMode?, Float?, Bool?, Bool?) -> Void)? { | ||
| didSet { didCall = false } | ||
|
|
@@ -143,6 +159,11 @@ final class AppKitContainer: NSHostingController<EmergeModifierView>, ScrollExpa | |
| widthAnchor?.isActive = false | ||
| heightAnchor = nil | ||
| widthAnchor = nil | ||
| previousHeight = nil | ||
| pendingContentSizeRetries = 0 | ||
| lastObservedContentHeight = nil | ||
| pendingIntrinsicSizeRetries = 0 | ||
| lastObservedIntrinsicSize = nil | ||
| } | ||
|
|
||
| public func setupView(layout: PreviewLayout) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AppKit
hostFittingSizeoverride prevents intended immediate completionHigh Severity
AppKitContaineroverrideshostFittingSizeto returnview.fittingSize(non-nil, typically non-zero), but itssetNeedsAnotherLayoutPass()is a no-op (protocol default). The comment on lines 130–134 states the settle loop "falls through to immediate completion," which only works ifhostFittingSizereturnsnil(the protocol extension default). With a real value returned, the non-scroll settle loop defers on its first observation (sincelastObservedIntrinsicSizeis nil ≠fittingSize) and calls the no-op, soupdateScrollViewHeight()may never be re-invoked — hanging therenderedcallback indefinitely. The same issue affects the scroll path's newpendingContentSizeRetriesguard.Additional Locations (1)
Sources/SnapshotPreviewsCore/ScrollExpansion.swift#L121-L136Reviewed by Cursor Bugbot for commit 79608ca. Configure here.