AndroidView with weight in a Column consumes all available space [274797771]

Fixed

Bug

Status Update

No update yet.

Description

di...@gmail.com

created issue #1

Mar 27, 2023 02:36PM

Jetpack Compose version: 1.4.0 Jetpack Compose component used: AndroidView Android Studio Build: Giraffe Canary 10 Kotlin version: 1.8.10

Given this setup:

Column {
  AndroidView(
    ...,
    Modifier.weight(1f)
  )
  Box(Modifier.height(100.dp))
}

In Compose 1.3, the AndroidView fills up most of the column, leaving 100dp for the Box.

In Compose 1.4, the AndroidView fills up the whole Column, leaving no room for the Box.

Sample project attached.

AndroidViewWeightBug.zip

498 KB

Download

Comments

sj...@gmail.com <sj...@gmail.com> #2Mar 27, 2023 02:40PM

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

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

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

as...@google.com <as...@google.com> #3Mar 27, 2023 04:18PM

The issue is caused by parent data modifier not being updated on layout node inside AndroidViewHolder.

parentData should be invalidated in measure delegate when the corresponding Modifier.Node is attached. However, when LayoutNode is detached, this invalidate call only marks parentDataDirty on attach, and nothing calls updateParentData again until modifier is updated.

Message last modified on Mar 27, 2023 05:56PM

ga...@gmail.com <ga...@gmail.com> #4Mar 27, 2023 08:50PM

I assume that could also cause Modifier.align(BottomCenter) on an Android View inside a Box to not work? Just asking because if not I'll file a separate bug for that

ap...@google.com <ap...@google.com> #5Mar 28, 2023 05:15PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 10798cab474d687a3e2fb3879632b54208501bd6
Author: Andrei Shikov <ashikov@google.com>
Date: Tue Mar 28 06:15:39 2023

Invalidate LayoutDelegate parent data on attach

Ensures parent data is up to date in layout delegate if it has been changed since the node was created/detached. If the parentData is not dirty, `updateParentData` will result in noop.

Test: AndroidViewTest
Fixes: 274797771
Change-Id: Ib298ac822b35cfb0f4c97d86cd0723eb7f71b24e

M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/viewinterop/AndroidViewTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNode.kt

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

as...@google.com <as...@google.com> #6Mar 29, 2023 03:01AM

The fix should be part of the April 5th release.

sj...@gmail.com <sj...@gmail.com> #7Mar 29, 2023 05:04AM

Thank you!

na...@google.com <na...@google.com> #8Apr 5, 2023 06:27PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.ui:ui:1.4.1
androidx.compose.ui:ui:1.5.0-alpha02

la...@gmail.com <la...@gmail.com> #9Apr 8, 2023 04:55AM

Issue https://issuetracker.google.com/274741610 related to AndroidView is not yet resolved

mt...@gmail.com <mt...@gmail.com> #10Apr 8, 2023 02:52PM

I still observe the issue with 1.4.1 whereas it is fixed with 1.5.0-alpha02.

as...@google.com <as...@google.com> #11Apr 17, 2023 10:46PM

Status: Assigned (reopened)

Seems like 1.5.0-alpha02 also includes aosp/2470074, which forced the invalidation.

da...@gmail.com <da...@gmail.com> #12Apr 18, 2023 07:48AM

This should be fixes ASAP to 1.4.2, as like mentioned above it is still observed for 1.4.1

qw...@gmail.com <qw...@gmail.com> #13Apr 20, 2023 05:42AM

Reproduces even for 1.4.2

ae...@google.com <ae...@google.com> #14Apr 28, 2023 10:17PM

Marked as fixed.

The initial fix aosp/2510735 was cherry-picked into 1.4.1 and fixes part of the bug. aosp/2470074 provides a complete fix, and it's been cherry-picked (as aosp/2544354) into the upcoming 1.4.3 (to be released next week I think).

Therefore, any release at 1.5.0-alpha02 or 1.4.3 or newer should have this fixed.

na...@google.com <na...@google.com> #15May 3, 2023 06:24PM