Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with dequeueing cells in RightMenuViewController #836

Open
wutschel opened this issue Jan 17, 2023 · 7 comments
Open

Issue with dequeueing cells in RightMenuViewController #836

wutschel opened this issue Jan 17, 2023 · 7 comments

Comments

@wutschel
Copy link
Collaborator

Since #768 the App faces problems when sending it to sleep and resume again. #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
Copy link
Collaborator Author

wutschel commented Jan 17, 2023

@kambala-decapitator, to me it looks like 6e315fd and/or c6c51a7 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
Copy link
Collaborator Author

wutschel commented Jan 18, 2023

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
Copy link
Collaborator

to me it looks like 6e315fd and/or c6c51a7 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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants