Skip to content

Commit

Permalink
Improve performance of presenting ChatChannelVC (#3448)
Browse files Browse the repository at this point in the history
  • Loading branch information
laevandus authored Oct 10, 2024
1 parent 69cd64f commit 67d3465
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 23 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### 🐞 Fixed
- Unread messages divider did not appear in the message list when marking messages as unread [#3444](https://github.com/GetStream/stream-chat-swift/pull/3444)
- Fix UI glitch in thread parent message when sending a message and scrolling [#3446](https://github.com/GetStream/stream-chat-swift/pull/3446)
### ⚡ Performance
- Improve performance of presenting `ChatChannelVC` [#3448](https://github.com/GetStream/stream-chat-swift/pull/3448)

# [4.64.0](https://github.com/GetStream/stream-chat-swift/releases/tag/4.64.0)
_October 02, 2024_
Expand Down
1 change: 1 addition & 0 deletions Scripts/updateDependency.sh
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ fi

if [[ $dependency_directory == *"SwiftyMarkdown"* ]]; then
# We currently use customized version of SwiftyMarkdown
git restore $output_directory/SwiftyMarkdown/PerformanceLog.swift || true
git restore $output_directory/SwiftyMarkdown/SwiftyLineProcessor.swift || true
git restore $output_directory/SwiftyMarkdown/SwiftyTokeniser.swift || true
fi
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ open class ChatMessageContentView: _View, ThemeProvider, UITextViewDelegate {
open var editedLabelSeparator: String {
""
}

/// Skip content update when tintColorDidChange is called but content was already updated for that color
/// Unnecessary updates can get expensive due to text updates.
private var previousContentUpdateTintColor: UIColor?

// MARK: - Content views

Expand Down Expand Up @@ -590,6 +594,7 @@ open class ChatMessageContentView: _View, ThemeProvider, UITextViewDelegate {
super.updateContent()
defer {
attachmentViewInjector?.contentViewDidUpdateContent()
previousContentUpdateTintColor = tintColor
setNeedsLayout()
}

Expand Down Expand Up @@ -747,7 +752,7 @@ open class ChatMessageContentView: _View, ThemeProvider, UITextViewDelegate {

override open func tintColorDidChange() {
super.tintColorDidChange()

guard previousContentUpdateTintColor != tintColor else { return }
guard UIApplication.shared.applicationState == .active else { return }
// We need to update the content and manually apply the updated `tintColor`
// to the subviews which don't listen for `tintColor` updates.
Expand Down
37 changes: 17 additions & 20 deletions Sources/StreamChatUI/ChatMessageList/ChatMessageListVC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,6 @@ open class ChatMessageListVC: _ViewController,

override open func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) {
super.traitCollectionDidChange(previousTraitCollection)

view.layoutIfNeeded()
listView.adjustContentInsetToPositionMessagesAtTheTop()
}
Expand Down Expand Up @@ -1304,10 +1303,14 @@ private extension ChatMessageListVC {
}

UIView.performWithoutAnimation {
self?.scrollToBottomIfNeeded(with: changes, newestChange: newestChange)
self?.reloadMovedMessage(newestChange: newestChange)
self?.reloadPreviousMessagesForVisibleRemoves(with: changes)
self?.reloadPreviousMessageWhenInsertingNewMessage()
guard let self else { return }
self.scrollToBottomIfNeeded(with: changes, newestChange: newestChange)
var additionalChanges = Set<IndexPath>()
additionalChanges.formUnion(self.reloadMovedMessage(newestChange: newestChange))
additionalChanges.formUnion(self.reloadPreviousMessagesForVisibleRemoves(with: changes))
additionalChanges.formUnion(self.reloadPreviousMessageWhenInsertingNewMessage(with: changes))
guard !additionalChanges.isEmpty else { return }
self.listView.reloadRows(at: additionalChanges.sorted(), with: .none)
}

self?.scrollPendingMessageIfNeeded()
Expand Down Expand Up @@ -1356,24 +1359,21 @@ private extension ChatMessageListVC {

// If we are inserting messages at the bottom, update the previous cell
// to hide the timestamp of the previous message if needed.
func reloadPreviousMessageWhenInsertingNewMessage() {
guard isFirstPageLoaded else { return }
if listView.isLastCellFullyVisible && listView.newMessagesSnapshot.count > 1 {
let previousMessageIndexPath = IndexPath(item: 1, section: 0)
listView.reloadRows(at: [previousMessageIndexPath], with: .none)
}
private func reloadPreviousMessageWhenInsertingNewMessage(with changes: [ListChange<ChatMessage>]) -> [IndexPath] {
guard isFirstPageLoaded else { return [] }
guard listView.isLastCellFullyVisible && listView.newMessagesSnapshot.count > 1 else { return [] }
guard !changes.contains(where: { $0.indexPath.item == 1 && $0.isInsertion }) else { return [] }
return [IndexPath(item: 1, section: 0)]
}

// When there are deletions, we should update the previous message, so that we add the
// avatar image is rendered back and the timestamp too. Since we have an inverted list, the previous
// message has the same index of the deleted message after the deletion has been executed.
func reloadPreviousMessagesForVisibleRemoves(with changes: [ListChange<ChatMessage>]) {
private func reloadPreviousMessagesForVisibleRemoves(with changes: [ListChange<ChatMessage>]) -> [IndexPath] {
let visibleRemoves = changes.filter {
$0.isRemove && isMessageVisible(at: $0.indexPath)
}
visibleRemoves.forEach {
listView.reloadRows(at: [$0.indexPath], with: .none)
}
return visibleRemoves.map(\.indexPath)
}

// Scroll to the bottom if the new message was sent by
Expand All @@ -1391,10 +1391,7 @@ private extension ChatMessageListVC {

// When a Giphy moves to the bottom, we need to also trigger a reload
// Since a move doesn't trigger a reload of the cell.
func reloadMovedMessage(newestChange: ListChange<ChatMessage>?) {
if newestChange?.isMove == true {
let movedIndexPath = IndexPath(item: 0, section: 0)
listView.reloadRows(at: [movedIndexPath], with: .none)
}
private func reloadMovedMessage(newestChange: ListChange<ChatMessage>?) -> [IndexPath] {
newestChange?.isMove == true ? [IndexPath(item: 0, section: 0)] : []
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class PerformanceLog {

init( with environmentVariableName : String, identifier : String, log : OSLog ) {
self.log = log
self.enablePerfomanceLog = (ProcessInfo.processInfo.environment[environmentVariableName] != nil)
self.enablePerfomanceLog = false
self.identifier = identifier
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,10 @@ final class ChatChannelVC_Tests: XCTestCase {

vc.components.isMessageEditedLabelEnabled = true

// Also update the default because in snapshot tests, message cells are created before they are in the responder chain
defer { Components.default.isMessageEditedLabelEnabled = false }
Components.default.isMessageEditedLabelEnabled = true

AssertSnapshot(vc, variants: [.defaultLight])
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ final class ChatMessageListVC_Tests: XCTestCase {
])
mockedListView.updateMessagesCompletion?()

XCTAssertEqual(mockedListView.reloadRowsCallCount, 2)
XCTAssertEqual(mockedListView.reloadRowsCallCount, 1)
XCTAssertEqual(mockedListView.reloadRowsCalledWith, [.init(item: 0, section: 0), .init(item: 1, section: 0)])
}

Expand Down

0 comments on commit 67d3465

Please sign in to comment.