Border API feedback [158160576]

Fixed

Bug

Status Update

No update yet.

Description

ma...@google.com

created issue #1

Jun 4, 2020 10:16AM

From compose API council:

Border discussion notes

Remove Border data class used by Box?
- Duplicates functionality found elsewhere
Rename Modifier.outline to Modifier.border *This modifier should affect layout and pad in the layout by the size of the border
How do I know how to write an API when these are the potential moving parts:
- Border class
- Shape
- Parameters
- Modifier
Decision!
- Rename Border data class to Stroke
- Remove Modifier.drawBorder modifiers, replace with Modifier.border() * border modifier should also incorporate layout and clip+shape modification
- border parameter on Box becomes "border: Stroke? = null"
- Stroke should have a Color-based convenience factory/constructor *Nice effects: * OutlinedButton now has a shape and a stroke; not conflating the "border" conceptual space * "border" is now a unified concept of shape+stroke parameters+padding+clip behavior * "border" never appears as a parameter; encourage that usage as a modifier * "outline" is part of the OutlinedButton's content

Comments

ja...@google.com <ja...@google.com> #2Jul 31, 2020 12:08PM

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> #3Jul 31, 2020 12:18PM

Hey. I have a commit already for this, that is almost ready (not a CL yet). I hope I can lend it before alpha, but not all of the stuff described above will happen:

All renaming will be done
There will be NO padding build-in. This is in general not what you want and most of the design sizes come from the edge of the component, not from the border edge. Also it's hard to opt-out of it.
there will be no clipping. Clipping makes little to no sense without padding there and also I asked few people about it and it's not what they would expect.

ap...@google.com <ap...@google.com> #4Aug 11, 2020 05:38PM

Marked as fixed.

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

commit d21bfdad68f4c6e66171c3d93e2fdeb356a0cdc5
Author: Matvei Malkov <malkov@google.com>
Date: Mon Aug 10 16:03:41 2020

Modifier.drawBorder -> Modifier.border

This CL renamed drawBorder to border per API council request. Also fixes the problem where border is drawn incorrectly when the shape is a rounded corner rect.

Change-Id: I4257d62b222e27c9ad67e1b2581b162cc9392c9e
Relnote: "Modifier.drawBorder has been deprecated. Use Modifier.border instead. Border data class has been replaced by BorderStroke"
Fixes: 158160576
Test: fixed appropriatelly. Will add screenshot tests separately

M ui/integration-tests/src/main/java/androidx/ui/integration/test/core/SimpleRadioButton1TestCase.kt
M ui/integration-tests/src/main/java/androidx/ui/integration/test/core/SimpleRadioButton2TestCase.kt
M ui/integration-tests/src/main/java/androidx/ui/integration/test/core/SimpleRadioButton4TestCase.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/DoubleTapGestureDetectorDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/DoubleTapInTapDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/HorizontalScrollersInVerticalScrollerDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/LongPressGestureDetectorDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/NestedLongPressDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/NestedPressDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/NestedScrollingDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/PointerInputDuringSubCompDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/PressIndicatorGestureDetectorDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/TapGestureDetectorDemo.kt
M ui/ui-core/integration-tests/ui-core-demos/src/main/java/androidx/compose/ui/demos/gestures/VerticalScrollerInDrawerLayoutDemo.kt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/BoxDemo.kt
M ui/ui-foundation/samples/src/main/java/androidx/compose/foundation/samples/BorderSamples.kt
M ui/ui-foundation/samples/src/main/java/androidx/compose/foundation/samples/BoxSamples.kt
M ui/ui-foundation/samples/src/main/java/androidx/compose/foundation/samples/InteractionStateSample.kt
M ui/ui-foundation/samples/src/main/java/androidx/compose/foundation/samples/ZoomableSample.kt
M ui/ui-foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/BorderTest.kt
M ui/ui-foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/BoxTest.kt
M ui/ui-foundation/src/commonMain/kotlin/androidx/compose/foundation/Border.kt
A ui/ui-foundation/src/commonMain/kotlin/androidx/compose/foundation/BorderStroke.kt
M ui/ui-foundation/src/commonMain/kotlin/androidx/compose/foundation/Box.kt
D ui/ui-foundation/src/commonMain/kotlin/androidx/compose/foundation/DrawBorder.kt
M ui/ui-material/api/current.txt
M ui/ui-material/api/public_plus_experimental_current.txt
M ui/ui-material/api/restricted_current.txt
M ui/ui-material/integration-tests/material-demos/src/main/java/androidx/compose/material/demos/ButtonDemo.kt
M ui/ui-material/integration-tests/material-demos/src/main/java/androidx/compose/material/demos/ColorPickerDemo.kt
M ui/ui-material/integration-tests/material-demos/src/main/java/androidx/compose/material/demos/ElevationDemo.kt
M ui/ui-material/integration-tests/material-demos/src/main/java/androidx/compose/material/demos/ProgressIndicatorDemo.kt
M ui/ui-material/samples/src/main/java/androidx/compose/material/samples/ScaffoldSamples.kt
M ui/ui-material/samples/src/main/java/androidx/compose/material/samples/TabSamples.kt
M ui/ui-material/src/androidAndroidTest/kotlin/androidx/compose/material/AlertDialogTest.kt
M ui/ui-material/src/commonMain/kotlin/androidx/compose/material/Button.kt
M ui/ui-material/src/commonMain/kotlin/androidx/compose/material/Card.kt
M ui/ui-material/src/commonMain/kotlin/androidx/compose/material/Surface.kt
M ui/ui-tooling/src/androidTest/java/androidx/ui/tooling/inspector/ParameterFactoryTest.kt
M ui/ui-tooling/src/main/java/androidx/ui/tooling/inspector/ParameterFactory.kt

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

Issue 158160576

Description

Issue summary

Comments

ja...@google.com <ja...@google.com> #2Jul 31, 2020 12:08PM

ma...@google.com <ma...@google.com> #3Jul 31, 2020 12:18PM

ap...@google.com <ap...@google.com> #4Aug 11, 2020 05:38PM

Add comment

Issue metadata