Issue with dequeueing cells in RightMenuViewController

[wutschel]

Since https://github.com/xbmc/Official-Kodi-Remote-iOS/pull/768 the App faces problems when sending it to sleep and resume again. https://github.com/xbmc/Official-Kodi-Remote-iOS/pull/811 aimed to fix this with setting the layout freshly for each call. Still the App faces layout issues after sleep/resume.

The problem becomes visible when having 5 or more custom buttons, including 1 custom button with a one-off switch. How to reproduce:

1. Enter custom button menu
2. Send App to sleep
3. Resume App -> Now the text label is not properly sized and positioned

Worse:

1. Enter custom button menu
2. Enter edit mode
3. Send App to sleep
4. Resume App -> Now the text label is not properly sized and positioned

[wutschel]

@kambala-decapitator, to me it looks like 6e315fda and/or c6c51a7c caused this. Can you help me understand what is the difference between the following implementations in detail? It seems that the old implementation enforced a clean assignment of the custom button cell each time by cell = rightMenuCell, only the volume/serverstatus/remote cells were using dequeueing. Isn't this simply overwriting any dequeued custom button cell?

old (working):

UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"rightMenuCell"];
[[NSBundle mainBundle] loadNibNamed:@"rightCellView" owner:self options:NULL];
if (cell == nil) {
    cell = rightMenuCell;
    ...
}
if (serverstatuscell || volumecell || remote cell) {
    ...
}
else { // the standard custom button cell
    cell = rightMenuCell;
    ...
}

new (issues):

UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"rightMenuCell"];
if (cell == nil) {
    NSArray *nib = [[NSBundle mainBundle] loadNibNamed:@"rightCellView" owner:self options:nil];
    cell = nib[0];
    ...
}
...

Edit: What also works is enforcing reloading the cell each time by simply removing the condition:

UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"rightMenuCell"];
//if (cell == nil) {
    NSArray *nib = [[NSBundle mainBundle] loadNibNamed:@"rightCellView" owner:self options:nil];
    cell = nib[0];
    ...
//}
...

Edit2: This also works.

UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"rightMenuCellIdentifier"];
if (cell == nil) {
    NSArray *nib = [[NSBundle mainBundle] loadNibNamed:@"rightCellView" owner:self options:nil];
    cell = nib[0];
    ....
}
else {
    [[NSBundle mainBundle] loadNibNamed:@"rightCellView" owner:self options:NULL];
    cell = rightMenuCell;
}
...

[wutschel]

Looked into this further. As already suspected the problem was finally introduced when I removed the additional cell = rightMenuCell; assignment for the condition which handled custom buttons. To my understanding this assignment simply overwrote the dequeued cell with a fresh one. I provided two possible work around solutions for this in #837.

[kambala-decapitator]

to me it looks like https://github.com/xbmc/Official-Kodi-Remote-iOS/commit/6e315fda7d031d1bddd211d03931c9db0d3a5bf8 and/or https://github.com/xbmc/Official-Kodi-Remote-iOS/commit/c6c51a7c4c0c1b40b540a510215c29edb76b233c caused this

FYI you could've used git bisect to find exact commit where it broke

in all the snippets dequeuing a cell and then immediately replacing it with a new object looks strange as it basically kills the whole purpose of cell recycling/reusing. But if you found a workaround, let's try it, although in the end all such code should be refactored to work properly.

[wutschel]

Thanks, this confirms my thoughts. It means the old implementation (App version 1.11 or earlier) did in fact never use the dequeued cells for the power menu or any of the custom buttons. Dequeueing only was effective for a subset of the cells (only iPhone, new iPhones: server status, old iPhone with small display: server status, volume slider, keyboard, gesture toggle, help, LED torch).

[wutschel]

Next issue which I need to handle is solved in #838. In this case RemoteControllerView was initialized on a certain cell. If not exactly the same cell was used again (e.g. after sleep/resume) the layout broke.

I assume the best will be to create different cell layouts and introduce dequeueing for each of them to avoid re-creation each time, but at the same time avoid mixing the types which obviously causes auto layout issues.

This could look something like this:

if (type A) {
    cell = dequeue(type A)
    if (!cell) {
        cell = new type A
    }
}
else if (type B) {
    cell = dequeue(type B)
    if (!cell) {
        cell = new type B
    }
}
else {
    cell = dequeue(type default
    if (!cell) {
        cell = new type default
    }
}

[kambala-decapitator]

yes, in general your proposal is correct. But to be more technical, the code would be slightly different:

1. use https://developer.apple.com/documentation/uikit/uitableview/1614888-registerclass?language=objc (or similar method for to register xib) at table/collection creation time to register all possible cell "layouts" (i.e. classes or xibs). Personally I'd prefer to avoid xibs and also use auto layout instead of manual frame computation to make it easier to read / modify / review, but xibs would also do if you think that's too much for you to learn at once :)
2. dequeue cells with https://developer.apple.com/documentation/uikit/uitableview/1614878-dequeuereusablecellwithidentifie?language=objc method (that has second parameter). This way cell object is guaranteed to be not nil thus allowing to drop the if (!cell) part.

[AbdelAzizMohamedMousa]

Thank you for providing the update on your progress with resolving the layout issue in your iOS app. It sounds like you have identified a solution involving creating separate cell layouts and using dequeueing to avoid re-creation each time, while also avoiding mixing the types to prevent auto layout issues.

The code snippet you provided looks like a good starting point for implementing this solution. You could create separate cell classes for each type of layout you need, and then use the appropriate class based on the type of data you are displaying in the cell. The dequeue() method could be used to obtain an existing cell of the appropriate class, or create a new cell if one does not exist.

To further optimize performance, you could also consider implementing reuse identifiers for each cell class to improve the efficiency of the dequeueing process.

I hope this helps, and please let me know if you have any additional questions or concerns.