API for changing selection handle color [139320907]

Fixed

Feature Request

Status Update

No update yet.

Description

si...@google.com

created issue #1

Aug 13, 2019 01:42AM

No description yet.

Comments

si...@google.com <si...@google.com> Oct 26, 2019 04:33PM

Reassigned to si...@google.com.

si...@google.com <si...@google.com> Oct 26, 2019 04:41PM

Accepted by si...@google.com.

si...@google.com <si...@google.com> #2Oct 27, 2019 12:27AM

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

Message last modified on Nov 6, 2020 05:37AM

si...@google.com <si...@google.com> Nov 22, 2019 07:20AM

Assigned to si...@google.com.

si...@google.com <si...@google.com> Dec 3, 2019 06:33PM

Status: New

si...@google.com <si...@google.com> Jan 15, 2020 12:48AM

Assigned to an...@google.com.

si...@google.com <si...@google.com> #3Feb 4, 2020 02:41AM

bulk comment, fyi: we have added icebox to all the tickets because it was suggested by blintz in order to prevent auto-close tickets. we want to keep our Compose tickets to remain opened.

see

b/123001228#comment6

si...@google.com <si...@google.com> Apr 18, 2020 12:24AM

Reassigned to qq...@google.com.

si...@google.com <si...@google.com> #4May 4, 2020 10:12PM

(bulk comment) Hi,

Can you please check if this task can be finished before June, and if not remove `Jetpack Compose Major Release (May 2020)` hotlist?

(apologies if this is a double comment)

Thank you

si...@google.com <si...@google.com> #5May 4, 2020 10:26PM

(bulk comment)
Please ignore last comment related to the June release. It was caused by a mistake in the query I used. Thank you.

si...@google.com <si...@google.com> #6Sep 14, 2020 11:22PM

further postponing to category 3, since it doesn't look like we can accomplish this one. If time allows one of us will pick it up during category 2.

al...@gmail.com <al...@gmail.com> #7Nov 6, 2020 04:39PM

out of curiousity… if this isnt immediately planned, have you considered changing he default colors at all? a couple things i noticed:

(1) i could see the choice of purple + blue clashing with a number of existing apps, and maybe there are some better defaults that could be used to at least make the colors stand out less.

(2) the defaults don’t support dark mode very well. in our app, the dark purple handles on top of a primary dark mode background has only a 2.14:1 color contrast ratio (whereas in light mode the contrast is 8.98:1)

lp...@google.com <lp...@google.com> #8Nov 6, 2020 04:49PM

I agree with Comment #7, I think this is an important issue we should resolve (P2 at least).

Siyamed, if you / your team does not have time to look at this, feel free to assign to me - happy to take this over. As well as allowing changing the color, MaterialTheme should also be configured to set a color from the theme by default so that it is correct out of the box for typicaly use cases.

si...@google.com <si...@google.com> #9Nov 6, 2020 04:56PM

Reassigned to lp...@google.com.

related: b/139320372

si...@google.com <si...@google.com> #10Nov 6, 2020 05:01PM

Thanks Louis.

Please keep in mind there are two main entry points: Text selection and TextField selection. as a note: In TextField it is possible that existing cursorColor defines those colors (or one of them)

si...@google.com <si...@google.com> #11Nov 6, 2020 05:05PM

it would be best to control selection as a whole for both of them I believe, but let's discuss it. Looks like material.TextField uses errorColor and activeColor as the cursor color. let's make sure we take that into account while designing the theming (they seem to be related)

si...@google.com <si...@google.com> #12Nov 6, 2020 05:14PM

open questions, assumptions that need verification:

is cursorColor and selection color the same
cursorColor is errorColor and activeColor in material.TextField, is there any relation
Text and TextField should use a single selection color for consistency
selection color is always the same for selection handle and selection background
cursor color determines insertion (cursor) handle color as well (we do not have cursor handle yet)
what the default color should be for light and dark mode
shape of the cursor handle is not urgent, and possibly something we specifically don't want customization for the phone end user experience (similar to cursor animation)
can there be a default color in foundation? choosing the text color, bg color etc too tricky. Using the exact text color wouldn't work since it would make text invisible.

Message last modified on Nov 6, 2020 05:20PM

ap...@google.com <ap...@google.com> #13Nov 20, 2020 11:49PM

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

commit 93195d42d1f079136053e33ccec6e7d5ac839dfe
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Tue Nov 17 15:20:25 2020

Adds AmbientTextSelectionColors to allow customizing the colors used for text selection

Also sets the correct Material defaults in MaterialTheme

Bug:

b/139320372

Bug:

b/139320907

Test: MaterialTextSelectionColorsScreenshotTest
Test: TextSelectionColorsScreenshotTest
Test: TextSelectionBackgroundColorTest
Relnote: "MaterialTheme now sets the correct colors for selection handles and selection background. Non-Material apps can manually use AmbientTextSelectionColors to customize the colors used for selection."
Change-Id: I1e6f4b495bdc713e162759a08ecf0a7311b26e33

M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text/TextFieldDelegateIntegrationTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/CoreText.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/CoreTextField.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/TextFieldDelegate.kt
M compose/material/material/api/current.txt
M compose/material/material/api/public_plus_experimental_current.txt
M compose/material/material/api/restricted_current.txt
A compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/MaterialTextSelectionColorsScreenshotTest.kt
A compose/material/material/src/commonMain/kotlin/androidx/compose/material/MaterialTextSelectionColors.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/MaterialTheme.kt
A compose/material/material/src/test/kotlin/androidx/compose/material/TextSelectionBackgroundColorTest.kt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/selection/SelectionHandlesTest.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/selection/TextSelectionColorsScreenshotTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/selection/SelectionHandles.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/selection/TextSelectionColors.kt

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

lp...@google.com <lp...@google.com> Nov 20, 2020 11:58PM

Marked as fixed.