4

I have a UITableView which loads images asynchronously in a UITableView. As all the images are different heights, I set a height constraint according to the height of the image.

This works fine when I use DataWithContentsOfUrl, however freezes the UI. When I load asynchronously, the images pile up on top of each other like so:

https://i.stack.imgur.com/TEUwK.png

The code i'm using is as follows:

NSURL * imageURL = [NSURL URLWithString:img];
NSURLRequest* request = [NSURLRequest requestWithURL:imageURL];
cell.imageViewHeightConstraint.constant=0;
[NSURLConnection sendAsynchronousRequest:request    queue:[NSOperationQueue mainQueue]  completionHandler:^(NSURLResponse * response,   NSData * data,  NSError * error) {
    if (!error){
        UIImage *imgz = [UIImage imageWithData:data];
        [cell.CellImg setBackgroundImage:imgz forState:UIControlStateNormal];
        [cell.CellImg sizeToFit];
        cell.imageViewHeightConstraint.constant=result.width*(imgz.size.height/imgz.size.width);
        [cell setNeedsUpdateConstraints];
    }
}];
[cell updateConstraints];

When I scroll up and down a few times the images correctly position themselves.

dblythy
  • 108
  • 1
  • 7

3 Answers3

2

The question doesn't specify the desired behavior for varying image sizes. Should the cell get taller to fit, or just the image view (what looks like a button from the code) within the cell?

But we should put aside that problem for a moment and work on the more serious problem with the code: it issues an unguarded network request from cellForRowAtIndexPath. As a result, (a) a user scrolling back and forth will generate many many redundant requests, and (b) a user scrolling a long way quickly will generate a request that's fulfilled when the cell that started it is gone - reused as the cell for another row.

To address (a), the datasource should cache fetched images, and only request those that haven't been received. To address (b), the completion block shouldn't refer directly to the cell.

A simple cache would look like this:

@property(strong,nonatomic) NSMutableDictionary *images;

// initialize this when you initialize your model
self.images = [@{} mutableCopy];

// move the network code into its own method for clarity
- (void)imageWithPath:(NSString *)path completion:(void (^)(UIImage *, NSError *))completion {
    if (self.images[indexPath]) {
        return completion(self.images[indexPath], nil);
    }
    NSURL *imageURL = [NSURL URLWithString:path];
    NSURLRequest *request = [NSURLRequest requestWithURL:imageURL];
    [NSURLConnection sendAsynchronousRequest:request queue:[NSOperationQueue mainQueue] completionHandler:^(NSURLResponse *response, NSData *data, NSError *error) {
        if (!error){
            UIImage *image = [UIImage imageWithData:data];
            self.images[indexPath] = image;
            completion(image, nil);
        } else {
            completion(nil, error);
        }
    }];
}

Now, we fix the multiple request problem by checking first for the image in the cache in cellForRowAtIndexPath.

UIImage *image = self.images[indexPath];
if (image) {
    [cell.CellImg setBackgroundImage:image forState:UIControlStateNormal];
} else {
    // this is a good place for a placeholder image if you want one
    [cell.CellImg setBackgroundImage:nil forState:UIControlStateNormal];
    // presuming that 'img' is a string from your mode
    [self imageWithPath:img completion:^(UIImage *image, NSError *error) {
        // the image is ready, but don't assign it to the cell's subview
        // just reload here, so we get the right cell for the indexPath
        [tableView reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationAutomatic];
    }];
}

Also notice what isn't done in the completion block... we're fixing the cell reuse by not referring to the cell. Instead, knowing that the image is now cached, we reload the indexPath.

Back to image sizing: most apps like to see the table view cell get taller or shorter along with the variable height subview. If that's the case, then you should not place a height constraint on that subview at all. Instead, constrain it's top and bottom edges to the cell's content view (or include it in a chain of subviews who constrain to each other top and bottom and contain the topmost and bottommost subviews top and bottom edge to the cell). Then (in iOS 5+, I think), this will allow your cell to change hight with that subview constraint chain...

self.tableView.rowHeight = UITableViewAutomaticDimension;
self.tableView.estimatedRowHeight = // your best guess at the average height here
danh
  • 62,181
  • 10
  • 95
  • 136
  • I did have an NSCache in there, just didn't show it as I didn't think it was the reason behind the error. – dblythy Sep 10 '15 at 13:42
0

Try the following along with constraint constant:

cell.imageViewHeightConstraint.constant=result.width*(imgz.size.height/imgz.size.width);
[table reloadRowsAtIndexPaths:@[indexPathForCurrentCell] withRowAnimation:UITableViewRowAnimationNone];

You can also set the animation.

hasan
  • 23,815
  • 10
  • 63
  • 101
0

Although reloadRowsAtIndexPaths works, it ends up causing the interface to be slow and laggy, especially when the user is scrolling. My solution was once the data was loaded to iterate through https://graph.facebook.com/v2.1/%@/?access_token=%@&fields=attachments for the object id, and then store the image height dimensions in an array. Then, in cellForRowAtIndexPath simply setting cell.imageViewHeightConstraint.constant to the value in the array. That way all the cells heights are set as soon as the cell loads, and the image fits into place perfectly as soon as it loads.

dblythy
  • 108
  • 1
  • 7