[NavigationSuiteScaffold] Intended use of containerColor and contentColor unclear [331993720]

Fixed

Customer Issue

Status Update

No update yet.

Description

ia...@google.com

created issue #1

Mar 30, 2024 01:09AM

NavigationSuiteScaffold draws a Surface using

Surface(modifier = modifier, color = containerColor, contentColor = contentColor) { ... }

This covers the whole scaffold, including the content lambda. It's a bit unclear what the intended usage is. For example, contentColor will override the color of text in the content lambda but not the nav items (since they have their own colors set separately). It seems a bit odd to have the scaffold affect the content in that way, but I'm not sure if there was a specific intent behind this.

The default Compose templates uses this layout:

setContent {
    AppTheme {
        Surface() {
            // app stuff
        }
    }
}

So maybe the intent is to replace the default surface with the NavigationSuiteScaffold?

Comments

ia...@google.com <ia...@google.com> #2Apr 1, 2024 11:23PM

Project: platform/frameworks/support
Branch: androidx-main
Author: Louis Pullen-Freilich <lpf@google.com>
Link: https://android-review.googlesource.com/3328184

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling

Expand for full commit details

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling 
 
These APIs allow overscroll to have events dispatched to it by one component, and rendered in a separate component. 
 
Fixes: b/266550551 
Fixes: b/204650733 
Fixes: b/255554340 
Fixes: b/229537244 
Test: OverscrollTest 
Relnote: "Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling APIs - these APIs create a wrapped instance of the provided overscroll effect that doesn't draw / handle events respectively, which allows for rendering  overscroll in a separate component from the component that is dispatching events. For example, disabling drawing the overscroll inside a lazy list, and then drawing the overscroll separately on top / elsewhere." 
Change-Id: Idbb3d91546b49c1987a041f959bce4b2b09a9f61

Files:

M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/OverscrollDemo.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/OverscrollSample.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/OverscrollTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Overscroll.kt

Hash: f64e25b7a473c757d080521e7dd97b3f6670f60d
Date: Fri Nov 01 18:43:56 2024

le...@google.com <le...@google.com> #3Apr 2, 2024 06:06PM

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

androidx.compose.foundation:foundation:1.8.0-alpha06
androidx.compose.foundation:foundation-android:1.8.0-alpha06
androidx.compose.foundation:foundation-jvmstubs:1.8.0-alpha06
androidx.compose.foundation:foundation-linuxx64stubs:1.8.0-alpha06

ia...@google.com <ia...@google.com> #4Apr 4, 2024 11:51PM

I think it's both containerColor and contentColor that aren't fully clear, but part of that is just that the main content is technically inside of the NSS but might not be in the developer or designer's mental model (especially because of the trailing lambda syntax.

Maybe something like:

@param containerColor the color used for the background of the navigation suite scaffold, including the passed content composable. Use [Color.Transparent] to have no color

The contentColor is harder because 'content' in that context means the Material3 concept (sort of, the 'ink' that's used for UI on the Surface) but it basically just applies to the content lambda since the navigation UI components have theirs set separately. AFAICT, this isn't talked about on the mio site and M3 doesn't seem to have a page for Surface like M2 did (best I could find was "Surface – A role used for backgrounds and large, low-emphasis areas of the screen."), so I'm not sure what the best language is.

Here's what I've got so far:

@param contentColor the preferred color to be used for typography and iconography within the content lambda. This defaults to the 'on' color of the [containerColor] or to the current [LocalContentColor] if [containerColor] is not a color from the theme

le...@google.com <le...@google.com> #5Apr 10, 2024 05:22PM

Marked as fixed.

Thank you! resolved in https://android-review.git.corp.google.com/c/platform/frameworks/support/+/3034074

na...@google.com <na...@google.com> #6Apr 17, 2024 05:17PM

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

androidx.compose.material3:material3-adaptive-navigation-suite:1.0.0-alpha06
androidx.compose.material3:material3-adaptive-navigation-suite-android:1.0.0-alpha06
androidx.compose.material3:material3-adaptive-navigation-suite-desktop:1.0.0-alpha06

Issue 331993720