Fixed a few issues around jumping between messages

Fixed a bug where tapping on quotes could break after loading lots of visible cells
Fixed a bug where a cell which contained both links and a quote would only ever let you interact with the quote
Fixed an issue where message highlight wasn't working when jumping between messages
pull/946/head
Morgan Pretty 1 year ago
parent e1d6a9dfc1
commit 3f19c776d9

@ -847,7 +847,8 @@ extension ConversationVC:
func handleItemTapped(
_ cellViewModel: MessageViewModel,
gestureRecognizer: UITapGestureRecognizer,
cell: UITableViewCell,
cellLocation: CGPoint,
using dependencies: Dependencies = Dependencies()
) {
guard cellViewModel.variant != .standardOutgoing || (cellViewModel.state != .failed && cellViewModel.state != .failedToSync) else {
@ -900,20 +901,10 @@ extension ConversationVC:
case .voiceMessage: viewModel.playOrPauseAudio(for: cellViewModel)
case .mediaMessage:
guard
let sectionIndex: Int = self.viewModel.interactionData
.firstIndex(where: { $0.model == .messages }),
let messageIndex: Int = self.viewModel.interactionData[sectionIndex]
.elements
.firstIndex(where: { $0.id == cellViewModel.id }),
let cell = tableView.cellForRow(at: IndexPath(row: messageIndex, section: sectionIndex)) as? VisibleMessageCell,
let albumView: MediaAlbumView = cell.albumView
else { return }
let locationInCell: CGPoint = gestureRecognizer.location(in: cell)
guard let albumView: MediaAlbumView = (cell as? VisibleMessageCell)?.albumView else { return }
// Figure out which of the media views was tapped
let locationInAlbumView: CGPoint = cell.convert(locationInCell, to: albumView)
let locationInAlbumView: CGPoint = cell.convert(cellLocation, to: albumView)
guard let mediaView = albumView.mediaView(forLocation: locationInAlbumView) else { return }
switch mediaView.attachment.state {
@ -1034,26 +1025,51 @@ extension ConversationVC:
navigationController?.present(shareVC, animated: true, completion: nil)
case .textOnlyMessage:
if let quote: Quote = cellViewModel.quote {
// Scroll to the original quoted message
let maybeOriginalInteractionInfo: Interaction.TimestampInfo? = Storage.shared.read { db in
try quote.originalInteraction
.select(.id, .timestampMs)
.asRequest(of: Interaction.TimestampInfo.self)
.fetchOne(db)
}
guard let visibleCell: VisibleMessageCell = cell as? VisibleMessageCell else { return }
let quotePoint: CGPoint = visibleCell.convert(cellLocation, to: visibleCell.quoteView)
let linkPreviewPoint: CGPoint = visibleCell.convert(cellLocation, to: visibleCell.linkPreviewView?.previewView)
let tappableLabelPoint: CGPoint = visibleCell.convert(cellLocation, to: visibleCell.bodyTappableLabel)
let containsLinks: Bool = (
// If there is only a single link and it matches the LinkPreview then consider this _just_ a
// LinkPreview
visibleCell.bodyTappableLabel?.containsLinks == true && (
(visibleCell.bodyTappableLabel?.links.count ?? 0) > 1 ||
visibleCell.bodyTappableLabel?.links[cellViewModel.linkPreview?.url ?? ""] == nil
)
)
let quoteViewContainsTouch: Bool = (visibleCell.quoteView?.bounds.contains(quotePoint) == true)
let linkPreviewViewContainsTouch: Bool = (visibleCell.linkPreviewView?.previewView.bounds.contains(linkPreviewPoint) == true)
switch (containsLinks, quoteViewContainsTouch, linkPreviewViewContainsTouch, cellViewModel.quote, cellViewModel.linkPreview) {
// If the message contains both links and a quote, and the user tapped on the quote; OR the
// message only contained a quote, then scroll to the quote
case (true, true, _, .some(let quote), _), (false, _, _, .some(let quote), _):
let maybeOriginalInteractionInfo: Interaction.TimestampInfo? = Storage.shared.read { db in
try quote.originalInteraction
.select(.id, .timestampMs)
.asRequest(of: Interaction.TimestampInfo.self)
.fetchOne(db)
}
guard let interactionInfo: Interaction.TimestampInfo = maybeOriginalInteractionInfo else {
return
}
self.scrollToInteractionIfNeeded(with: interactionInfo, focusBehaviour: .highlight)
guard let interactionInfo: Interaction.TimestampInfo = maybeOriginalInteractionInfo else {
return
}
// If the message contains both links and a LinkPreview, and the user tapped on
// the LinkPreview; OR the message only contained a LinkPreview, then open the link
case (true, _, true, _, .some(let linkPreview)), (false, _, _, _, .some(let linkPreview)):
switch linkPreview.variant {
case .standard: openUrl(linkPreview.url)
case .openGroupInvitation: joinOpenGroup(name: linkPreview.title, url: linkPreview.url)
}
self.scrollToInteractionIfNeeded(with: interactionInfo, focusBehaviour: .highlight)
}
else if let linkPreview: LinkPreview = cellViewModel.linkPreview {
switch linkPreview.variant {
case .standard: openUrl(linkPreview.url)
case .openGroupInvitation: joinOpenGroup(name: linkPreview.title, url: linkPreview.url)
}
// If the message contained links then interact with them directly
case (true, _, _, _, _): visibleCell.bodyTappableLabel?.handleTouch(at: tappableLabelPoint)
default: break
}
default: break

@ -29,7 +29,6 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
var focusedInteractionInfo: Interaction.TimestampInfo?
var focusBehaviour: ConversationViewModel.FocusBehaviour = .none
var shouldHighlightNextScrollToInteraction: Bool = false
// Search
var isShowingSearchUI = false
@ -1058,7 +1057,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
self?.searchController.resultsBar.stopLoading()
self?.scrollToInteractionIfNeeded(
with: focusedInteractionInfo,
focusBehaviour: (self?.shouldHighlightNextScrollToInteraction == true ? .highlight : .none),
focusBehaviour: (self?.focusBehaviour ?? .none),
isAnimated: true
)
@ -1135,7 +1134,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
self?.searchController.resultsBar.stopLoading()
self?.scrollToInteractionIfNeeded(
with: focusedInteractionInfo,
focusBehaviour: (self?.shouldHighlightNextScrollToInteraction == true ? .highlight : .none),
focusBehaviour: (self?.focusBehaviour ?? .none),
isAnimated: true
)
}
@ -1155,7 +1154,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
self?.searchController.resultsBar.stopLoading()
self?.scrollToInteractionIfNeeded(
with: focusedInteractionInfo,
focusBehaviour: (self?.shouldHighlightNextScrollToInteraction == true ? .highlight : .none),
focusBehaviour: (self?.focusBehaviour ?? .none),
isAnimated: true
)
@ -1660,17 +1659,15 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
}
func scrollViewDidEndScrollingAnimation(_ scrollView: UIScrollView) {
guard
let focusedInteractionInfo: Interaction.TimestampInfo = self.focusedInteractionInfo,
self.shouldHighlightNextScrollToInteraction
else {
guard let focusedInteractionInfo: Interaction.TimestampInfo = self.focusedInteractionInfo else {
self.focusedInteractionInfo = nil
self.focusBehaviour = .none
self.shouldHighlightNextScrollToInteraction = false
return
}
let behaviour: ConversationViewModel.FocusBehaviour = self.focusBehaviour
self.focusedInteractionInfo = nil
self.focusBehaviour = .none
DispatchQueue.main.async { [weak self] in
self?.markFullyVisibleAndOlderCellsAsRead(interactionInfo: focusedInteractionInfo)
@ -1832,8 +1829,8 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
isAnimated: Bool = true
) {
// Store the info incase we need to load more data (call will be re-triggered)
self.focusBehaviour = focusBehaviour
self.focusedInteractionInfo = interactionInfo
self.shouldHighlightNextScrollToInteraction = (focusBehaviour == .highlight)
// Ensure the target interaction has been loaded
guard
@ -1920,7 +1917,6 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
self?.updateScrollToBottom(force: true)
}
self.shouldHighlightNextScrollToInteraction = false
self.focusedInteractionInfo = nil
self.focusBehaviour = .none
return
@ -1935,6 +1931,8 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
guard !self.tableView.bounds.contains(targetRect) else {
self.markFullyVisibleAndOlderCellsAsRead(interactionInfo: interactionInfo)
self.highlightCellIfNeeded(interactionId: interactionInfo.id, behaviour: focusBehaviour)
self.focusedInteractionInfo = nil
self.focusBehaviour = .none
return
}
@ -1999,7 +1997,6 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
}
func highlightCellIfNeeded(interactionId: Int64, behaviour: ConversationViewModel.FocusBehaviour) {
self.shouldHighlightNextScrollToInteraction = false
self.focusedInteractionInfo = nil
self.focusBehaviour = .none

@ -179,6 +179,6 @@ final class CallMessageCell: MessageCell {
// Should only be tappable if the info icon is visible
guard messageInfo.state == .permissionDenied && !Storage.shared[.areCallsEnabled] else { return }
self.delegate?.handleItemTapped(cellViewModel, gestureRecognizer: gestureRecognizer)
self.delegate?.handleItemTapped(cellViewModel, cell: self, cellLocation: gestureRecognizer.location(in: self))
}
}

@ -18,6 +18,8 @@ final class LinkPreviewView: UIView {
private lazy var imageViewContainerHeightConstraint = imageView.set(.height, to: 100)
// MARK: UI Components
public var previewView: UIView { hStackView }
private lazy var imageView: UIImageView = {
let result: UIImageView = UIImageView()

@ -88,7 +88,7 @@ public class MessageCell: UITableViewCell {
protocol MessageCellDelegate: ReactionDelegate {
func handleItemLongPressed(_ cellViewModel: MessageViewModel)
func handleItemTapped(_ cellViewModel: MessageViewModel, gestureRecognizer: UITapGestureRecognizer, using dependencies: Dependencies)
func handleItemTapped(_ cellViewModel: MessageViewModel, cell: UITableViewCell, cellLocation: CGPoint, using dependencies: Dependencies)
func handleItemDoubleTapped(_ cellViewModel: MessageViewModel)
func handleItemSwiped(_ cellViewModel: MessageViewModel, state: SwipeState)
func openUrl(_ urlString: String)
@ -99,7 +99,7 @@ protocol MessageCellDelegate: ReactionDelegate {
}
extension MessageCellDelegate {
func handleItemTapped(_ cellViewModel: MessageViewModel, gestureRecognizer: UITapGestureRecognizer) {
handleItemTapped(cellViewModel, gestureRecognizer: gestureRecognizer, using: Dependencies())
func handleItemTapped(_ cellViewModel: MessageViewModel, cell: UITableViewCell, cellLocation: CGPoint) {
handleItemTapped(cellViewModel, cell: cell, cellLocation: cellLocation, using: Dependencies())
}
}

@ -12,6 +12,8 @@ final class VisibleMessageCell: MessageCell, TappableLabelDelegate {
private var previousX: CGFloat = 0
var albumView: MediaAlbumView?
var quoteView: QuoteView?
var linkPreviewView: LinkPreviewView?
var bodyTappableLabel: TappableLabel?
var voiceMessageView: VoiceMessageView?
var audioStateChanged: ((TimeInterval, Bool) -> ())?
@ -467,6 +469,8 @@ final class VisibleMessageCell: MessageCell, TappableLabelDelegate {
subview.removeFromSuperview()
}
albumView = nil
quoteView = nil
linkPreviewView = nil
bodyTappableLabel = nil
// Handle the deleted state first (it's much simpler than the others)
@ -509,6 +513,7 @@ final class VisibleMessageCell: MessageCell, TappableLabelDelegate {
bodyLabelTextColor: bodyLabelTextColor,
lastSearchText: lastSearchText
)
self.linkPreviewView = linkPreviewView
bubbleView.addSubview(linkPreviewView)
linkPreviewView.pin(to: bubbleView, withInset: 0)
snContentView.addArrangedSubview(bubbleBackgroundView)
@ -551,6 +556,7 @@ final class VisibleMessageCell: MessageCell, TappableLabelDelegate {
hInset: hInset,
maxWidth: maxWidth
)
self.quoteView = quoteView
let quoteViewContainer = UIView(wrapping: quoteView, withInsets: UIEdgeInsets(top: 0, leading: hInset, bottom: 0, trailing: hInset))
stackView.addArrangedSubview(quoteViewContainer)
}
@ -768,22 +774,6 @@ final class VisibleMessageCell: MessageCell, TappableLabelDelegate {
}
// MARK: - Interaction
override func hitTest(_ point: CGPoint, with event: UIEvent?) -> UIView? {
// We are currently using Appium to do automated UI testing, unfortunately it seems to run into
// issues when trying to long-press an element which has custom interaction logic - the TappableLabel
// only needs to custom handle touches for interacting with links so we check to see if it contains
// links before forwarding touches to it
if let bodyTappableLabel: TappableLabel = bodyTappableLabel, bodyTappableLabel.containsLinks {
let bodyTappableLabelLocalTapCoordinate: CGPoint = convert(point, to: bodyTappableLabel)
if bodyTappableLabel.bounds.contains(bodyTappableLabelLocalTapCoordinate) {
return bodyTappableLabel
}
}
return super.hitTest(point, with: event)
}
override func gestureRecognizer(_ gestureRecognizer: UIGestureRecognizer, shouldRecognizeSimultaneouslyWith otherGestureRecognizer: UIGestureRecognizer) -> Bool {
return true // Needed for the pan gesture recognizer to work with the table view's pan gesture recognizer
@ -918,7 +908,7 @@ final class VisibleMessageCell: MessageCell, TappableLabelDelegate {
}
}
else if snContentView.bounds.contains(snContentView.convert(location, from: self)) {
delegate?.handleItemTapped(cellViewModel, gestureRecognizer: gestureRecognizer, using: dependencies)
delegate?.handleItemTapped(cellViewModel, cell: self, cellLocation: location, using: dependencies)
}
}

@ -12,7 +12,7 @@ public protocol TappableLabelDelegate: AnyObject {
}
public class TappableLabel: UILabel {
private var links: [String: NSRange] = [:]
public private(set) var links: [String: NSRange] = [:]
private lazy var highlightedMentionBackgroundView: HighlightMentionBackgroundView = HighlightMentionBackgroundView(targetLabel: self)
private(set) var layoutManager = NSLayoutManager()
private(set) var textContainer = NSTextContainer(size: CGSize.zero)
@ -125,9 +125,13 @@ public class TappableLabel: UILabel {
return
}
handleTouch(at: locationOfTouch)
}
public func handleTouch(at point: CGPoint) {
textContainer.size = bounds.size
let indexOfCharacter = layoutManager.glyphIndex(for: locationOfTouch, in: textContainer)
let indexOfCharacter = layoutManager.glyphIndex(for: point, in: textContainer)
for (urlString, range) in links where NSLocationInRange(indexOfCharacter, range) {
delegate?.tapableLabel(self, didTapUrl: urlString, atRange: range)
return

Loading…
Cancel
Save