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

[Permissions] Support calling rememberPermissionState in a Compose preview #1803

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

eygraber
Copy link
Contributor

@eygraber eygraber commented Dec 2, 2024

Spinning this out while the fate of #1793 hangs in the balance 😅

@eygraber eygraber force-pushed the permission-in-previews branch from f54f01e to a3c24c2 Compare December 2, 2024 20:27
@@ -36,10 +36,12 @@ import androidx.compose.ui.util.fastMap
@Composable
public fun rememberMultiplePermissionsState(
permissions: List<String>,
onPermissionsResult: (Map<String, Boolean>) -> Unit = {}
onPermissionsResult: (Map<String, Boolean>) -> Unit = {},
permissionStatuses: Map<String, PermissionStatus> = emptyMap()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this would prevent a release because of the API change I can move it to a separate PR. Not sure what the policy is with default params, but metalavaGenerateSignatureRelease seems to treat it as an API change (also see next commit where I address this with JvmOverloads).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, default params don't fix binary compat unfortunately. Just add a new function and then make this one call your new one and then it is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the JvmOverloads work for that, or does there have to be an actual new function in source?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good question, I'm not 100% sure actually but on Compose we always do it with a new function in source so I assume there is a reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I'll push that shortly.

@eygraber eygraber force-pushed the permission-in-previews branch from a3c24c2 to fadb0fa Compare December 9, 2024 00:28
@eygraber eygraber force-pushed the permission-in-previews branch from fadb0fa to 2d05d95 Compare December 9, 2024 00:29
@eygraber eygraber requested a review from bentrengrove December 9, 2024 00:30
Copy link
Collaborator

@bentrengrove bentrengrove left a comment

Choose a reason for hiding this comment

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

Thank you for this!

@bentrengrove bentrengrove merged commit bb1d0e7 into google:main Dec 9, 2024
9 checks passed
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.

2 participants