5

Really strange problem I'm having here.

Basically, the user comes to a View that has a TableView attached to it. There's a call to the backend for some data, and in the completion block of that data is provided to the TableView to display.

In cellForRow 1 of 4 reusable, custom cells is dequeued, based on some logic, and then a configure method for that cell is called.

Here's where it gets tricky: Some of the cells might require further asynchronous calls (to fetch more data for their custom displays), thus required a tableView.reloadRows call...What's happening, then, is best explained by these two screenshots:

Wrong Right

In the first one, that cell with "Ever wonder what YOU'D do for a klondike bar?" is replicated on top of the "Self-Management for Actor's" cell which, as a cell containing Twitter info DID require an additional asynch call to fetch that data.

Any thoughts? I'm running the tableView.reloadRows call inside of a begin/endUpdates block, but no dice.

cellForRow:

override func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    let postAtIndexPath = posts[indexPath.row]
    let cell: PostCell!

    if postAtIndexPath.rawURL == "twitter.com" {
        cell = tableView.dequeueReusableCell(withIdentifier: "TwitterCell", for: indexPath) as! TwitterCell
    } else if !postAtIndexPath.rawURL.isEmpty {
        cell = tableView.dequeueReusableCell(withIdentifier: "LinkPreviewCell", for: indexPath) as! LinkPreviewCell
    } else if postAtIndexPath.quotedID > -1 {
        cell = tableView.dequeueReusableCell(withIdentifier: "QuoteCell", for: indexPath) as! QuoteCell
    } else {
        cell = tableView.dequeueReusableCell(withIdentifier: "PostCell", for: indexPath) as! PostCell
    }

    cell.isUserInteractionEnabled = true

    cell.privacyDelegate = self
    cell.shareDelegate = self
    cell.likeDelegate = self
    cell.linkPreviewDelegate = self
    cell.reloadDelegate = self
    postToIndexPath[postAtIndexPath.id] = indexPath

    if parentView == "MyLikes" || parentView == "Trending" {
        cell.privatePostButton.isHidden = true
        cell.privatePostLabel.isHidden = true
    }

    cell.configureFor(post: postAtIndexPath,
                         liked: likesCache.postIsLiked(postId: postAtIndexPath.id),
                         postIdToAttributions: postIdToAttributions,
                         urlStringToOGData: urlStringToOGData,
                         imageURLStringToData: imageURLStringToData)

    cell.contentView.layoutIfNeeded()

    return cell
}

asynch Twitter call:

private func processForTwitter(og: OpenGraph, urlLessString: NSMutableAttributedString, completion:(() -> ())?) {
    let ogURL = og[.url]!
    let array = ogURL.components(separatedBy: "/")
    let client = TWTRAPIClient()
    let tweetID = array[array.count - 1]
    if let tweet = twitterCache.tweet(forID: Int(tweetID)!)  {
        for subView in containerView.subviews {
            subView.removeFromSuperview()
        }

        let tweetView = TWTRTweetView(tweet: tweet as? TWTRTweet, style: .compact)
        containerView.addSubview(tweetView)
        tweetView.translatesAutoresizingMaskIntoConstraints = false
        tweetView.bottomAnchor.constraint(equalTo: containerView.bottomAnchor).isActive = true
        tweetView.topAnchor.constraint(equalTo: containerView.topAnchor).isActive = true
        containerView.leftAnchor.constraint(equalTo: tweetView.leftAnchor).isActive = true
        containerView.rightAnchor.constraint(equalTo: tweetView.rightAnchor).isActive = true
        containerView.isHidden = false
        postText.isHidden = false

    } else {
        client.loadTweet(withID: tweetID, completion: { [weak self] (t, error) in
            if let weakSelf = self {
                if let tweet = t {
                    weakSelf.twitterCache.addTweetToCache(tweet: tweet, forID: Int(tweetID)!)
                }
            }

            if let complete = completion {
                complete()
            }
        })`enter code here`
    }

    if urlLessString.string.isEmpty {
        if let ogTitle = og[.title] {
            postText.text = String(htmlEncodedString: ogTitle)
        }
    } else {
        postText.attributedText = urlLessString
    }
}

completion block in the tableviewcontroller:

func reload(postID: Int) {

    tableView.beginUpdates()
    self.tableView.reloadRows(at: [self.postToIndexPath[postID]!], with: .automatic)
    tableView.endUpdates()

}

NB: this doesn't happen 100% of the time, it is, presumably, network dependent.

