CreateBinding is being called each time item is being collapsed or expanded.
Ceees2 opened this issue · 8 comments
About this issue
When using AbstractBindingItem and the expandable extension, expanding/collapsing an item causes createBinding to be called each time. I feel like this is not intended behaviour, since the actual clicked item is not dissapearing/appearing from/on the screen. And also because when using AbstractItem a new view holder is not being created newly each time on expanding/collapsing. Correct me if I'm wrong.
I've previously created a pull request to "fix" something caused by this, but rather than fixing it, it was more a work around and preventing us from using notifying about item changes.
If this is indeed wrong/unintended behaviour i'd be happy to help out fixing it.
Details
- Used library version - Latest
- Used support library version - Latest
- Used gradle build tools version - Several - 7.2
- Used tooling / Android Studio version - Bumblebee
- Other used libraries, potential conflicting libraries - inapplicable
@Ceees2 you can modify this behaviour by configuring notifyOnAutoToggleExpandable within the ExpandableExtension
It's used here when we toggle the expandable state for the clicked item:
https://github.com/mikepenz/FastAdapter/blob/develop/fastadapter-extensions-expandable/src/main/java/com/mikepenz/fastadapter/expandable/ExpandableExtension.kt#L168
to:
https://github.com/mikepenz/FastAdapter/blob/develop/fastadapter-extensions-expandable/src/main/java/com/mikepenz/fastadapter/expandable/ExpandableExtension.kt#L309
to:
https://github.com/mikepenz/FastAdapter/blob/develop/fastadapter-extensions-expandable/src/main/java/com/mikepenz/fastadapter/expandable/ExpandableExtension.kt#L401-L404
If you don't notify this item, you'd need to make sure alternatively to highlight any indicator that it's expanded
@Ceees2 you can modify this behaviour by configuring
notifyOnAutoToggleExpandablewithin theExpandableExtensionIt's used here when we toggle the expandable state for the clicked item: https://github.com/mikepenz/FastAdapter/blob/develop/fastadapter-extensions-expandable/src/main/java/com/mikepenz/fastadapter/expandable/ExpandableExtension.kt#L168 to: https://github.com/mikepenz/FastAdapter/blob/develop/fastadapter-extensions-expandable/src/main/java/com/mikepenz/fastadapter/expandable/ExpandableExtension.kt#L309 to: https://github.com/mikepenz/FastAdapter/blob/develop/fastadapter-extensions-expandable/src/main/java/com/mikepenz/fastadapter/expandable/ExpandableExtension.kt#L401-L404
If you don't notify this item, you'd need to make sure alternatively to highlight any indicator that it's expanded
I know, that was my proposed solution in the linked PR, but I feel like that is more a work around than an actual solution. notifyOnAutoToggleExpandable false is fixing the animation issue, but causes the inability to used the extensions expand or collapse since click listeners wont be called and thus no animations. notifyOnAutoToggleExpandable true would somewhat fix that issue but can cause any unwanted animation behaviour. The linked PR also shows this behaviour is actually present in the current example app.
Might also just be that I'm trying to do something thats just not possible, but not giving up yet.
Sorry, missed that this was the exact thing you had linked.
One potential solution would be to do the notify with a payload in this case, so you could manually handle the bind in case of the existence of the payload, allowing you to update the UI?
Sorry, missed that this was the exact thing you had linked.
One potential solution would be to do the
notifywith apayloadin this case, so you could manually handle thebindin case of the existence of the payload, allowing you to update the UI?
I thought about that, but that would mean updating the expand and collapse methods wouldn't it?
Would it also be possible to call bindView/unbindView with the current view instead of creating a new view. Seems like that's also causing issues for animations if the starting state of the view (when starting the animation) is different than when the view is created initially. eg the jumping of the expand icon; when the item is expanded and you want to collapse, the icon jumps to the collapsed rotation(since the view is being recreated), then immediately back to the expanded rotation and then the animation starts to rotate it back to collapse.
@Ceees2 it will mean we have to update the extension, but it would still be a good option, and consumers of the SDK can start acting on it optionally.
No it's not anticipated to call bindView/unbindView with the current view as those methods are called by the RecyclerView as the RV will properly handle recycling the re-use of views, and any interception of that is most likely causing significant issues.
The animation parts can all be handled by making use of the payload, as within the bindView the RV will make sure the right view is provided (reused or new) and based on the payload you can then run the animation
That was fast, thanks for all your work.
Works like a charm, I've completely removed the click listener triggering the animation and am using the payload now to determine if animations should be shown, and no need to use notifyOnAutoToggleExpandable = false anymore. All in all nice change, thanks a bunch. 👌
If wanted I can also open a PR updating and applying these changes in the expand example in the fast adapter example app @mikepenz