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

feat: Implement "scroll-back" to latest message when navigating replies #877

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Session/Conversations/ConversationVC+Interaction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,8 @@ extension ConversationVC:
}

self.scrollToInteractionIfNeeded(with: interactionInfo, focusBehaviour: .highlight)
let originalMessageTimestamp = Interaction.TimestampInfo(id: cellViewModel.id, timestampMs: cellViewModel.timestampMs)
self.replyNavigationStack.add(originalMessageTimestamp)
}
else if let linkPreview: LinkPreview = cellViewModel.linkPreview {
switch linkPreview.variant {
Expand Down
120 changes: 85 additions & 35 deletions Session/Conversations/ConversationVC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
var didFinishInitialLayout = false
var scrollDistanceToBottomBeforeUpdate: CGFloat?
var baselineKeyboardHeight: CGFloat = 0
var replyNavigationStack = ConversationReplyNavigationStack()

/// These flags are true between `viewDid/Will Appear/Disappear` and is used to prevent keyboard changes
/// from trying to animate (as the animations can cause buggy transitions)
Expand Down Expand Up @@ -256,13 +257,17 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
image: UIImage(named: "ic_chevron_down")?
.withRenderingMode(.alwaysTemplate)
) { [weak self] in
// The table view's content size is calculated by the estimated height of cells,
// so the result may be inaccurate before all the cells are loaded. Use this
// to scroll to the last row instead.
self?.scrollToBottom(isAnimated: true)
if let previousPosition = self?.replyNavigationStack.removeLast() {
self?.scrollToInteractionIfNeeded(with: previousPosition, focusBehaviour: .highlight)
} else {
// The table view's content size is calculated by the estimated height of cells,
// so the result may be inaccurate before all the cells are loaded. Use this
// to scroll to the last row instead.
self?.scrollToBottom(isAnimated: true)
}
}
result.alpha = 0

return result
}()

Expand Down Expand Up @@ -1640,6 +1645,13 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers

func scrollViewDidEndDragging(_ scrollView: UIScrollView, willDecelerate decelerate: Bool) {
isUserScrolling = false
if !decelerate {
self.onScrollFinished()
}
}

func scrollViewDidEndDecelerating(_ scrollView: UIScrollView) {
self.onScrollFinished()
}

func scrollViewDidScroll(_ scrollView: UIScrollView) {
Expand Down Expand Up @@ -1669,6 +1681,16 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
self?.markFullyVisibleAndOlderCellsAsRead(interactionInfo: focusedInteractionInfo)
self?.highlightCellIfNeeded(interactionId: focusedInteractionInfo.id, behaviour: behaviour)
}

self.onScrollFinished()
}

func onScrollFinished() {
DispatchQueue.main.async { [weak self] in
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we perhaps want to add the older code from scrollViewDidEndDragging and scrollViewDidEndScrollingAnimation to onScrollFinished as well? I'm not entirely sure they need to be fired every time scrolling is complete, though :)

if let newestMessageViewModel: MessageViewModel = self?.getNewestMessageViewModel() {
self?.replyNavigationStack.removeTimestampsOlder(than: newestMessageViewModel.timestampMs)
}
}
}

func updateUnreadCountView(unreadCount: UInt?) {
Expand Down Expand Up @@ -1935,36 +1957,9 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
}

func markFullyVisibleAndOlderCellsAsRead(interactionInfo: Interaction.TimestampInfo?) {
// We want to mark messages as read on load and while we scroll, so grab the newest message and mark
// everything older as read
//
// Note: For the 'tableVisualBottom' we remove the 'Values.mediumSpacing' as that is the distance
// the table content appears above the input view
let tableVisualBottom: CGFloat = (tableView.frame.maxY - (tableView.contentInset.bottom - Values.mediumSpacing))

guard
let visibleIndexPaths: [IndexPath] = self.tableView.indexPathsForVisibleRows,
let messagesSection: Int = visibleIndexPaths
.first(where: { self.viewModel.interactionData[$0.section].model == .messages })?
.section,
let newestCellViewModel: MessageViewModel = visibleIndexPaths
.sorted()
.filter({ $0.section == messagesSection })
.compactMap({ indexPath -> (frame: CGRect, cellViewModel: MessageViewModel)? in
guard let cell: VisibleMessageCell = tableView.cellForRow(at: indexPath) as? VisibleMessageCell else {
return nil
}

return (
view.convert(cell.frame, from: tableView),
self.viewModel.interactionData[indexPath.section].elements[indexPath.row]
)
})
// Exclude messages that are partially off the bottom of the screen
.filter({ $0.frame.maxY <= tableVisualBottom })
.last?
.cellViewModel
else {
// We want to mark messages as read on load and while we scroll,
// so grab the newest message and mark everything older as read
guard let newestCellViewModel = getNewestMessageViewModel() else {
// If we weren't able to get any visible cells for some reason then we should fall back to
// marking the provided interactionInfo as read just in case
if let interactionInfo: Interaction.TimestampInfo = interactionInfo {
Expand Down Expand Up @@ -2001,9 +1996,64 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
}
}


func getNewestMessageViewModel() -> MessageViewModel? {
// Note: For the 'tableVisualBottom' we remove the 'Values.mediumSpacing' as that is the distance
// the table content appears above the input view
let tableVisualBottom: CGFloat = (tableView.frame.maxY - (tableView.contentInset.bottom - Values.mediumSpacing))

let visibleIndexPaths: [IndexPath]? = self.tableView.indexPathsForVisibleRows
let messagesSection: Int? = visibleIndexPaths?.first(where: { self.viewModel.interactionData[$0.section].model == .messages })?.section
let newestCellViewModel: MessageViewModel? = visibleIndexPaths?
.sorted()
.filter { $0.section == messagesSection }
.compactMap { indexPath -> (frame: CGRect, cellViewModel: MessageViewModel)? in
guard let cell: VisibleMessageCell = tableView.cellForRow(at: indexPath) as? VisibleMessageCell else {
return nil
}

return (
view.convert(cell.frame, from: tableView),
self.viewModel.interactionData[indexPath.section].elements[indexPath.row]
)
}
// Exclude messages that are partially off the bottom of the screen
.filter { $0.frame.maxY <= tableVisualBottom }
.last?
.cellViewModel

return newestCellViewModel
}

// MARK: - SessionUtilRespondingViewController

func isConversation(in threadIds: [String]) -> Bool {
return threadIds.contains(self.viewModel.threadData.threadId)
}
}

struct ConversationReplyNavigationStack {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a better place to add this, but let me know if there is one, and I'll move it :)

private var messageTimestamps: [Interaction.TimestampInfo] = []

mutating func add(_ timestamp: Interaction.TimestampInfo) {
if !messageTimestamps.contains(where: { $0.id == timestamp.id }) {
self.messageTimestamps.append(timestamp)
}
}

mutating func removeLast() -> Interaction.TimestampInfo? {
if isEmpty { return nil }

return messageTimestamps.removeLast()
}

var isEmpty: Bool { messageTimestamps.isEmpty }

mutating func removeTimestampsOlder(than timestamp: Int64) {
for i in (0 ..< self.messageTimestamps.count).reversed() {
if self.messageTimestamps[i].timestampMs <= timestamp {
self.messageTimestamps.remove(at: i)
}
}
}
}