Nick Coelius
  • 4,668
  • 3
  • 24
  • 30
  • Can u try tableView.reloadData instead of tableView.reloadRows – Shehata Gamal Feb 06 '18 at 19:48
  • A quick suggestion - Looking at your code I did not see any background calls, you can give it a try. Place network calls in the background and reloads table cells on the main thread. – Kampai Feb 13 '18 at 09:52

5 Answers5

1

After the async call, instead of tableView.reloadRows try to call:

tableView.beginUpdates()
tableView.setNeedsLayout()
tableView.endUpdates()

It seems like the data get populated, but the size of the cell does not get reflected. If you call reloadRows I'm afraid it does not recalculate the new positions of the following cells - that means if cell row 3 resizes to a bigger height, it will get bigger, but the following cells won't get pushed further down.

Milan Nosáľ
  • 19,169
  • 4
  • 55
  • 90
  • This....appears to be working! I moved some of my logic around (Previously the reloadRows call was utilizing a cache that I'd loaded after the first asynch call) and now...it seems happy? At least, I haven't observed it acting up yet! Thanks! – Nick Coelius Feb 06 '18 at 19:59
  • I spoke too soon - it's still happening :-/ – Nick Coelius Feb 06 '18 at 20:01
  • @NickCoelius can you include your code for `cellForRowAt`, and the async downloading code? – Milan Nosáľ Feb 06 '18 at 20:03
  • Edited with code. Q is getting long now, woof. That's the code as it stood before trying your answer, the version for your answer would just involve moving the constraint code etc. into the loadTweet callback and then adding setNeedsDisplsy obviously – Nick Coelius Feb 06 '18 at 20:08
  • @NickCoelius have you tried calling `reloadRows` AND `setNeedsDisplay` after? – Milan Nosáľ Feb 06 '18 at 20:24
  • Was JUST trying that, and it JUST failed after a few successful passes...So weird! – Nick Coelius Feb 06 '18 at 20:25
  • @NickCoelius are `processForTwitter` and `reload(postId:)` in the cell, or in the tableViewController? – Milan Nosáľ Feb 06 '18 at 20:26
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/164649/discussion-between-milan-nosa-and-nick-coelius). – Milan Nosáľ Feb 06 '18 at 20:28
  • setNeedsDisplay makes no sense, it is only used for views that custom draw their contents. You should use setNeedsLayout instead. – Werner Altewischer Feb 16 '18 at 15:21
  • @WernerAltewischer works both ways.. but I guess setNeedsLayout might be a more performant way, so I'll update the answer – Milan Nosáľ Feb 16 '18 at 15:27
1

I guess the issue lies in calculating the size of the components that are present on cell. It's like cell is dequed and frame along with subviews are set before data is available.

Maybe you can try,

tableView.rowHeight = UITableViewAutomaticDimension tableView.estimatedRowHeight = yourPrefferedHeight

but you need to make sure that your autolayout constraints are proper and cell can determine it's height if all the components has got height.

nikBhosale
  • 531
  • 1
  • 5
  • 27
  • Yeah, I have automatic dimension and estimated row height set but no dice. I'll double check my constraints though, maybe there's an issue there. – Nick Coelius Feb 13 '18 at 16:34
  • Maybe try adding a separate view in cell's contentView and add all your cell subviews on newly added view. And try giving some predefined height constraint to new view and see if cells are overlapping and afterwards you can arrange height constraints based on height of each individual view on cell. – nikBhosale Feb 14 '18 at 04:23
1

Turns out there's weird, or at least I don't understand it, behavior around estimatedRowHeight and reloadRows; solution, via a tangentially related SO question, was to cache the row heights as they become available and return that if available in heightForRow.

I guess it doesn't recalculate the heights, or something, on a reload? Really don't understand specifically WHY this works, but I'm happy it does!

iGatiTech
  • 2,306
  • 1
  • 21
  • 45
Nick Coelius
  • 4,668
  • 3
  • 24
  • 30
  • 1
    In my case, disabling estimated heights worked (`tableView.estimatedRowHeight=0`). I was getting the overlapping after a begin/endUpdates call on the table view, where the height of a cell would slightly need to change. – alex-i Apr 05 '19 at 14:30
  • @alex-i Your suggestion is working fine for me but when I tried to give estimatedRowheight as zero initially all the content in the cell are overlapped. Can you please assist me to fix that issue? – SRI Sep 11 '19 at 05:54
  • 1
    @SRI I can only guess the issues. I suggest setting the `tableView.rowHeight = ;`, and also make sure that you're also implementing the `-(CGFloat)tableView:(UITableView *)tableView heightForRowAtIndexPath:(NSIndexPath *)indexPath` delegate method – alex-i Sep 11 '19 at 06:20
