-
Notifications
You must be signed in to change notification settings - Fork 74
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
Host-side API for view-owned widgets #2458
Conversation
This introduces a new mutable property, windowInsets, that is propagated to the composition where it can be consumed. We put the burden on the user of RedwoodView to set this value to something non-zero if they have insets to be consumed. I've added an accelerator on Android for this, RedwoodLayout.consumeWindowInsets(mask: Int)
c5b65db
to
c1ac72b
Compare
@@ -112,7 +118,7 @@ public open class RedwoodLayout( | |||
|
|||
private fun computeUiConfiguration( | |||
config: Configuration = context.resources.configuration, | |||
insets: Insets = rootWindowInsetsCompat.safeDrawing, |
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.
☠️
This is the core problem of the old API. If we end up calling this function for a configuration change, it forgets what the view-local insets were and reverts to the window insets.
* | ||
* See https://developer.android.com/develop/ui/views/layout/edge-to-edge | ||
*/ | ||
public val viewInsets: Margin = Margin.Zero, |
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.
Unfortunately safeAreaInsets
are potentially non-zero on already-shipping clients, but they don’t necessarily apply to the view we want them to.
Instead I’m creating another property that’s my attempt at fixing the window-local vs. view-local problem.
val value2 = awaitItem() | ||
assertThat(value2.safeAreaInsets).isEqualTo(Margin.Zero) | ||
assertThat(value2.viewInsets).isEqualTo(expectedInsets) |
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.
Change log item for this behavior change? Probably want to give a heads-up to internal users as well, if they still exist, that is.
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.
Yep, lemme do.
This introduces a new mutable property, windowInsets, that is propagated to the composition where it can be consumed.
We put the burden on the user of RedwoodView to set this value to something non-zero if they have insets to be consumed. I've added an accelerator on Android for this,