Skip to content
This repository has been archived by the owner on Aug 1, 2019. It is now read-only.

[head prefetch] Prefetching does not seem to keep the correct position #185

Open
frenetisch-applaudierend opened this issue Jan 31, 2018 · 10 comments

Comments

@frenetisch-applaudierend

When I batch prefetch messages, the message view scrolls to the top instead of keeping the scroll position where the last message was. Is there a way to keep the scroll position at the last position?

I tried manually scrolling back, but it feels brittle and results in weird scrolling behaviour as well.

Any hint what I can change is appreciated.

Here's what my very basic controller looks like:

import UIKit
import NMessenger


class ViewController: UIViewController, NMessengerDelegate {

    private let messenger: NMessenger = NMessenger()


    // MARK: - View Lifecycle

    override func loadView() {
        self.view = self.messenger
    }

    override func viewDidLoad() {
        super.viewDidLoad()

        self.messenger.delegate = self
        self.messenger.doesBatchFetch = true
        self.messenger.addMessages(self.createNextBatch(), scrollsToMessage: true)
    }


    // MARK: - NMessengerDelegate

    func batchFetchContent() {
        DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(1)) {
            let messageCells = self.createNextBatch()

            self.endBatchFetchDirectly(messageCells: messageCells)
            //self.endBatchFetchAdjusted(messageCells: messageCells)
        }
    }

    private func endBatchFetchDirectly(messageCells: [MessageNode]) {
        self.messenger.endBatchFetchWithMessages(messageCells)
    }

    private func endBatchFetchAdjusted(messageCells: [MessageNode]) {
        let currentHeight = self.messenger.messengerNode.view.contentSize.height
        self.messenger.endBatchFetchWithMessages(messageCells)
        self.messenger.addMessagesWithBlock([], scrollsToMessage: false, withAnimation: .none) {
            DispatchQueue.main.async {
                let addedHeight = self.messenger.messengerNode.view.contentSize.height - currentHeight
                self.messenger.messengerNode.view.contentOffset.y = addedHeight
            }
        }
    }


    // MARK: - Generating Data

    private var currentBatchCounter: Int = 0

    private func createNextBatch() -> [MessageNode] {
        let size = 20
        let startIndex = self.currentBatchCounter
        let endIndex = self.currentBatchCounter + size

        self.currentBatchCounter = endIndex

        return (startIndex ..< endIndex).reversed().map { index in
            let cell = MessageNode(content: TextContentNode(textMessageString: "\(index)\nHello"))
            cell.currentViewController = self
            return cell
        }
    }
}
@dodikk
Copy link

dodikk commented Jan 31, 2018

@frenetisch-applaudierend , not sure if I'm getting your issue right...
But endBatchFetchDirectly ==> endBatchFetchWithMessages is supposed to not perform any programmatic scrolling according to this line of code.

https://github.com/eBay/NMessenger/blob/master/nMessenger/Source/Messenger/Components/NMessenger.swift#L203

@frenetisch-applaudierend
Copy link
Author

Hi @dodikk thanks for your reply.

The problem I believe is that this message does nothing to adjust the scroll position when new content is added. I assume what happens is that the content is added to the top which moves everything down, but there is no adjustment of the contentOffset to account for this.

So what I see now when I have e.g. batches of 20 cells is that cell 20 is displayed and above the spinner to load new messages. When those are loaded the top visible cell is now 40.

What I would like to achieve instead is that after the new cells are loaded that the top visible cell is still 20 or at least close to it.

Does that make more sense?

@dodikk
Copy link

dodikk commented Feb 1, 2018

Then the best way to go is

  1. Add the batchFetchDidEnd delegate metod to NMessengerDelegate
  2. Invoke it right here https://github.com/eBay/NMessenger/blob/master/nMessenger/Source/Messenger/Components/NMessenger.swift#L204
  3. Make the desired adjustments in the callback handler.

P.S. Unfortunately, this involves forking NMessenger and including it to your codebase. And yeah, you are going to be on your own since eBay seems to have abandoned this project. They say "we do care" but in fact PRs are not getting approved for months (for mine it's almost a year).

@dodikk
Copy link

dodikk commented Feb 1, 2018

I tried manually scrolling back, but it feels brittle and results in weird scrolling behaviour as well.

Have you tried changing your dispatch_async hotfix to dispatch_after with some reasonable timeout?

@frenetisch-applaudierend
Copy link
Author

I will try then first with a higher timeout to allow for the layout pass to be finished (this was why I added the "superfluous" self.messenger.addMessagesWithBlock call). If all else fails, a fork it is then...

And yeah, you are going to be on your own since eBay seems to have abandoned this project.

I noticed this :-( Thanks for keeping at it though! 👍

@munsifhayat
Copy link

@dodikk I am stuck with batch fetch content . Can you please look into #199

@dodikk
Copy link

dodikk commented Apr 16, 2018

@dodikk I am stuck with batch fetch content . Can you please look into #199

@munsifhayat , sorry but I'm short of time at the moment.
P.S. A kind reminder that I am neither a maintainer of NMessenger, nor do I work for eBay.

@munsifhayat
Copy link

@frenetisch-applaudierend any solution so far ?

@frenetisch-applaudierend
Copy link
Author

@munsifhayat Unfortunately no. I kind of gave up on this, frankly.

@munsifhayat
Copy link

My issue is of same kind. Batch fetch content isn’t calling. When does UI is so ambiguous. Huge waste of time :(

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

No branches or pull requests

3 participants