Litho: Updating the state in section is not updating the state of the view

[vkalmath]

• [x] I have searched existing issues and this is not a duplicate

Version

lithoVersion = '0.11.0'
// Litho
implementation "com.facebook.litho:litho-core:${lithoVersion}"
implementation "com.facebook.litho:litho-widget:${lithoVersion}"
compileOnly "com.facebook.litho:litho-annotations:${lithoVersion}"
kapt "com.facebook.litho:litho-processor:${lithoVersion}"

// Sections
implementation "com.facebook.litho:litho-sections-core:${lithoVersion}"
implementation "com.facebook.litho:litho-sections-widget:${lithoVersion}"
compileOnly "com.facebook.litho:litho-sections-annotations:${lithoVersion}"

kapt "com.facebook.litho:litho-sections-processor:${lithoVersion}"

Issues and Steps to Reproduce

I am using the section api of Litho. This sections are backed by the List. Each item in the list is a RadioGroup. When I change the selection in radio group an event is emitted and that is handled in the parent component which is GroupSection. In that component I have a method with is annotated with OnUpdateState and is called from Eventhandler. As this is a list if I slowly scroll to second item(next set of radio group) and and scrollback to original list item, the radio selection does not reflect the changed selection. If I scroll to the end of the list and come back that time the list shows changed selection.

Expected Behavior

slow scrolling should also show the changed state of list item.

Link to Code

@GroupSectionSpec
object OrderItemListSectionSpec {

@OnCreateInitialState
internal fun createInitialState(
        c: SectionContext,
        orders: StateValue<List<*>>,
        @Prop orderList: List<ViewOrderResponse.Order>) {
    orders.set(orderList)
}

@OnUpdateState
fun updateOrders(orders: StateValue<List<*>>,  @Param status: String?, @Param isCheckinEnabled: Boolean?, @Param index: Int) {
   //updating the state of item
    if (status != null) {
        val orderList: List<ViewOrderResponse.Order> = orders.get() as List<ViewOrderResponse.Order>
        orderList[index].orderStatus = status
    }

    .....
}

@OnCreateChildren
internal fun onCreateChildren(c: SectionContext,
                              @State orders: List<*>,
                              @Prop orderList: List<ViewOrderResponse.Order>,
                              @Prop(optional = true) orderStateChglistener: ((Int, String) -> Unit)?,
                              @Prop(optional = true) checkinStateChgListener: ((Int, Boolean?) -> Unit)?): Children {
    val builder = Children.create()

    for (i in orders.indices) {
        builder.child(
                SingleComponentSection.create(c)
                        .key(i.toString())
                        .component(OrderItem.create(c)
                                .key("key ${i}")
                                .order(orders[i] as ViewOrderResponse.Order)
                                .orderStateChglistener(orderStateChglistener)
                                .checkinStateChgListener(checkinStateChgListener)
                                .orderStatusHandler(OrderItemListSection.onOrderUpdated(c))
                                .index(i)
                                .build()))

    }
    return builder.build()
}

@OnEvent(UpdateOrderStatusEvent::class)
fun onOrderUpdated(
        c: SectionContext,
        @FromEvent index: Int,
        @FromEvent status: String) {

   // updating the sate
    OrderItemListSection.updateOrders(c, status, null, index)
}

}

videos.zip

[IanChilds]

Can you log to see if the update state method is being called? Is the event being called?

[vkalmath]

yes updateOrders is being called when the radio selection is changed...

[pasqualeanatriello]

Your code seems to be mutating the state rather than creating a new state
value. State is supposed to be immutable and to change the selection state
you should create a new list with a new (immutable) object with the new
selection state. If you simply mutate the old state the diffing logic won't
be able to determine that something in your list changed.

On Tue, 27 Mar 2018, 20:47 Veeresh, [e-mail] wrote:

yes updateOrders is being called when the radio selection is changed...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/facebook/litho/issues/333#issuecomment-376633426, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAaY4BzZ58uLd9xr8EprNpGBXivVPY3tks5tiokmgaJpZM4S6nnf
.

[vkalmath]

I've tried that also, still no luck.. Is there anything wrong with below code?

