From 04284104ef905a93bf58024519c1d66d1ac9fd3d Mon Sep 17 00:00:00 2001 From: Jake Wharton Date: Fri, 13 Oct 2023 15:37:42 -0400 Subject: [PATCH] Track widget node index within its parent This will eventually be used to recreate the view within its parent should a modifier interceptor change the returned instance. --- redwood-compose/build.gradle | 2 + .../app/cash/redwood/compose/WidgetApplier.kt | 71 ++++++++-- .../redwood/compose/ChildrenNodeIndexTest.kt | 132 ++++++++++++++++++ 3 files changed, 192 insertions(+), 13 deletions(-) create mode 100644 redwood-compose/src/commonTest/kotlin/app/cash/redwood/compose/ChildrenNodeIndexTest.kt diff --git a/redwood-compose/build.gradle b/redwood-compose/build.gradle index b8c814f9da..cfb4ff2701 100644 --- a/redwood-compose/build.gradle +++ b/redwood-compose/build.gradle @@ -1,3 +1,4 @@ +import app.cash.redwood.buildsupport.ComposeHelpers import app.cash.redwood.buildsupport.KmpTargets apply plugin: 'org.jetbrains.kotlin.multiplatform' @@ -13,6 +14,7 @@ kotlin { sourceSets { commonMain { + kotlin.srcDir(ComposeHelpers.get(tasks, 'app.cash.redwood.compose')) dependencies { api libs.kotlinx.coroutines.core api projects.redwoodRuntime diff --git a/redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/WidgetApplier.kt b/redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/WidgetApplier.kt index 17db89fb0a..57626b83f5 100644 --- a/redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/WidgetApplier.kt +++ b/redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/WidgetApplier.kt @@ -21,6 +21,8 @@ import app.cash.redwood.Modifier import app.cash.redwood.RedwoodCodegenApi import app.cash.redwood.widget.ChangeListener import app.cash.redwood.widget.Widget +import kotlin.math.max +import kotlin.math.min /** * An [Applier] for Redwood's tree of nodes. @@ -45,11 +47,7 @@ import app.cash.redwood.widget.Widget * ``` * The node tree produced by this applier is not actually a tree. We do not maintain a relationship * from each [WidgetNode] to their [ChildrenNode]s as they can never be individually moved/removed. - * Similarly, no relationship is maintained from a [ChildrenNode] to their [WidgetNode]s. Instead, - * the [WidgetNode.widget] is what's added to the parent [ChildrenNode.children]. - * - * Compose maintains the tree structure internally. All non-insert operations are performed - * using indexes and counts rather than references which are forwarded to [ChildrenNode.children]. + * Compose maintains that relationship internally to support positioning the applier. */ @OptIn(RedwoodCodegenApi::class) internal class NodeApplier( @@ -97,12 +95,8 @@ internal class NodeApplier( if (instance is WidgetNode<*, *>) { val widgetNode = instance as WidgetNode, W> val current = current as ChildrenNode - val children = current.children - - widgetNode.container = children - children.insert(index, widgetNode.widget) - current.parent?.let(::recordChanged) + current.insert(index, widgetNode) } } @@ -110,7 +104,7 @@ internal class NodeApplier( check(!closed) val current = current as ChildrenNode - current.children.remove(index, count) + current.remove(index, count) current.parent?.let(::recordChanged) } @@ -118,7 +112,7 @@ internal class NodeApplier( check(!closed) val current = current as ChildrenNode - current.children.move(from, to, count) + current.move(from, to, count) current.parent?.let(::recordChanged) } @@ -149,6 +143,9 @@ public class WidgetNode, V : Any>( public var container: Widget.Children? = null + /** The index of [widget] within its parent [container] when attached. */ + public var index: Int = -1 + public companion object { public val SetModifiers: WidgetNode<*, *>.(Modifier) -> Unit = { recordChanged() @@ -186,6 +183,54 @@ internal class ChildrenNode private constructor( constructor(accessor: (Widget) -> Widget.Children) : this(accessor, null, null) constructor(children: Widget.Children) : this(null, null, children) + private val nodes = mutableListOf, W>>() + + fun insert(index: Int, node: WidgetNode, W>) { + nodes.let { nodes -> + // Bump the index of any nodes which will be shifted. + for (i in index until nodes.size) { + nodes[i].index++ + } + + node.index = index + nodes.add(index, node) + } + + children.let { children -> + node.container = children + children.insert(index, node.widget) + } + } + + fun remove(index: Int, count: Int) { + nodes.let { nodes -> + nodes.remove(index, count) + + // Drop the index of any nodes shifted after the removal. + for (i in index until nodes.size) { + nodes[i].index -= count + } + } + + children.remove(index, count) + } + + fun move(from: Int, to: Int, count: Int) { + nodes.let { nodes -> + nodes.move(from, to, count) + + // If moving up, lower bound is from. If moving down, lower bound is to. + val lowerBound = min(from, to) + // If moving up, upper bound is to, If moving down, upper bound is from + count. + val upperBound = max(to, from + count) + for (i in lowerBound until upperBound) { + nodes[i].index = i + } + } + + children.move(from, to, count) + } + /** The parent of this children group. Null when the root children instance. */ var parent: Widget? = parent get() { @@ -194,7 +239,7 @@ internal class ChildrenNode private constructor( } private set - val children: Widget.Children get() = checkNotNull(_children) { "Not attached" } + private val children: Widget.Children get() = checkNotNull(_children) { "Not attached" } fun attachTo(parent: Widget) { _children = checkNotNull(accessor).invoke(parent) diff --git a/redwood-compose/src/commonTest/kotlin/app/cash/redwood/compose/ChildrenNodeIndexTest.kt b/redwood-compose/src/commonTest/kotlin/app/cash/redwood/compose/ChildrenNodeIndexTest.kt new file mode 100644 index 0000000000..89f3588a92 --- /dev/null +++ b/redwood-compose/src/commonTest/kotlin/app/cash/redwood/compose/ChildrenNodeIndexTest.kt @@ -0,0 +1,132 @@ +/* + * Copyright (C) 2023 Square, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package app.cash.redwood.compose + +import app.cash.redwood.Modifier +import app.cash.redwood.RedwoodCodegenApi +import app.cash.redwood.widget.MutableListChildren +import app.cash.redwood.widget.Widget +import assertk.assertThat +import assertk.assertions.isEqualTo +import kotlin.test.Test + +/** + * This class tests an implementation detail of [ChildrenNode] which maintains the correct + * index value on its child [WidgetNode]s. While it could conceivably be tested through public + * API it's far easier to validate the behavior this way. + */ +@OptIn(RedwoodCodegenApi::class) +class ChildrenNodeIndexTest { + private val root = ChildrenNode(MutableListChildren()) + + @Test fun insert() { + val a = WidgetNode(NoOpRedwoodApplier, StringWidget("a")) + assertThat(a.index).isEqualTo(-1) + root.insert(0, a) + assertThat(a.index).isEqualTo(0) + + val b = WidgetNode(NoOpRedwoodApplier, StringWidget("a")) + root.insert(1, b) + assertThat(a.index).isEqualTo(0) + assertThat(b.index).isEqualTo(1) + + val c = WidgetNode(NoOpRedwoodApplier, StringWidget("a")) + root.insert(0, c) + assertThat(c.index).isEqualTo(0) + assertThat(a.index).isEqualTo(1) + assertThat(b.index).isEqualTo(2) + } + + @Test fun remove() { + val a = WidgetNode(NoOpRedwoodApplier, StringWidget("a")) + val b = WidgetNode(NoOpRedwoodApplier, StringWidget("b")) + val c = WidgetNode(NoOpRedwoodApplier, StringWidget("c")) + val d = WidgetNode(NoOpRedwoodApplier, StringWidget("d")) + val e = WidgetNode(NoOpRedwoodApplier, StringWidget("e")) + root.insert(0, a) + root.insert(1, b) + root.insert(2, c) + root.insert(3, d) + root.insert(4, e) + assertThat(a.index).isEqualTo(0) + assertThat(b.index).isEqualTo(1) + assertThat(c.index).isEqualTo(2) + assertThat(d.index).isEqualTo(3) + assertThat(e.index).isEqualTo(4) + + root.remove(2, 1) // c + assertThat(a.index).isEqualTo(0) + assertThat(b.index).isEqualTo(1) + assertThat(d.index).isEqualTo(2) + assertThat(e.index).isEqualTo(3) + + root.remove(1, 2) // b, d + assertThat(a.index).isEqualTo(0) + assertThat(e.index).isEqualTo(1) + + root.remove(1, 1) // e + assertThat(a.index).isEqualTo(0) + } + + @Test fun move() { + val a = WidgetNode(NoOpRedwoodApplier, StringWidget("a")) + val b = WidgetNode(NoOpRedwoodApplier, StringWidget("b")) + val c = WidgetNode(NoOpRedwoodApplier, StringWidget("c")) + val d = WidgetNode(NoOpRedwoodApplier, StringWidget("d")) + val e = WidgetNode(NoOpRedwoodApplier, StringWidget("e")) + root.insert(0, a) + root.insert(1, b) + root.insert(2, c) + root.insert(3, d) + root.insert(4, e) + assertThat(a.index).isEqualTo(0) + assertThat(b.index).isEqualTo(1) + assertThat(c.index).isEqualTo(2) + assertThat(d.index).isEqualTo(3) + assertThat(e.index).isEqualTo(4) + + root.move(0, 5, 1) // a 0 --> 4 + assertThat(b.index).isEqualTo(0) + assertThat(c.index).isEqualTo(1) + assertThat(d.index).isEqualTo(2) + assertThat(e.index).isEqualTo(3) + assertThat(a.index).isEqualTo(4) + + root.move(1, 4, 2) // c,d 1 --> 2 + assertThat(b.index).isEqualTo(0) + assertThat(e.index).isEqualTo(1) + assertThat(c.index).isEqualTo(2) + assertThat(d.index).isEqualTo(3) + assertThat(a.index).isEqualTo(4) + + root.move(2, 1, 3) // c,d,a 2 --> 1 + assertThat(b.index).isEqualTo(0) + assertThat(c.index).isEqualTo(1) + assertThat(d.index).isEqualTo(2) + assertThat(a.index).isEqualTo(3) + assertThat(e.index).isEqualTo(4) + } +} + +private class StringWidget(override val value: String) : Widget { + override var modifier: Modifier = Modifier +} + +@OptIn(RedwoodCodegenApi::class) +private object NoOpRedwoodApplier : RedwoodApplier { + override val provider get() = throw UnsupportedOperationException() + override fun recordChanged(widget: Widget) = Unit +}