defagos / SwiftUICollection

A collection view for SwiftUI
MIT License
91 stars 6 forks source link

Cells can become unfocusable #6

Closed danielsaidi closed 3 years ago

danielsaidi commented 3 years ago

I have a strange problem that occurs when I use the shelf layout (not the grid layout).

My home screen has a paginated collection with two initial lists. The screen then loads new lists as the user scrolls. Since the first two lists fit the screen, an initial pagination will take place immediately after they are loaded into the screen.

I first noticed that the section titles became empty when the pagination took place immediately. I therefore tried delaying the first pagination 1 second. This worked, but I then noticed another problem.

Take a look at the attached gif. In this movie, I have delayed the first pagination 3 seconds. If I navigate down to an item before the first pagination takes place, the item will deselect once new data is loaded into the collection. After this, I can never select the item again.

IMG_2948

defagos commented 3 years ago

Well, that's quite strange. I didn't experience this issue while writing the CollectionView or now while using it. You should probably check that items are correctly focusable.

Your question is also blurring the line between issues in your implementation and issues in mine. I am not sure if the problem you describe is a problem related to my implementation or if it is related to your own collection view inspired by my implementation.

An issue on the present repository is probably not the best the place to discuss your implementation issues, otherwise people looking here will have a hard time figuring out issues related to my implementation (which I am sure some can be found).

danielsaidi commented 3 years ago

Hi again,

The problem arises when the screen appends new rows to the collection view. I have to delay the first implicit pagination, otherwise the section headers stay blank until they leave the screen and reappears. I also experience the focus problems that the movie above demonstrate.

My implementation is just a derivate of your original code, where I've redesigned the api:s slightly. The collection view logic is intact, though, and I also do not tamper with any focus in my app, which I know that the original code does to solve focus problems on tvOS. I haven't had the time to dive into these problems, but will do so when I get some spare time on my hands.

Even though I think that something in the original implementation (or the way tvOS wraps it into SwiftUI) causes these focus problems, I want to emphasize that I post here only to make you aware of any problems that may exist. I don't expect you to spend time on these edge cases and will inform you of any findings I may stumble across while investigating.

If you think it may be distracting to others, I'd be happy to move this issue to my own repo, but chances are that you may get new reports when others try to do the same thing :)

defagos commented 3 years ago

Hi,

Thanks for your feedbacks. Of course if you think the problems are related to this implementation this is the place to report them.

Focus is really a tricky beast on tvOS and in SwiftUI things are for the moment even harder. I'll keep an eye for related issues and will of course update this implementation to fix them if I can.

I really would like to investigate and fix all reported issues if they are related to my implementation, but at the moment I am sadly quite busy, so I can only really fix what I find within the app I am implementing (and for the moment I had no issues, otherwise my implementation would already have been updated 🙂 ).

danielsaidi commented 3 years ago

Sounds great - keep your eyes open for any unexpected behavior, and I will dig deeper into this problem and see if I can find anything as well. Maybe it has to do with the focus fiddling and dequeueing when the collection view is reloaded? Anyhow, I will spend some time with it this weekend and hope that I have something to share after that.

danielsaidi commented 3 years ago

Ok, here's an update.

The way that I did lazy load more content into the collection view was to trigger a lazy load operation when a new cell was being displayed. If so, a loading flag was set to true, which blocked any other operation from being performed.

However, I believe that this could have caused the collection view to be updated multiple times in a very short time, which may have messed up some internal state.

I now rewrote the logic to instead look at if the displayed item was the first item in the last section. Only then was the lazy load operation allowed to proceed. This seems to have solved the problem. At least, I can't reproduce it now.

So even though there may be some risk of temporarily messing up state in the collection view, being careful with how you add more data into it seems to reduce the risk of this happening.

I will close this now and add any more information if my assumption that I've solved the problem proves to be wrong.

defagos commented 3 years ago

Thanks for the update. Quite interesting, as the diffable data source should guarantee an internally consistent state. I guess there was no update triggered from a background thread (coz this kind of behavior would make me think of such kinds of issues).

danielsaidi commented 3 years ago

I wonder if some cells are dequeued in the middle of a reload, while focus is disabled...could that be it? It's the only thing I can think of, since reloading the collection view fiddles with its focusable state, but I haven't done anything to confirm this :)