Modal/Standard Bottomsheet size change changes Expanded state to PartiallyExpanded [324934884]

Fixed

Bug

Status Update

No update yet.

Description

ma...@wmg.com

created issue #1

Feb 12, 2024 09:49PM

Jetpack Compose version: 1.5.9 Jetpack Compose component used: BottomModalSheet Android Studio Build: Hedgehog 2023.1.1 Patch 2 Kotlin version: 1.9.22

Steps to Reproduce or Code Sample to Reproduce:

We have a ModalBottomSheet with a sticky TextField on the bottom. We allow both PartiallyExpanded and Expanded states. Whenever we are in the Expanded state and focus on the TextField, the resulting screen resize from the keyboard showing causes the ModalBottomSheet to switch its target state to PartiallyExpanded.

This is a pretty jarring UI experience and ideally we would just stay in the Expanded state.

I've traced it down to these lines in ModalBottomSheet.android.kt that prioritize PartiallyExpanded whenever the anchors change (via the screen size changing).

Here is a sample project and video is included:

@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun Screen() {
    var isShowingDialog by remember { mutableStateOf(false) }
    var text by remember { mutableStateOf("") }

    var modalHeight by remember { mutableIntStateOf(0) }
    val sheetState = rememberModalBottomSheetState()

    Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
        Button(onClick = { isShowingDialog = true }) {
            Text(text = "Show dialog")
        }
    }

    if (isShowingDialog) {
        ModalBottomSheet(
            onDismissRequest = { isShowingDialog = false },
            modifier = Modifier.onGloballyPositioned { modalHeight = it.size.height },
            sheetState = sheetState,
        ) {
            Column {
                Text("Content", modifier = Modifier.weight(1f))
                TextField(
                    value = text,
                    onValueChange = { text = it},
                    modifier = Modifier
                        .padding(16.dp)
                        .fillMaxWidth()
                        .offset {
                            IntOffset(
                                x = 0,
                                y = -sheetState
                                    .requireOffset()
                                    .toInt()
                                    .coerceAtMost(modalHeight / 2)
                            )
                        }
                )
            }
        }
    }
}

modalbottomsheet.mp4

2.2 MB

Download

Comments

ma...@wmg.com <ma...@wmg.com> #2Feb 12, 2024 09:53PM

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

ma...@wmg.com <ma...@wmg.com> #3Feb 12, 2024 10:10PM

My proposed solution would be:

val targetValue = sheetState.anchoredDraggableState.targetValue
val newTarget = when (targetValue) {
  Hidden -> Hidden
  PartiallyExpanded, Expanded -> {
    when {
      newAnchors.hasAnchorFor(targetValue)) -> targetValue
      newAnchors.hasAnchorFor(PartiallyExpanded)) -> PartiallyExpanded
      newAnchors.hasAnchorFor(Expanded)) -> Expanded 
      else -> Hidden
    }
  }
}

Message last modified on Feb 12, 2024 10:12PM

ke...@google.com <ke...@google.com> Feb 13, 2024 03:44PM

Reassigned to ma...@google.com.

ap...@google.com <ap...@google.com> #4May 3, 2024 11:03PM

Project: chromium/src
Branch: main

commit 0fe5e3c558992a1c0b5b2aef2594049862b1c703
Author: Zach Katz <katzz@google.com>
Date: Fri May 03 23:02:15 2024

[ASH] Add fieldtrial_testing_config

Bug: 324934884
Change-Id: Ib9d0ade90999a67cd74d1b94e6ab43087b5e1ead
Reviewed-on:

https://chromium-review.googlesource.com/c/chromium/src/+/5516358
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Commit-Queue: Zach Katz <katzz@google.com>
Cr-Commit-Position: refs/heads/main@{#1296421}

M testing/variations/fieldtrial_testing_config.json

https://chromium-review.googlesource.com/5516358

64...@gmail.com <64...@gmail.com> #5Sep 22, 2024 08:40PM

any update on this? 😅

tj...@fieldwire.com <tj...@fieldwire.com> #6Oct 23, 2024 05:45PM

any updates?

se...@google.com <se...@google.com> Nov 8, 2024 09:57PM

Accepted by se...@google.com.

ap...@google.com <ap...@google.com> #7Nov 13, 2024 02:28PM

Project: platform/frameworks/support
Branch: androidx-main
Author: Jose Figueroa <serniebanders@google.com>
Link: https://android-review.googlesource.com/3341638

[M3][BottomSheets] Update Expanded target to remain expanded on resize.

Expand for full commit details

[M3][BottomSheets] Update Expanded target to remain expanded on resize. 
 
Expanded anchor is always defined. 
 
Bug: 324934884 
Test: Add Expanded check to anchor testing 
RelNote: "Expanded bottom sheet remains expanded on size change." 
Change-Id: I2870ba38420d18b02676e6f280ef96dddc6a3a8a

Files:

M compose/material3/material3/src/androidInstrumentedTest/kotlin/androidx/compose/material3/ModalBottomSheetTest.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/ModalBottomSheet.kt

Hash: 0a8673ffe397d5cfe341fa60a281212388c3f1e0
Date: Fri Nov 08 16:59:04 2024

se...@google.com <se...@google.com> Nov 13, 2024 02:31PM

Marked as fixed.

le...@gmail.com <le...@gmail.com> #8Dec 20, 2024 01:35AM

version 1.4.0-alpha05
not fixed

ri...@td.com <ri...@td.com> #9Feb 4, 2025 03:29PM

This behavior is pretty frustrating for our users, can we port this backwards to a stable version instead of an alpha version? I'd rather not consume an alpha version and ship it to production.