onSizeChanged reports size without padding [177562900]

Fixed

Bug

Status Update

No update yet.

Description

vs...@gmail.com

created issue #1

Jan 14, 2021 06:35PM

Jetpack Compose release version: 1.0.0-alpha10

Steps to Reproduce:

@Preview
@Composable
fun Test() {
    val density = AmbientDensity.current
    Box(
        modifier = Modifier
            .onSizeChanged { println(it) }
            .padding(with(density) { 50.toDp() })
            .size(with(density) { 50.toDp() })
            .background(Color.Red)
    )
}

Expected printout: 150 x 150
Actual printout: 50 x 50

On the other hand if I replace onSizeChanged with layout it prints expected result.

modifier = Modifier
    .layout { measurable, constraints ->
        val placeable = measurable.measure(constraints)
        println("${placeable.width} x ${placeable.height}")
        layout(placeable.width, placeable.height) {
            placeable.place(0, 0)
        }
    }
    .padding(with(density) { 50.toDp() })
    .size(with(density) { 50.toDp() })
    .background(Color.Red)

Comments

po...@google.com <po...@google.com> Jan 14, 2021 06:39PM

Assigned to an...@google.com.

an...@google.com <an...@google.com> #2Jan 14, 2021 07:10PM

Reassigned to mo...@google.com.

We will see :)

mo...@google.com <mo...@google.com> #3Jan 14, 2021 08:33PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit a9b67eca31e73a3f975b53e22a9e5983e2d76864
Author: Filip Pavlis <pavlis@google.com>
Date: Thu Oct 01 15:32:27 2020

Add FLAG_SECURE on Dialog and Popup.

Added a new way on how to pass platform specific parameters for popups
and dialogs.

Also renamed children to content. And made expect / actual to be
internal for dialog as it is for popup.

Bug: 143778148
Bug: 143778149
Test: Added tests for both dialog and popup.
Relnote: "Popups and dialogs now inherit FLAG_SECURE from parent Window.
Also added option to configure this explicitly".

Change-Id: I649663482e91757df751315b03fee9867b580e96

M compose/material/material/api/current.txt
M compose/material/material/api/public_plus_experimental_current.txt
M compose/material/material/api/restricted_current.txt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/AlertDialog.kt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/src/androidAndroidTest/AndroidManifest.xml
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/window/DialogSecureFlagTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/window/PopupDismissTest.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/window/PopupSecureFlagTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/window/PopupTestUtils.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/AndroidDialog.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/AndroidPopup.kt
A compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/SecureFlagPolicy.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/selection/SelectionLayout.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/window/Dialog.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/window/Popup.kt
M compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/window/DesktopDialog.kt
M compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/window/DesktopPopup.kt

https://android-review.googlesource.com/1444435

vs...@gmail.com <vs...@gmail.com> #4Jan 14, 2021 09:13PM

Thanks for your feedback! We have just landed a CL that enables to configure a FLAG_SECURE on a dialog. By default it will inherit it from window launching it, so all compose popups from now on should inherit the flag. The flag can be also configured for AlertDialog. This will be available in the next release.

ae...@google.com <ae...@google.com> #5Jan 14, 2021 09:58PM

I am tremendously grateful that you added fixes for this and https://issuetracker.google.com/issues/143778148!

For testing purposes, by "the next release", do you mean alpha05? I don't know when your internal cutoff is for what makes it into a particular release.

Thanks again!

ap...@google.com <ap...@google.com> #6Jan 19, 2021 08:27PM

Marked as fixed.

Sorry, yes you are right, alpha05 is the release that will have this change included. Thanks!

Issue 177562900