Status Update
Comments
si...@google.com <si...@google.com> #2
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
si...@google.com <si...@google.com>
si...@google.com <si...@google.com> #3
mo...@google.com <mo...@google.com> #4
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
si...@google.com <si...@google.com> #5
George, is this correct: You think this should not be a layout API but focus API?
The reason why I thought it is a layout API is the fact that it is a layout related View API in android independent of the focus management.
It is being used by focus on EditText, but it also is not 1-1 related to focus. For example it is possible that as a developer I'd like to show different components for a tutorial as the user clicks on next on my tutorial (I guess this was an example given on the chat)
si...@google.com <si...@google.com> #6
FYI: Happy to implement this one if you can guide me on where to start, or how to do it.
mo...@google.com <mo...@google.com> #7
OK, I have to admit that I made a mistake while reading. I saw the platform
prefix and in the top-left of that page and thought this was a View
method that we were overriding.
Stream of consciousness here:
It looks like notifyFocusedRect()
is intended to call requestRectangleOnScreen()
. This is a layout system API, not a focus system API.
I think we have some of what you need already in public API. What you're looking for is the bounding box of a Layout
, right? To get this, look at LayoutCoordinates.boundsInRoot()
.
I think the thing you're looking for is the actual requestRectangleOnScreen()
equivalent, right? I don't know if we want to expose it as an arbitrary rectangle or if we want to expose it as applying to an entire LayoutCoordinates
. My initial thought is that any LayoutCoordinates
is sufficient and an easier API to work with. I would recommend that we start with that API and see how much need there is to request an arbitrary rectangle on the screen.
I haven't looked at the implementation of requestRectangleOnScreen()
but I imagine that there is a lot of setting scroll positions. We will have to set scroll positions of scrollable
s in the hierarchy. There's currently no way for a scrollable
to interact with the layout system in a way that allows the layout system to provide feedback to scrollable
.
Matvei, do you have any suggestions here? Can the layout system poke a scrollable
to change its scroll position?
If it isn't possible (my initial thought), then we have to interact at a higher level of abstraction, using the nested scrolling interface to bring a rectangle on screen. Maybe something like this:
fun NestedScrollDispatcher.requestBringOnScreen(coordinates: LayoutCoordinates)
It looks like the nested scrolling is implemented in NestedScrollDelegatingWrapper
, so it may be possible to have:
fun LayoutCoordinates.requestBringOnScreen()
mo...@google.com <mo...@google.com> #8
Assigning to Matvei for his input.
se...@google.com <se...@google.com> #9
Bumping this ticket as it's coming up very regularly in #Compose chat.
Use cases that developers are hitting regularly.
- TextField in a LazyColumn is not visible after keyboard shows, even if scrolling would allow it to be visible
- TextField in a scrollable Column is not visible after keyboard shows, even if scrolling would allow it to be visible
se...@google.com <se...@google.com> #10
One workaround (in app code) is to save the layout position for the TextField then manually scroll a ScrollState.
ma...@google.com <ma...@google.com> #11
Coming back to this to give me 5 cents.
Modifier.scrollable
itself doesn't have any notion of layout at all, so I'm unsure if that would be the right place for it. Thing to node is that we also need to provide a proper nestedscroll support (+ account for nested scroll as well, as you want your toolbar to collapse if the textField is below the soft input, don't you?).
It seems like we would need to account for that in the scrollable implementations, e.g Modifier.verticalScroll
or LazyColumn
, as they also might want to process it differently. Some bits might live in the Modifier.scrollable
as well, but it's mostly a layout thing I suppose.
Another alternative: could it be possible to utilise semantics for this purpose? I don't know how idiomatic it would be but it seems like it is somewhat related. WE could basically ask whatever container to scroll to the bounds (which we could retrieve from the layout node) and the scroll bit will just work out of the box if the semantics are set right.
ra...@google.com <ra...@google.com>
ma...@google.com <ma...@google.com> #12
Ralston, let me know if you need any help or input from the scrolling side! Thanks!
ap...@google.com <ap...@google.com> #13
Branch: androidx-main
commit 421a738e446c6118de69765225a1508bdfd39372
Author: Ralston Da Silva <ralu@google.com>
Date: Wed May 05 06:33:28 2021
Golden images for RelocationRequester Modifier
This CL adds snapshots for the RelocationRequesterModifierTest
Bug: 178211874
Change-Id: I337d81b2176be1da4b7645b941c48c92e0d3075e
Test: ./gradlew compose:ui:ui:connectedCheck -P android.testInstrumentationRunnerArguments.class=androidx.compose.ui.layout.RelocationRequesterModifierTest
A compose/ui/ui/bringIntoParentBounds_blueBoxBottom_cuttlefish.png
A compose/ui/ui/bringIntoParentBounds_blueBoxCenterHorizontal_cuttlefish.png
A compose/ui/ui/bringIntoParentBounds_blueBoxCenterVertical_cuttlefish.png
A compose/ui/ui/bringIntoParentBounds_blueBoxLeft_cuttlefish.png
A compose/ui/ui/bringIntoParentBounds_blueBoxRight_cuttlefish.png
A compose/ui/ui/bringIntoParentBounds_blueBoxTop_cuttlefish.png
A compose/ui/ui/bringIntoParentBounds_blueBox_cuttlefish.png
A compose/ui/ui/bringIntoParentBounds_grayRectangleHorizontal_cuttlefish.png
A compose/ui/ui/bringIntoParentBounds_grayRectangleVertical_cuttlefish.png
ap...@google.com <ap...@google.com> #14
Branch: androidx-main
commit bd6db32649c6060ddead0d1f7e55636e60c9ff61
Author: Ralston Da Silva <ralu@google.com>
Date: Mon Apr 19 17:54:18 2021
Relocation Modifier
This is a modifier that can be used to bring an item into view by requesting that
all its scrollable parents scroll so that this item is visible.
Bug: 178211874
Test: ./gradlew compose:ui:ui:connectedCheck -P android.testInstrumentationRunnerArguments.class=androidx.compose.ui.layout.RelocationRequesterModifierTest
Relnote: Added Experimental Modifier.relocate()
Change-Id: I48322e5f84f5d688f92c7a397e9b1328b3824f63
M compose/ui/ui/api/1.0.0-beta08.txt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/public_plus_experimental_1.0.0-beta08.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/api/restricted_1.0.0-beta08.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/UiDemos.kt
A compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/scroll/BringIntoParentBoundsDemo.kt
A compose/ui/ui/samples/src/main/java/androidx/compose/ui/samples/RelocationSamples.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/RelocationRequesterModifierTest.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/RelocationRequester.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/RelocationRequesterModifier.kt
ap...@google.com <ap...@google.com> #15
Branch: androidx-main
commit 42303ba13af7818755606f2a3ebf931914b2b50e
Author: Ralston Da Silva <ralu@google.com>
Date: Thu May 20 15:12:54 2021
Bring item into View
This CL renames RelocationRequester.bringIntoParentBounds() to RelocationRequester.bringIntoView.
It also supports the use-case where they keyobard could be hiding the element, and brings
it into view by calling view.requestRectangleOnScreen.
Bug: 178211874
Test: ./gradlew compose:ui:ui:connectedCheck -P android.testInstrumentationRunnerArguments.class=androidx.compose.ui.layout.RelocationRequesterModifierTest
Relnote: N/A
Change-Id: I4124888de3b67fda965372380b5a166406817ce1
M compose/ui/ui/api/public_plus_experimental_1.0.0-beta08.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/UiDemos.kt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/scroll/BringIntoViewDemo.kt
A compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/scroll/RequestRectangleOnScreeenDemo.kt
M compose/ui/ui/samples/src/main/java/androidx/compose/ui/samples/RelocationSamples.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/pointer/PointerInputEventProcessorTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/RelocationRequesterModifierTest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/RelocationRequester.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/RelocationRequesterModifier.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/Owner.kt
M compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/DesktopOwner.desktop.kt
M compose/ui/ui/src/test/kotlin/androidx/compose/ui/node/LayoutNodeTest.kt
ni...@google.com <ni...@google.com> #16
Bulk edit: This bug is currently marked as blocking Compose 1.0. Once rc01 is cut, all changes will need to go through our cherry-pick process.
If you intend to fix it in Compose 1.0, no action is needed except prioritizing finishing this work.
Otherwise, please remove this bug from the appropriate 1.0 hotlist (likely
Thanks!
ra...@google.com <ra...@google.com> #17
For this fix we need TextField to use RelocationRequester.bringIntoView() which is an experimental UI API, and it so it can't be fixed for Compose 1.0
ap...@google.com <ap...@google.com> #18
Branch: androidx-main
commit e40d3772b9a36196f94dfaef0483e3fc52951b53
Author: Ralston Da Silva <ralu@google.com>
Date: Fri Jun 04 18:02:01 2021
Implement bringIntoView
We initially tried to implement bringIntoView by making changes
to nested scroll. This CL adds a Modifier.onRelocationRequest
that responds to requests from Modifier.relocationRequester
more details at go/compose-bringIntoView
Bug: 178211874
Test: ./gradlew compose:ui:ui:connectedCheck -P android.testInstrumentationRunnerArguments.class=androidx.compose.ui.layout.RelocationModifierTest
Relnote: Added experimental modifier to handle relocation requests.
Change-Id: I65a97bd7fca7271781efe31fcc9cb387e9857b51
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/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/UiDemos.kt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/scroll/BringIntoViewDemo.kt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/scroll/RequestRectangleOnScreeenDemo.kt
M compose/ui/ui/samples/src/main/java/androidx/compose/ui/samples/RelocationSamples.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/pointer/PointerInputEventProcessorTest.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/RelocationModifierTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/RelocationRequesterModifierTest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/RelocationModifier.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/RelocationRequester.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/RelocationRequesterModifier.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNode.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeWrapper.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/ModifiedRelocationNode.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/ModifiedRelocationRequesterNode.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/Owner.kt
M compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/DesktopOwner.desktop.kt
M compose/ui/ui/src/test/kotlin/androidx/compose/ui/node/LayoutNodeTest.kt
si...@google.com <si...@google.com>
so...@google.com <so...@google.com> #19
ap...@google.com <ap...@google.com> #20
Branch: androidx-main
commit a5c92f31181087df9a6a7100111d4248b94e75d4
Author: Zach Klippenstein <klippenstein@google.com>
Date: Fri Jan 28 12:36:11 2022
Change CoreTextField to use BringIntoViewRequester.
CoreTextField needs to ensure the cursor is in the visible bounds of all
scrollable parents, including the one in the text field itself.
Originally, it tried doing this by going through some of the text
service abstractions and ultimately calling
`View.requestRectangleOnScreen`. However, composable parents also need
to be aware of the request, so the correct way to do this is to use the
relatively recent `BringIntoViewRequester`. I'm not sure why all the
indirection through text services was done, it isn't necessary and
obscures the code, so I just made `CoreTextField` directly call
`BringIntoViewRequester.bringIntoView()`. This helps with
but doesn't completely fix it, due to
Bug:
Fixes:
Test: Tested manually.
Test: Refactored TextFieldDelegate unit tests to use the new code path:
./gradlew :compose:f:f:test
Relnote: "`notifyFocusedRect` methods in `TextInputSession` and
`TextInputService` are now deprecated and won't be called. Use
`BringIntoViewRequester` instead."
Change-Id: Ia4302c5f6ee79eec30a9f42c149da8775e1ed57e
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Focusable.kt
M compose/ui/ui-text/api/restricted_current.txt
M compose/foundation/foundation/benchmark/src/androidTest/java/androidx/compose/foundation/benchmark/text/TextFieldToggleTextTestCase.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/text/input/TextInputServiceAndroid.android.kt
A compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/text/TextFieldBringIntoViewTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/TextFieldDelegate.kt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/input/TextInputService.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/CoreTextField.kt
M compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/text/TextFieldDelegateTest.kt
M compose/ui/ui-text/api/public_plus_experimental_current.txt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/TextInputServiceTest.kt
M compose/ui/ui-text/api/current.txt
ap...@google.com <ap...@google.com> #21
Branch: androidx-main
commit b47ac54b27a3e77b40f672cbcd5a713f464a4d27
Author: Zach Klippenstein <klippenstein@google.com>
Date: Thu Feb 03 12:09:04 2022
Revert removing implementations of notifyFocusedRect.
The notifyFocusedRect methods on TextInputService and TextInputSession
were deprecated and had their implementations replaced by no-ops in the
change aosp/1959647. This was probably too aggressive, so this change
adds the old implementations back in but leaves them deprecated, in case
any third-party code was calling them. The implementations were actually
broken before, and this change doesn't try to fix them, it just restores
the previous broken behavior.
Discussion thread about revert:
Bug:
Bug:
Test: Old tests restored.
Change-Id: I385749b629fd35b72bd148122ab1ec0f6d4ffa96
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/input/TextInputService.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/text/input/TextInputServiceAndroid.android.kt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/TextInputServiceTest.kt
da...@gmail.com <da...@gmail.com> #22
Is there something that has to be done from the developer perspective for the textfield to scroll into view when the keyboard comes up to cover them?
Description
What the function does is:
- calls requestRectangle on View.
Therefore it is something that we can bubble up from current node to parents and then the View system until the provided area is visible on screen.
Not sure how compose scrollers would react to this and if it is possible.
Related tickets