0

I am assuming processForTwitter is called on background/async from within configureFor. If true this is the fault. Your api call should be on back ground but if you are getting tweet from your cache than it addSubview should be done on main thread and before returning cell. Try doing this, make sure that processForTwitter is called on main thread and configure it as

   private func processForTwitter(og: OpenGraph, urlLessString: NSMutableAttributedString, completion:(() -> ())?) {
        let ogURL = og[.url]!
        let array = ogURL.components(separatedBy: "/")
        let client = TWTRAPIClient()
        let tweetID = array[array.count - 1]
        if let tweet = twitterCache.tweet(forID: Int(tweetID)!)  {
            for subView in containerView.subviews {
                subView.removeFromSuperview()
            }

            let tweetView = TWTRTweetView(tweet: tweet as? TWTRTweet, style: .compact)
            containerView.addSubview(tweetView)
            tweetView.translatesAutoresizingMaskIntoConstraints = false
            tweetView.bottomAnchor.constraint(equalTo: containerView.bottomAnchor).isActive = true
            tweetView.topAnchor.constraint(equalTo: containerView.topAnchor).isActive = true
            containerView.leftAnchor.constraint(equalTo: tweetView.leftAnchor).isActive = true
            containerView.rightAnchor.constraint(equalTo: tweetView.rightAnchor).isActive = true
            containerView.isHidden = false
            postText.isHidden = false

        } else {
    // run only this code in background/async

            client.loadTweet(withID: tweetID, completion: { [weak self] (t, error) in
                if let weakSelf = self {
                    if let tweet = t {
                        weakSelf.twitterCache.addTweetToCache(tweet: tweet, forID: Int(tweetID)!)
                    }
                }
// completion on main

                if let complete = completion {
  DispatchQueue.main.async {
 complete()

    }   
                }
            })
        }

        if urlLessString.string.isEmpty {
            if let ogTitle = og[.title] {
                postText.text = String(htmlEncodedString: ogTitle)
            }
        } else {
            postText.attributedText = urlLessString
        }
    }

Now sequence would be like this

  1. Cell is loaded for first time
  2. No tweet found in cache
  3. Async network call in processForTwitter and cell is returned without proper data
  4. Async call gets tweet. And calls completion on main thread which results in reload
  5. configure for is called again on main thread
  6. tweet is found in cache while remaining on main thread.
  7. add your TWTRTweetView (you are still on main thread) and this will be done before your cell is returned.

remember this, every component in your cell which will play role in calculating size of cell should be added on main thread before returning cell in cellForIndexPath

ibnetariq
  • 718
  • 1
  • 4
  • 20
  • 1
    Not sure if this would work or not, I'm pretty sure I was calling everything from the main thread at various points. As it stands, I actually just yesterday finally found a solve - apparently there's weird behavior surrounding estimatedRowHeight and reloadRows, so I now cache the row heights and return that if available in heightForRow...works like a charm! – Nick Coelius Feb 14 '18 at 16:51
0

I came across a similar problem. After

    [self.tableView beginUpdates];
    [self configureDataSource]; // initialization of cells by dequeuing 
    [self.tableView insertSections:[[NSIndexSet alloc] initWithIndex:index] withRowAnimation:UITableViewRowAnimationFade];
    [self.tableView endUpdates];

and also after

    [self.tableView beginUpdates];
    [self configureDataSource]; // initialization of cells by dequeuing  
    [self.tableView deleteSections:[NSIndexSet indexSetWithIndex:index] withRowAnimation:UITableViewRowAnimationFade];
    [self.tableView endUpdates];

two rows of my TableViewController were overlapped.

Then I found out where's the catch.

After beginUpdate and endUpdate are two methods called for cells:estimatedHeightForRowAtIndexPath and heightForRowAtIndexPath. But there's no call of cellForRowAtIndexPath.

One of overlapping cells was dequeued but I set it's text/value only in cellForRowAtIndexPath method. So cell height was computed correctly but for incorrect content.

My fix was to set text/value also after dequeuing the cell. So the only difference is inside my configureDataSource method:

    [self.tableView beginUpdates];
    [self configureDataSource]; // init of cells by dequeuing and set its texts/values  
    // delete or insert section
    [self.tableView endUpdates];

(Other possible fix is to avoid reinitialization of cells. But this caching of initialized cells wouldn't be good for TableViews with tons of cells)

Marek Manduch
  • 2,303
  • 1
  • 19
  • 22