Remove TextFieldColorsWithIcons interface and move its methods to base interface. [199377790]

Assigned

Bug

Status Update

No update yet.

Description

os...@google.com

created issue #1

Sep 9, 2021 12:01PM

Currently in aosp/1810198 suggests interface with new methods instead of adding this methods to TextFieldColors interface (because of b/198571248). Move these methods to TextFieldColors and deprecate

fun leadingIconColor(enabled: Boolean, isError: Boolean): State<Color> 

fun trailingIconColor(enabled: Boolean, isError: Boolean): State<Color>

when it'll be possible

Comments

so...@google.com <so...@google.com> #2Jan 25, 2022 04:22PM

Relevant APIs: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/ProgressIndicator.kt;l=318

so...@google.com <so...@google.com> #3Feb 10, 2022 07:01PM

Reassigned to ma...@google.com.

After some brief poking around:

Relevant code in LayoutNode: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNode.kt;l=421?q=semantics%20f:layoutnode&ss=androidx%2Fplatform%2Fframeworks%2Fsupport:compose%2F

Ends up here: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeViewAccessibilityDelegateCompat.android.kt;l=2152?q=onSemanticsChange%20lang:kt&ss=androidx%2Fplatform%2Fframeworks%2Fsupport:compose%2F

If accessibility is off, looks like changing progress semantics a no op which would be a big improvement to performance.

Otherwise semantics does a remeasure / relayout, and walks the full semantics tree. This would likely have no immediate benefit if accessibility is turned on.

ma...@google.com <ma...@google.com> Mar 3, 2022 07:40PM

Accepted by ma...@google.com.

ap...@google.com <ap...@google.com> #4Mar 8, 2022 12:59PM

Would this suggestion also avoid recomposing, which seems like it would still be a problem even if it's a no-op in the guts of semantic node code.

co...@google.com <co...@google.com> #5Jun 6, 2022 03:39PM

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

Indeed we could change the progressSemantics API to be lambda-based like ScrollAxisRange, but I'm reluctant to add to our API for this. A number of properties are likely getting updated at high frequency by some application or other. Each one will add API clutter and add to the number of decisions developers have to make to optimize performance.

The semantics block is already a lambda, so we have been talking about being able to make it run separately from composition when one of its inputs changes, much like layout. Ralston is already starting to work on a performance-oriented rearchitecture to semantics and we can discuss how this might fit into that plan.

co...@google.com <co...@google.com> #6Jun 7, 2022 01:18PM

Status: Assigned (reopened)

The semantics block is already executed outside of composition. That change was made and landed in 1.5.

We honestly don't have to create a new API for this. Modifier.progressSemantics is just a convenience API around the already public progressBarRangeInfo API. Material3 could easily just use a plain Modifier.semantics call with the value resolved inside the lambda and this semantics property set and it would avoid composition and fix this issue.

ap...@google.com <ap...@google.com> #7Jun 21, 2023 06:28PM

+1 to Leland's approach. That is aligned with my thinking here.