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 display cutout insets as well as the raw window insets #685

Merged
merged 2 commits into from Sep 4, 2021

Conversation

ansman
Copy link
Contributor

@ansman ansman commented Aug 27, 2021

This fixes #681

@google-cla google-cla bot added the cla: yes label Aug 27, 2021
*
* Prefer using one of the predefined types instead of this.
*/
val rawInsets: WindowInsetsCompat
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are exposed because copying all of the intricacies of DisplayCutout isn't feasible so users who require advanced insets handling can use this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure on this. We specifically want to keep Android types out of the API surface.

What's the use case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can agree with that. The use case would be more advanced display cutout handling such as knowing the exact path or something like that. For my use case the safe insets are typically fine.

Another option is to surface it as a separate composition local that is android specific.

The problem in general is that a view can only have a single insets listener so if you use accompanist you cannot access the underlying insets and you have to wait for a PR to fix things that is missing from the API.

I'm OK with removing it for now though.

@ansman
Copy link
Contributor Author

ansman commented Aug 27, 2021

While converting our old code I found that most apps probably want to use systemBars() or displayCutout() since it covers the most common cases. Is that something that accompanist should expose too?

Copy link
Contributor

@chrisbanes chrisbanes left a comment

Choose a reason for hiding this comment

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

Thanks for the PR

@ansman
Copy link
Contributor Author

ansman commented Sep 2, 2021

I removed the raw insets.

@@ -79,12 +79,14 @@ package com.google.accompanist.insets {
}

@androidx.compose.runtime.Stable public interface WindowInsets {
method public default com.google.accompanist.insets.WindowInsets copy(optional com.google.accompanist.insets.WindowInsets.Type navigationBars, optional com.google.accompanist.insets.WindowInsets.Type statusBars, optional com.google.accompanist.insets.WindowInsets.Type systemGestures, optional com.google.accompanist.insets.WindowInsets.Type ime);
method public default com.google.accompanist.insets.WindowInsets copy(optional com.google.accompanist.insets.WindowInsets.Type navigationBars, optional com.google.accompanist.insets.WindowInsets.Type statusBars, optional com.google.accompanist.insets.WindowInsets.Type systemGestures, optional com.google.accompanist.insets.WindowInsets.Type ime, optional com.google.accompanist.insets.WindowInsets.Type displayCutout);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It binary compatibility is important we'd have to add another copy method

@chrisbanes chrisbanes merged commit 1b42fe0 into google:main Sep 4, 2021
@chrisbanes
Copy link
Contributor

Thanks!

@ansman ansman deleted the feature/display-cutout branch September 7, 2021 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for display cutout insets
2 participants