nativescript-community / ui-collectionview

Allows you to easily add a collection view (grid list view) to your projects. Supports vertical and horizontal modes, templating, and more.
Apache License 2.0
59 stars 18 forks source link

Different behaviour 4.x 5.x refreshVisibleItems/refresh #59

Closed jcassidyav closed 1 year ago

jcassidyav commented 1 year ago

Which platform(s) does your issue occur on?

Please, provide the following version numbers that your issue occurs with: 5.0.2

Please, tell us how to recreate the issue in as much detail as possible.

Run the attached project and tap either refresh or refreshVisibleItems a few times.

The ObservableArray bound to the collectionView does not change, but you can see the background colours of the cells fluctuates.

switching collectionView to 4.x you don't see this behaviour.

Is there any code involved?

This is a sample project: testlist.zip

farfromrefug commented 1 year ago

@jcassidyav can you try 5.0.3 should be fixed

jcassidyav commented 1 year ago

@farfromrefug Yes that fixes the issue in the sample, however it then allows me to illustrate the following issue:

In this project testlist2.zip, the 3rd item should change height ( as a portion is collapsed based on a toggled value) when the refresh/refreshVisible items is clicked.

When you click refresh it does, but clicking refreshVisibleItems it does not. Shouldn't both methods have the same visible effects?

farfromrefug commented 1 year ago

@jcassidyav thanks a lot I ll fix it. Correspond to another user issue. Sorry I worked on making the iOS version much faster, but there were some regressions... No the 2 methods dont do the same. RefreshVisibleItems should be much faster. Refresh do everything from scratch

farfromrefug commented 1 year ago

@jcassidyav actually that resize but was always there. It also exists in 4.x. I ll look a bit at refreshVisibleItems but what you are trying to do is the wrong way to do it. If you want to update a single item (even size) you should use this.items.setItem(2, item). And it will only update the changed item which will be much faster than refreshVisibleItems Though i see an issue with it in 5.x!

jcassidyav commented 1 year ago

@farfromrefug yeah you are of course correct, this usage is buried in an old app with many complicated lists so didn't really want to refactor it. The strange thing is that in this app 4.x does work and the change is reflected but when we drop 5.x in it doesn't, but in this small example app I see that 4.x doesn't work correctly either.

But this.items.setItem(2, item) really is the correct way to do it.

farfromrefug commented 1 year ago

@jcassidyav here this case of refreshVisibleItems does not work either in 4.x But the setItem cell size update is broken in 5.x! EDIT: @jcassidyav actually was not working either in 4.x Just the rendering is not the same with 5.x :s

farfromrefug commented 1 year ago

@jcassidyav ok i found the issue and it does not seem to be an issue in the plugin but in the core xml. When you update an item with setItem then a cell measurement is called then a layout. In the cell measurement we update the cell binding context but the view property is not updated yet (might be async) so the measurement is wrong (still collapsed when should be visible). but then in cell layout it works and is updated

I tested with svelte and vue and they work correctly in cell measurement

jcassidyav commented 1 year ago

@farfromrefug ah ok that makes sense now since my app is angular it probably explains 4.x working there as is as well, Will test and let you know.

jcassidyav commented 1 year ago

@farfromrefug I think that PR should fix the issue ( it just replicates what happens when the ObservableArray is updated ).

I also confirmed 'setItem' works as expected in a small angular app. so with this PR I think it refreshes correctly in all cases.