@OnUpdateState
    fun updateOrders(orders: StateValue<List<*>>,  @Param status: String?, @Param isCheckinEnabled: Boolean?, @Param index: Int) {
        if (status != null) {
            val orderList: List<ViewOrderResponse.Order> = orders.get() as List<ViewOrderResponse.Order>
            orderList[index].orderStatus = status
            val newList = ArrayList<ViewOrderResponse.Order>()
            newList.addAll(orderList)
            orders.set(newList)
        }
      ....
    }

[pasqualeanatriello]

You are still mutating the order object when you do
orderList[index].orderStatus = status.
Then you just copy the order that you just mutated into a new list.
You want to create a new list that contains a new (different) order object
for anything that needs to change.

On Tue, 27 Mar 2018, 21:08 Veeresh, [e-mail] wrote:

I've tried that also, still no luck.. Is there anything wrong with below
code?
@onupdatestate
fun updateOrders(orders: StateValue>, @param
https://github.com/param status: String?, @param
https://github.com/param isCheckinEnabled: Boolean?, @param
https://github.com/param index: Int) {

if (status != null) {
val orderList: List = orders.get() as
List
orderList[index].orderStatus = status

val newList = ArrayList()
newList.addAll(orderList)
orders.set(newList)
}
....
}

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/facebook/litho/issues/333#issuecomment-376640244, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAaY4GdlweYD4EA4vJ6sJjOG3vCWzld8ks5tio5GgaJpZM4S6nnf
.

[vkalmath]

Thank you guys. You are amazing. That worked.

@OnUpdateState
    fun updateOrders(orders: StateValue<List<*>>,  @Param status: String?, @Param isCheckinEnabled: Boolean?, @Param index: Int) {
        if (status != null) {
            val orderList: List<ViewOrderResponse.Order> = orders.get() as List<ViewOrderResponse.Order>
            orderList[index].orderStatus = status
            val newList = ArrayList<ViewOrderResponse.Order>()
            for(order in orderList){
                val newOrder = UpdateOrderStatusEvent.deepClone(order)
                newList.add(newOrder)
            }
            orders.set(newList)
        }
      ......
    }

Is there a efficient way to handle this scenario. Creating a new list every time is expensive..
can I use code below which works, where only the changed item is created newly?

if (status != null) {
            val orderList = orders.get() as MutableList<ViewOrderResponse.Order>
            orderList[index].orderStatus = status
            orderList[index] = UpdateOrderStatusEvent.deepClone(orderList[index])
            val newList = ArrayList<ViewOrderResponse.Order>()
            newList.addAll(orderList)
            orders.set(newList)
        }

[pasqualeanatriello]

Only recreating the object that changed is not only allowed but also
advisable as also diffing will be much more efficient. The only thing is
that in your sample code I still see you modifying the order in the
original list. You want to update the order status on the cloned object
rather than the original one

On Tue, 27 Mar 2018, 23:18 Veeresh, [e-mail] wrote:

Thank you guys. You are amazing. That worked.

@OnUpdateState
fun updateOrders(orders: StateValue>, @Param status: String?, @Param isCheckinEnabled: Boolean?, @Param index: Int) {
if (status != null) {
val orderList: List = orders.get() as List
orderList[index].orderStatus = status
val newList = ArrayList()
for(order in orderList){
val newOrder = UpdateOrderStatusEvent.deepClone(order)
newList.add(newOrder)
}
orders.set(newList)
}
......
}

Is there a efficient way to handle this scenario. Creating a new list
every time is expensive..
can I use code below which works, where only the changed item is created
newly?

if (status != null) {
val orderList = orders.get() as MutableList
orderList[index].orderStatus = status
orderList[index] = UpdateOrderStatusEvent.deepClone(orderList[index])
val newList = ArrayList()
newList.addAll(orderList)
orders.set(newList)
}

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/facebook/litho/issues/333#issuecomment-376678876, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAaY4NxRmYFU-nZV0sF1CYt4kTzdraNeks5tiqyogaJpZM4S6nnf
.

[vkalmath]

Thank-you. That helped a lot.....

[pasqualeanatriello]

Happy it helped :) let's close the issue?

On Tue, 27 Mar 2018, 23:29 Veeresh, [e-mail] wrote:

Thank-you. That helped a lot.....

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/facebook/litho/issues/333#issuecomment-376681821, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAaY4NgLEcF_RztBxIJmRjShSxxcCXKxks5tiq8ggaJpZM4S6nnf
.