[go: nahoru, domu]

Deprecate @Model

Relnote: “
@Model annotation is now deprecated. Use state and mutableStateOf as alternatives. This deprecation decision was reached after much careful discussion.

Justification
=============

Rationale includes but is not limited to:
- Reduces API surface area and concepts we need to teach
- More closely aligns with other comparable toolkits (Swift UI, React, Flutter)
- Reversible decision. We can always bring @Model back later.
- Removes corner-case usage and difficult to answer questions about configuring @Model as things we need to handle
  - @Model data classes, equals, hashcode, etc.
  - How do I have some properties “observed” and others not?
  - How do I specify structural vs. referential equality to be used in observation?
- Reduces “magic” in the system. Would reduce the likelihood of someone assuming system was smarter than it is (ie, it knowing how to diff a list)
- Makes the granularity of observation more intuitive.
- Improves refactorability from variable -> property on class
- Potentially opens up possibilities to do hand-crafted State-specific optimizations
- More closely aligns with the rest of the ecosystem and reduces ambiguity towards immutable or us “embracing mutable state”


Migration Notes
===============

Almost all existing usages of @Model are fairly trivially transformed in one of two ways. The example below has a @Model class with two properties just for the sake of example, and has it being used in a composable.

```
@Model class Position(
 var x: Int,
 var y: Int
)

@Composable fun Example() {
 var p = remember { Position(0, 0) }
 PositionChanger(
   position=p,
    p.x = it }
    p.y = it }
 )
}
```

Alternative 1: Use State<OriginalClass> and create copies.
----------------------------------------------------------

This approach is made easier with Kotlin’s data classes. Essentially, make all previously `var` properties into `val` properties of a data class, and then use `state` instead of `remember`, and assign the state value to cloned copies of the original using the data class `copy(...)` convenience method.

It’s important to note that this approach only works when the only mutations to that class were done in the same scope that the `State` instance is created. If the class is internally mutating itself outside of the scope of usage, and you are relying on the observation of that, then the next approach is the one you will want to use.

```
data class Position(
 val x: Int,
 val y: Int
)

@Composable fun Example() {
 var p by state { Position(0, 0) }
 PositionChanger(
   position=p,
    p = p.copy(x=it) }
    p = p.copy(y=it) }
 )
}
```

Alternative 2: Use mutableStateOf and property delegates
--------------------------------------------------------

This approach is made easier with Kotlin’s property delegates and the `mutableStateOf` API which allows you to create MutableState instances outside of composition. Essentially, replace all `var` properties of the original class with `var` properties with `mutableStateOf` as their property delegate. This has the advantage that the usage of the class will not change at all, only the internal implementation of it. The behavior is not completely identical to the original example though, as each property is now observed/subscribed to individually, so the recompositions you see after this refactor could be more narrow (a good thing).

```
class Position(x: Int, y: Int) {
 var x by mutableStateOf(x)
 var y by mutableStateOf(y)
}

// source of Example is identical to original
@Composable fun Example() {
 var p = remember { Position(0, 0) }
 PositionChanger(
   position=p,
    p.x = it }
    p.y = it }
 )
}
```
“

Bug: 156546430
Bug: 152993135
Bug: 152050010
Bug: 148866188
Bug: 148422703
Bug: 148394427
Bug: 146362815
Bug: 146342522
Bug: 143413369
Bug: 135715219
Bug: 126418732
Bug: 147088098
Bug: 143263925
Bug: 139653744

Change-Id: I409e8c158841eae1dd548b33f1ec80bb609cba31
diff --git a/ui/ui-material/src/androidTest/java/androidx/ui/material/DrawerTest.kt b/ui/ui-material/src/androidTest/java/androidx/ui/material/DrawerTest.kt
index 0219b37..613d4e2 100644
--- a/ui/ui-material/src/androidTest/java/androidx/ui/material/DrawerTest.kt
+++ b/ui/ui-material/src/androidTest/java/androidx/ui/material/DrawerTest.kt
@@ -17,8 +17,8 @@
 package androidx.ui.material
 
 import android.os.SystemClock.sleep
-import androidx.compose.Model
 import androidx.compose.emptyContent
+import androidx.compose.mutableStateOf
 import androidx.test.filters.MediumTest
 import androidx.ui.core.LayoutCoordinates
 import androidx.ui.core.Modifier
@@ -53,9 +53,6 @@
 import java.util.concurrent.TimeUnit
 import kotlin.math.roundToInt
 
-@Model
-data class DrawerStateHolder(var state: DrawerState)
-
 @MediumTest
 @RunWith(JUnit4::class)
 class DrawerTest {
@@ -154,11 +151,11 @@
         var contentWidth: IntPx? = null
         var openedLatch: CountDownLatch? = null
         var closedLatch: CountDownLatch? = CountDownLatch(1)
-        val drawerState = DrawerStateHolder(DrawerState.Closed)
+        val drawerState = mutableStateOf(DrawerState.Closed)
         composeTestRule.setMaterialContent {
             TestTag("Drawer") {
                 Semantics(container = true) {
-                    ModalDrawerLayout(drawerState.state, { drawerState.state = it },
+                    ModalDrawerLayout(drawerState.value, { drawerState.value = it },
                         drawerContent = {
                             Box(
                                 Modifier.fillMaxSize().onPositioned { info: LayoutCoordinates ->
@@ -186,7 +183,7 @@
         // When the drawer state is set to Opened
         openedLatch = CountDownLatch(1)
         runOnIdleCompose {
-            drawerState.state = DrawerState.Opened
+            drawerState.value = DrawerState.Opened
         }
         // Then the drawer should be opened
         assertThat(openedLatch.await(5, TimeUnit.SECONDS)).isTrue()
@@ -194,7 +191,7 @@
         // When the drawer state is set to Closed
         closedLatch = CountDownLatch(1)
         runOnIdleCompose {
-            drawerState.state = DrawerState.Closed
+            drawerState.value = DrawerState.Closed
         }
         // Then the drawer should be closed
         assertThat(closedLatch.await(5, TimeUnit.SECONDS)).isTrue()
@@ -204,12 +201,12 @@
     fun modalDrawer_bodyContent_clickable() {
         var drawerClicks = 0
         var bodyClicks = 0
-        val drawerState = DrawerStateHolder(DrawerState.Closed)
+        val drawerState = mutableStateOf(DrawerState.Closed)
         composeTestRule.setMaterialContent {
             // emulate click on the screen
             TestTag("Drawer") {
                 Semantics(container = true) {
-                    ModalDrawerLayout(drawerState.state, { drawerState.state = it },
+                    ModalDrawerLayout(drawerState.value, { drawerState.value = it },
                         drawerContent = {
                             Clickable( drawerClicks += 1 }) {
                                 Box(Modifier.fillMaxSize(), children = emptyContent())
@@ -231,7 +228,7 @@
             assertThat(drawerClicks).isEqualTo(0)
             assertThat(bodyClicks).isEqualTo(1)
 
-            drawerState.state = DrawerState.Opened
+            drawerState.value = DrawerState.Opened
         }
         sleep(100) // TODO(147586311): remove this sleep when opening the drawer triggers a wait
 
@@ -255,11 +252,11 @@
         var openedHeight: IntPx? = null
         var openedLatch: CountDownLatch? = null
         var closedLatch: CountDownLatch? = CountDownLatch(1)
-        val drawerState = DrawerStateHolder(DrawerState.Closed)
+        val drawerState = mutableStateOf(DrawerState.Closed)
         composeTestRule.setMaterialContent {
             TestTag("Drawer") {
                 Semantics(container = true) {
-                    BottomDrawerLayout(drawerState.state, { drawerState.state = it },
+                    BottomDrawerLayout(drawerState.value, { drawerState.value = it },
                         drawerContent = {
                             Box(Modifier.fillMaxSize().onPositioned { info: LayoutCoordinates ->
                                 val pos = info.localToGlobal(PxPosition.Origin)
@@ -288,7 +285,7 @@
         // When the drawer state is set to Opened
         openedLatch = CountDownLatch(1)
         runOnIdleCompose {
-            drawerState.state = DrawerState.Opened
+            drawerState.value = DrawerState.Opened
         }
         // Then the drawer should be opened
         assertThat(openedLatch.await(5, TimeUnit.SECONDS)).isTrue()
@@ -296,7 +293,7 @@
         // When the drawer state is set to Closed
         closedLatch = CountDownLatch(1)
         runOnIdleCompose {
-            drawerState.state = DrawerState.Closed
+            drawerState.value = DrawerState.Closed
         }
         // Then the drawer should be closed
         assertThat(closedLatch.await(5, TimeUnit.SECONDS)).isTrue()
@@ -306,12 +303,12 @@
     fun bottomDrawer_bodyContent_clickable() {
         var drawerClicks = 0
         var bodyClicks = 0
-        val drawerState = DrawerStateHolder(DrawerState.Closed)
+        val drawerState = mutableStateOf(DrawerState.Closed)
         composeTestRule.setMaterialContent {
             // emulate click on the screen
             TestTag("Drawer") {
                 Semantics(container = true) {
-                    BottomDrawerLayout(drawerState.state, { drawerState.state = it },
+                    BottomDrawerLayout(drawerState.value, { drawerState.value = it },
                         drawerContent = {
                             Clickable( drawerClicks += 1 }) {
                                 Box(Modifier.fillMaxSize(), children = emptyContent())
@@ -335,7 +332,7 @@
         }
 
         runOnUiThread {
-            drawerState.state = DrawerState.Opened
+            drawerState.value = DrawerState.Opened
         }
         sleep(100) // TODO(147586311): remove this sleep when opening the drawer triggers a wait
 
diff --git a/ui/ui-material/src/androidTest/java/androidx/ui/material/ProgressIndicatorTest.kt b/ui/ui-material/src/androidTest/java/androidx/ui/material/ProgressIndicatorTest.kt
index fd9ab03..9df7c78 100644
--- a/ui/ui-material/src/androidTest/java/androidx/ui/material/ProgressIndicatorTest.kt
+++ b/ui/ui-material/src/androidTest/java/androidx/ui/material/ProgressIndicatorTest.kt
@@ -15,7 +15,7 @@
  */
 package androidx.ui.material
 
-import androidx.compose.Model
+import androidx.compose.mutableStateOf
 import androidx.test.filters.LargeTest
 import androidx.ui.core.TestTag
 import androidx.ui.foundation.Strings
@@ -33,11 +33,6 @@
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 
-@Model
-private class State {
-    var progress = 0f
-}
-
 @LargeTest
 @RunWith(JUnit4::class)
 class ProgressIndicatorTest {
@@ -51,12 +46,12 @@
     @Test
     fun determinateLinearProgressIndicator_Progress() {
         val tag = "linear"
-        val state = State()
+        val progress = mutableStateOf(0f)
 
         composeTestRule
             .setMaterialContent {
                 TestTag(tag = tag) {
-                    LinearProgressIndicator(progress = state.progress)
+                    LinearProgressIndicator(progress = progress.value)
                 }
             }
 
@@ -66,7 +61,7 @@
             .assertRangeInfoEquals(AccessibilityRangeInfo(0f, 0f..1f))
 
         runOnUiThread {
-            state.progress = 0.5f
+            progress.value = 0.5f
         }
 
         findByTag(tag)
@@ -115,12 +110,12 @@
     @Test
     fun determinateCircularProgressIndicator_Progress() {
         val tag = "circular"
-        val state = State()
+        val progress = mutableStateOf(0f)
 
         composeTestRule
             .setMaterialContent {
                 TestTag(tag = tag) {
-                    CircularProgressIndicator(progress = state.progress)
+                    CircularProgressIndicator(progress = progress.value)
                 }
             }
 
@@ -130,7 +125,7 @@
             .assertRangeInfoEquals(AccessibilityRangeInfo(0f, 0f..1f))
 
         runOnUiThread {
-            state.progress = 0.5f
+            progress.value = 0.5f
         }
 
         findByTag(tag)
diff --git a/ui/ui-material/src/androidTest/java/androidx/ui/material/RadioGroupUiTest.kt b/ui/ui-material/src/androidTest/java/androidx/ui/material/RadioGroupUiTest.kt
index 38f0b7d..38a7691 100644
--- a/ui/ui-material/src/androidTest/java/androidx/ui/material/RadioGroupUiTest.kt
+++ b/ui/ui-material/src/androidTest/java/androidx/ui/material/RadioGroupUiTest.kt
@@ -17,7 +17,7 @@
 package androidx.ui.material
 
 import androidx.compose.Composable
-import androidx.compose.Model
+import androidx.compose.mutableStateOf
 import androidx.test.filters.MediumTest
 import androidx.ui.core.TestTag
 import androidx.ui.foundation.Strings
@@ -36,9 +36,6 @@
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 
-@Model
-internal class RadioGroupSelectedState<T>(var selected: T)
-
 @MediumTest
 @RunWith(JUnit4::class)
 class RadioGroupUiTest {
@@ -73,7 +70,7 @@
 
     @Test
     fun radioGroupTest_defaultSemantics() {
-        val select = RadioGroupSelectedState(itemOne)
+        val selected = mutableStateOf(itemOne)
 
         composeTestRule.setMaterialContent {
             VerticalRadioGroupforTests {
@@ -81,8 +78,8 @@
                     TestTag(tag = item) {
                         RadioGroupTextItem(
                             text = item,
-                            selected = (select.selected == item),
-                             select.selected = item })
+                            selected = (selected.value == item),
+                             selected.value = item })
                     }
                 }
             }
@@ -95,7 +92,7 @@
 
     @Test
     fun radioGroupTest_ensureUnselectable() {
-        val select = RadioGroupSelectedState(itemOne)
+        val selected = mutableStateOf(itemOne)
 
         composeTestRule.setMaterialContent {
             VerticalRadioGroupforTests {
@@ -103,8 +100,8 @@
                     TestTag(tag = item) {
                         RadioGroupTextItem(
                             text = item,
-                            selected = (select.selected == item),
-                             select.selected = item })
+                            selected = (selected.value == item),
+                             selected.value = item })
                     }
                 }
             }
@@ -124,15 +121,15 @@
 
     @Test
     fun radioGroupTest_clickSelect() {
-        val select = RadioGroupSelectedState(itemOne)
+        val selected = mutableStateOf(itemOne)
         composeTestRule.setMaterialContent {
             VerticalRadioGroupforTests {
                 options.forEach { item ->
                     TestTag(tag = item) {
                         RadioGroupTextItem(
                             text = item,
-                            selected = (select.selected == item),
-                             select.selected = item })
+                            selected = (selected.value == item),
+                             selected.value = item })
                     }
                 }
             }
@@ -151,7 +148,7 @@
 
     @Test
     fun radioGroupTest_clickSelectTwoDifferentItems() {
-        val select = RadioGroupSelectedState(itemOne)
+        val selected = mutableStateOf(itemOne)
 
         composeTestRule.setMaterialContent {
             VerticalRadioGroupforTests {
@@ -159,8 +156,8 @@
                     TestTag(tag = item) {
                         RadioGroupTextItem(
                             text = item,
-                            selected = (select.selected == item),
-                             select.selected = item })
+                            selected = (selected.value == item),
+                             selected.value = item })
                     }
                 }
             }
diff --git a/ui/ui-material/src/main/java/androidx/ui/material/Color.kt b/ui/ui-material/src/main/java/androidx/ui/material/Color.kt
index 7ccf65b..eae3e7c 100644
--- a/ui/ui-material/src/main/java/androidx/ui/material/Color.kt
+++ b/ui/ui-material/src/main/java/androidx/ui/material/Color.kt
@@ -219,7 +219,7 @@
  * components that consume the specific color(s) that have been changed - so this default
  * implementation is intended to be memoized in the ambient, and then when a new immutable
  * [ColorPalette] is provided, we can simply diff and update any values that need to be changed.
- * Because the internal values are provided by an @Model delegate class, components consuming the
+ * Because the internal values are provided by an State delegate class, components consuming the
  * specific color will be recomposed, while everything else will remain the same. This allows for
  * large performance improvements when the theme is being changed, especially if it is being
  * animated.
diff --git a/ui/ui-material/src/main/java/androidx/ui/material/Scaffold.kt b/ui/ui-material/src/main/java/androidx/ui/material/Scaffold.kt
index 1c24800..954f11e 100644
--- a/ui/ui-material/src/main/java/androidx/ui/material/Scaffold.kt
+++ b/ui/ui-material/src/main/java/androidx/ui/material/Scaffold.kt
@@ -17,9 +17,12 @@
 package androidx.ui.material
 
 import androidx.compose.Composable
-import androidx.compose.Model
+import androidx.compose.Stable
+import androidx.compose.getValue
+import androidx.compose.mutableStateOf
 import androidx.compose.onDispose
 import androidx.compose.remember
+import androidx.compose.setValue
 import androidx.ui.core.Alignment
 import androidx.ui.core.DensityAmbient
 import androidx.ui.core.Layout
@@ -48,16 +51,18 @@
  * programmatically.
  * @param isDrawerGesturesEnabled whether or not drawer can be interacted with via gestures
  */
-@Model
+@Stable
 class ScaffoldState(
-    var drawerState: DrawerState = DrawerState.Closed,
-    var isDrawerGesturesEnabled: Boolean = true
+    drawerState: DrawerState = DrawerState.Closed,
+    isDrawerGesturesEnabled: Boolean = true
 ) {
 
+    var drawerState by mutableStateOf(drawerState)
+    var isDrawerGesturesEnabled by mutableStateOf(isDrawerGesturesEnabled)
     // TODO: add showSnackbar() method here
 
-    internal var fabConfiguration: FabConfiguration? = null
-    internal var bottomBarSize: IntPxSize? = null
+    internal var fabConfiguration: FabConfiguration? by mutableStateOf<FabConfiguration?>(null)
+    internal var bottomBarSize: IntPxSize? by mutableStateOf<IntPxSize?>(null)
 }
 
 object Scaffold {