Support sizing of Compose hosting views on iOS#2984
Support sizing of Compose hosting views on iOS#2984
Conversation
c3fcdd8 to
9d38020
Compare
|
Marked as draft as I still need to add tests for the UIKit/SwiftUI intrinsic sizing path. |
28c50d6 to
5b3ada9
Compare
5b3ada9 to
0ae5bce
Compare
| /** | ||
| * Returns the current content size (in pixels) measured with the provided [constraints]. | ||
| * | ||
| * @throws IllegalStateException when [ComposeScene] content has lazy layouts without maximum | ||
| * size bounds (e.g. LazyColumn without maximum height) and the constraints are unbounded. | ||
| */ |
There was a problem hiding this comment.
This comment is only correct if the relevant dimension is unconstrained (e.g. height=Infinity).
There was a problem hiding this comment.
so "the constraints are unbounded" doesn't reflect the behavior correctly? It seems to me that the doc is the same as for unconstrainedSize() only specifies the unbounded constraints in addition which is exactly what happens in unconstrainedSize()
There was a problem hiding this comment.
Sorry, you're right. I didn't read the comment to the end, though it was just copy/pasted from unconstrainedSize.
I would just clarify in the comment that this applies on each axis separately.
| * platform integrations to avoid retain cycles between a hosting container and | ||
| * the scene/root. | ||
| */ | ||
| fun registerOnLayoutCompletedListener(listener: () -> Unit): AutoCloseable |
There was a problem hiding this comment.
Not a blocker, but I'd slightly prefer a dedicated class instead of AutoCloseable.
Existing examples:
PinnableContainer.PinnedHandleDelegatableNode.RegistrationHandle(maybe even just use this)
There was a problem hiding this comment.
added a custom OnLayoutCompletedListenerHandle
MatkovIvan
left a comment
There was a problem hiding this comment.
I don't think it's the right way to support it.
The root problem now is that compose stages are not aligned with the platform framework.
We already have an item to do it properly, let's combine the efforts to do it right instead of redoing things again later
| * the listener. This is important for platform integrations to avoid retain cycles between | ||
| * a hosting container and the scene/root. | ||
| */ | ||
| fun registerOnLayoutCompletedListener(listener: () -> Unit): OnLayoutCompletedListenerHandle |
There was a problem hiding this comment.
It's supposed to be a separate "layout" call from the platform (TBD). The scene itself shouldn't trigger layout independently, so such event shouldn't exist/be required
There was a problem hiding this comment.
Could you please clarify for me: We already have an item to do it properly ?
Do you mean
- we already have a way to do this properly?
- we have a TODO item to do it properly?
If the second, do we have some more description as to what is missing and what we require?
| * `sizeThatFits` / intrinsic sizing). | ||
| */ | ||
| @ExperimentalComposeUiApi | ||
| var useSelfSizing: Boolean = false |
There was a problem hiding this comment.
Do we need this option? I assumed that self-sizing (like, providing intrinsicContentSize) should be always ON.
There was a problem hiding this comment.
I added this flag so we don't change behavior for existing users. If we turn it on by default, it will affect what they already have. Do we want that, or keep it opt-in for now?
There was a problem hiding this comment.
Hm.. Previously we didn't have any constraints there - so users must configure them explicitly. I expect that transition should go smooth to the new state in most cases.
It looks like a safe option here is to make useSelfSizing = true by default, with "Breaking Change" release note and also with intent to remove the flag in the next version (1.13).
|
|
||
| rootViewController.view.let { | ||
| if (configuration.useSelfSizing) { | ||
| it.addSubview(hostingView) |
There was a problem hiding this comment.
Not sure I get the point. If we're adding view with the translatesAutoresizingMaskIntoConstraints == true, it means the frame of this view must be manually adjusted somewhere (btw, I didn't find the place where it happened). The idea behind the "Self Sizing" Compose views was to make it work correctly inside Autolayout and in the SwiftUI system. No sure we should manually update the frame.
| invalidateComposeSceneContainerSize = { | ||
| // SwiftUI observes the hosting view’s intrinsic size, not the internal Compose | ||
| // container view. | ||
| view.superview?.invalidateIntrinsicContentSize() |
There was a problem hiding this comment.
The comment is true, but in case of ComposeHostingViewController, the view.superview will refer the user's view, not a compose-related one.
This PR adds a sizing integration that lets a Compose-hosting UIView participate in UIKit/SwiftUI sizing loops by reporting Compose’s preferred size for a given sizing proposal.
Fixes CMP-5788 iOS investigate intrinsic sizing of ComposeUIViewController
Fixes CMP-5873 [Epic] iOS intrinsic sizing of interop elements
How it works
sizeThatFits(...)with a proposal (includingUIViewNoIntrinsicMetricfor unbounded axes).Fallback handling uses
super.sizeThatFitsfor non-intrinsic axes to avoid 0 sizes before Compose has produced a measurement.Public API changes
useSelfSizingtoComposeContainerConfiguration. Default isfalseto preserve existing behavior. When enabled, the container can size itself to fit Compose content under UIKit/SwiftUI size proposals.Example usage
(SwiftUI, iOS 16+): sizeThatFits
On the Kotlin side, enable the feature (defaults to false):
On the SwiftUI side, wrap the returned
UIView/UIViewControllerand forward SwiftUI’s proposal to UIKitsizeThatFits:(SwiftUI, iOS <16): sizeThatFits
The same mechanism works via intrinsic sizing alone: SwiftUI reacts to invalidateIntrinsicContentSize() and re-queries layout.
Testing
Adds instrumented tests that simulate the SwiftUI sizing loop (proposal →
sizeThatFits→ apply frame → wait for intrinsic invalidation → repeat) and validate that Compose scene size and hosting view frame converge as Compose content or size proposed by SwiftUI changes.This should be tested by QA
Release Notes
Features - iOS
ComposeHostingView/ComposeHostingViewController) used on the UIKit / SwiftUI side. Enabled by default; opt out viaComposeContainerConfiguration.useSelfSizing.Breaking Changes - iOS
ComposeContainerConfiguration.useSelfSizingallows opting out. Set it tofalseto preserve the previous behavior. This flag is temporary and will be removed in version 1.13.0.