Skip to content
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

Expose the host's safe area insets in HostConfiguration. #913

Merged
merged 5 commits into from
Apr 18, 2023

Conversation

colinrtwhite
Copy link
Member

@colinrtwhite colinrtwhite commented Apr 16, 2023

Adds a new safeAreaInsets property to HostConfiguration. This lets us modify the UI to account for a device's reserved system UI area if necessary. It can be combined with the other layout components like so:

val configuration = LocalHostConfiguration.current

// Column
Column(
  margin = configuration.safeAreaInsets,
) {
  // Content
}

// Spacer
Spacer(
  height = configuration.safeAreaInsets.top
)
// Content

Eventually we can add an argument to automatically inset the TreehouseView based on the device's insets, however I opted not to implement that yet as we'll probably need to do it without redwood-layout and Spacer.

Copy link
Collaborator

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with moving forward with this as-is for now. The tests and support for all platforms is great!

However, long term, I think we should change the approach to be more like Compose UI. This would mean two things:

  • The insets are available as their own composition local within pure Redwood (not Treehouse).
  • The insets are consumable rather than static so that they reflect the composables position in the native UI toolkit view hierarchy.

Maybe we can track that in an issue and figure out the right way to do it over time.

@Composable
public actual fun safeAreaInsets(): Margin {
val layoutDirection = LocalLayoutDirection.current
return WindowInsets.systemBars.asPaddingValues().toMargin(layoutDirection)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably want to use safeDrawing insets here (link). System bars only contains the nav/status bars, but we also need to handle IME and cutouts.

// Temporary constructor before migrating everything to doubles.
@Stable
public fun Margin(
left: Double = 0.0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meta comment: no start/end?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! Will fix this soon. I changed this to left/right as I'm pretty sure we're not handling LTR properly in some spots.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, there was a bit of a discussion about it here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah cool, thanks!

@chrisbanes
Copy link
Contributor

+1 to Jake's comment about making these consumable in the long term.

I'm personally not a huge fan of the way that Compose UI exposes this though, but we can work out the mechanics later

@colinrtwhite
Copy link
Member Author

Sounds good! I'll file an issue to track better support for this. For the moment this unblocks one of our current screens that needs to know the status bar size to avoid accidentally drawing under it.

However, long term, I think we should change the approach to be more like Compose UI.

Makes sense, though I'm curious what the API would look like for consuming them. Also I think we'll need to avoid applying them as a Modifier given how Redwood modifiers are currently only read by the parent widget.

@colinrtwhite colinrtwhite enabled auto-merge (squash) April 18, 2023 00:40
@colinrtwhite colinrtwhite merged commit 4708310 into trunk Apr 18, 2023
@colinrtwhite colinrtwhite deleted the colin/safe_area_insets branch April 18, 2023 01:15
colinrtwhite added a commit that referenced this pull request Apr 19, 2023
* trunk:
  Ensure children lambda type returns Unit (#932)
  Respect event type nullability (#931)
  Update kotlin monorepo to v1.8.20 (#889)
  No default for embedded Zipline code (#926)
  Annotate schema properties, not parameters (#924)
  Propagate network failures in Emoji Search (#927)
  Cancel HTTP call on coroutine cancellation in Emoji Search (#928)
  Remove empty lambda from jvm target declaration (#925)
  Add LazyListScope#itemsIndexed (#917)
  Parse and generate deprecations from the schema (#922)
  Add LazyListScope#items(Array<T>, …) and LazyListScope#itemsIndexed(Array<T>, …) (#918)
  Add LazyListScope#item (#916)
  Expose the host's safe area insets in HostConfiguration. (#913)
  Move LazyListScope to LazyDsl.kt (#915)
  Update jbCompose to v1.4.0 (#904)
  Update dependency com.vanniktech:gradle-maven-publish-plugin to v0.25.2 (#914)
  Update dependency com.android.tools.lint:lint to v31 (#911)
  Build treehouse-lazylayout-compose on all platforms (#909)
  Add instructions for building the Counter app for iOS/Android. (#906)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants