Scaffold and BottomSheetScaffold top bar elevation handling Inconsistency [187771422]

Fixed

Bug

Status Update

No update yet.

Description

ze...@gmail.com

created issue #1

May 11, 2021 10:39AM

Jetpack Compose release version: 1.0.0-beta06

Android Studio Build: Android Studio Arctic Fox | 2020.3.1 Canary 15

Build #AI-203.7717.56.2031.7321754, built on April 29, 2021

Runtime version: 11.0.10+0-b96-7281165 x86_64

It seems that BottomSheetScaffold's content is overlapping topBar's elevation. Minimal code to reproduce:

import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.height
import androidx.compose.material.*
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp

@Composable
fun GoodElevation() {
    Scaffold(
        topBar = {
            TopAppBar(backgroundColor = MaterialTheme.colors.background) { Text(text = "The good") }
        },
    ) {
        Surface(modifier = Modifier.fillMaxSize()) {}
    }
}

@Composable
fun BadElevation() {
    val scaffoldState = rememberBottomSheetScaffoldState()
    BottomSheetScaffold(
        scaffoldState = scaffoldState,
        topBar = {
            TopAppBar(backgroundColor = MaterialTheme.colors.background) { Text(text = "The bad") }
        },
        sheetContent = {
            Box(modifier = Modifier.height(250.dp)) { Text("whatever") }
        }
    ) {
        Surface(modifier = Modifier.fillMaxSize()) {}
    }
}

@Preview
@Composable
fun TheGoodPreview() {
    GoodElevation()
}

@Preview
@Composable
fun TheBadPreview() {
    BadElevation()
}

Zrzut ekranu 2021-05-11 o 12.33.13.png

8.3 KB

View

Download

Zrzut ekranu 2021-05-11 o 12.33.18.png

9 KB

View

Download

Comments

ma...@google.com <ma...@google.com> May 11, 2021 02:13PM

Assigned to je...@google.com.

ni...@google.com <ni...@google.com> #2Jun 15, 2021 01:09PM

Status: New

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...@google.com <ma...@google.com> #3Jun 17, 2021 05:27PM

Assigned to je...@google.com.

moving particular sheets implementation bugs under material alias, keeping swipeable design-related issues to malkov@

jo...@google.com <jo...@google.com> Apr 8, 2022 05:56PM

Reassigned to jo...@google.com.

jo...@google.com <jo...@google.com> Apr 11, 2022 05:25PM

Accepted by jo...@google.com.

ap...@google.com <ap...@google.com> #4May 25, 2022 08:48AM

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

commit ee16c2fa8bfbd261a382f0c7fc51965357bb8155
Author: Jossi Wolf <jossiwolf@google.com>
Date: Tue Apr 12 16:59:15 2022

Draw order & Snackbar placement for BottomSheetScaffold

Previously, we were drawing the layout's body content over the top bar's elevation, but we now respect the draw order.

BottomSheetScaffold now also takes the sheet's state into account when placing Snackbars: in the collapsed state, Snackbars are placed above the sheet and FAB; in the expanded state, Snackbars are anchored to the sheet's bottom.

We are now also explicit about the case of sheetHeight == peekHeight, only having a collapsed anchor/state in this case. We are looking to explore different structures (i.e. a list) to represent multiple associated values for anchors.

Relnote: Fixed a bug where BottomSheetScaffold was drawing over the top app bar's shadow. BottomSheetScaffold now also takes the sheet's state into account when placing Snackbars: in the collapsed state, Snackbars are placed above the sheet and FAB; in the expanded state, Snackbars are anchored to the sheet's bottom.

Bug:

b/187771422

Test: Added a few tests around snackbar positioning and expected anchors in case sheetHeight == peekHeight, removed bottomSheetScaffold_topAppBarIsDrawnOnTopOfContent and created a screenshot test as replacement
Change-Id: Ia80b5d1ebcd4c86431eab22a985ab2b1fa824843

A compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/BottomSheetScaffoldScreenshotTest.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/BottomSheetScaffold.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/BottomSheetScaffoldTest.kt

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

jo...@google.com <jo...@google.com> May 25, 2022 09:46AM

Marked as fixed, reassigned to jo...@google.com.