-
Notifications
You must be signed in to change notification settings - Fork 177
[fluentui-apple] Have resizingHandleView overlap content in BottomSheetController #2236
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
base: main
Are you sure you want to change the base?
Conversation
| ]) | ||
|
|
||
| if let headerContentView { | ||
| resizingHandleContentOverlapConstraints = headerContentView.topAnchor.constraint(equalTo: resizingHandleView.bottomAnchor, constant: -currentResizingHandleHeight) |
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.
why not just constrain to top anchor instead of constraining to bottom anchor and then applying negative offset?
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.
Also the resizing view should just be added as a subview of the bottomSheetContentView. It shouldn't matter what's in the content, just that we overlay on top. And then depending on the bool value, we either shift the content stack down by the resizing handle height, or not. No constraint activating / deactivating needed.
| } else { | ||
| resizingHandleContentOverlapConstraints = expandedContentView.topAnchor.constraint(equalTo: resizingHandleView.bottomAnchor, constant: -currentResizingHandleHeight) | ||
| } | ||
| stackView.bringSubviewToFront(resizingHandleView) |
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.
I believe uikit works in order so if you add resizing handle last, you won't have to call bring subview to front
| private func updateResizingHandleConstraints() { | ||
| if let resizingHandleContentOverlapConstraints { | ||
| if shouldResizingViewOverlayContent { | ||
| NSLayoutConstraint.activate([resizingHandleContentOverlapConstraints]) |
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.
will there not be conflicting constraints now?
| stackView.bottomAnchor.constraint(equalTo: bottomSheetContentView.bottomAnchor) | ||
| ]) | ||
|
|
||
| if let headerContentView { |
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.
combine with the if let on line 706
|
nit: update view hierarchy on line 470 |
| if let headerContentView { | ||
| resizingHandleContentOverlapConstraints = headerContentView.topAnchor.constraint(equalTo: resizingHandleView.bottomAnchor, constant: -currentResizingHandleHeight) | ||
| } else { | ||
| resizingHandleContentOverlapConstraints = expandedContentView.topAnchor.constraint(equalTo: resizingHandleView.bottomAnchor, constant: -currentResizingHandleHeight) |
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.
we'll need to account for new height in expandedSheetHeight/collapsedSheetHeight
Platforms Impacted
Description of changes
In iOS 26, the bottom sheet’s content is designed to align concentrically with the sheet’s rounded corners. However, the presence of the
resizingHandleViewintroduces a visible gap between the content and the corner, breaking this concentric alignment.To address this, the
resizingHandleViewneeds to overlap the bottom sheet’s content, so I’m introducing the new property,shouldResizingViewOverlayContent, which enables this behavior when set to true (the default is false).When this option is enabled, the layout applies
resizingHandleContentOverlapConstraintswhich shifts the headerContentView orexpandedContentViewup and also ensures that theresizingHandleViewis brought to the front of the view hierarchy so it appears above the content as intended.Binary change
(how is our binary size impacted -- see https://github.com/microsoft/fluentui-apple/wiki/Size-Comparison)
Verification
Visual Verification
Before
https://github.com/user-attachments/assets/6ac7b1fc-4a90-45ec-a95d-35f942ba2ee5
After
https://github.com/user-attachments/assets/450286ad-deab-4fe9-8dea-056e7a726304
Pull request checklist
This PR has considered:
Microsoft Reviewers: Open in CodeFlow