From eacb8a4653f48add5f5b8ab5c07a832f0fc83c23 Mon Sep 17 00:00:00 2001 From: Doug Date: Wed, 21 Jul 2021 15:14:25 +0100 Subject: [PATCH 01/28] Begin adding link detection to RoomBubbleCellData. --- Riot/Categories/MXEvent.swift | 34 +++++++++++++ Riot/Categories/NSString.swift | 49 +++++++++++++++++++ .../Managers/URLPreviews/PreviewManager.swift | 45 +++++++++++++++++ .../Room/CellData/RoomBubbleCellData.h | 7 ++- .../Room/CellData/RoomBubbleCellData.m | 46 +++++++++++++++++ 5 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 Riot/Categories/MXEvent.swift create mode 100644 Riot/Categories/NSString.swift create mode 100644 Riot/Managers/URLPreviews/PreviewManager.swift diff --git a/Riot/Categories/MXEvent.swift b/Riot/Categories/MXEvent.swift new file mode 100644 index 0000000000..8c3a42dcb5 --- /dev/null +++ b/Riot/Categories/MXEvent.swift @@ -0,0 +1,34 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +extension MXEvent { + /// Gets the first URL contained in a text message event's body. + /// - Returns: A URL if detected, otherwise nil. + @objc func vc_firstURLInBody() -> NSURL? { + guard + type == kMXEventTypeStringRoomMessage, + content["msgtype"] as? String == kMXMessageTypeText, + let textMessage = content["body"] as? String, + let url = textMessage.vc_firstURLDetected() + else { + return nil + } + + return url + } +} diff --git a/Riot/Categories/NSString.swift b/Riot/Categories/NSString.swift new file mode 100644 index 0000000000..e593c1a263 --- /dev/null +++ b/Riot/Categories/NSString.swift @@ -0,0 +1,49 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +extension NSString { + /// Check if the string contains a URL. + /// - Returns: True if the string contains at least one URL, otherwise false. + @objc func vc_containsURL() -> Bool { + guard let linkDetector = try? NSDataDetector(types: NSTextCheckingResult.CheckingType.link.rawValue) else { + MXLog.debug("[NSString+URLDetector]: Unable to create link detector.") + return false + } + + // return true if there is at least one match + return linkDetector.numberOfMatches(in: self as String, options: [], range: NSRange(location: 0, length: self.length)) > 0 + } + + /// Gets the first URL contained in the string. + /// - Returns: A URL if detected, otherwise nil. + @objc func vc_firstURLDetected() -> NSURL? { + guard let linkDetector = try? NSDataDetector(types: NSTextCheckingResult.CheckingType.link.rawValue) else { + MXLog.debug("[NSString+URLDetector]: Unable to create link detector.") + return nil + } + + // find the first match, otherwise return nil + guard let match = linkDetector.firstMatch(in: self as String, options: [], range: NSRange(location: 0, length: self.length)) else { + return nil + } + + // create a url and return it. + let urlString = self.substring(with: match.range) + return NSURL(string: urlString) + } +} diff --git a/Riot/Managers/URLPreviews/PreviewManager.swift b/Riot/Managers/URLPreviews/PreviewManager.swift new file mode 100644 index 0000000000..fefc6ef991 --- /dev/null +++ b/Riot/Managers/URLPreviews/PreviewManager.swift @@ -0,0 +1,45 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +@objcMembers +class PreviewManager: NSObject { + let restClient: MXRestClient + + // temporary in memory cache for now + let cache = NSCache() + + init(restClient: MXRestClient) { + self.restClient = restClient + cache.countLimit = 20 + } + + func preview(for url: URL, success: @escaping (MXURLPreview?) -> Void, failure: @escaping (Error?) -> Void) { + if let preview = cache.object(forKey: url as NSURL) { + success(preview) + return + } + + restClient.preview(for: url, success: { preview in + if let preview = preview { + self.cache.setObject(preview, forKey: url as NSURL) + } + + success(preview) + }, failure: failure) + } +} diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.h b/Riot/Modules/Room/CellData/RoomBubbleCellData.h index af8c190900..f77d99adc1 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.h +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.h @@ -79,7 +79,12 @@ typedef NS_ENUM(NSInteger, RoomBubbleCellDataTag) @property(nonatomic, readonly) CGFloat additionalContentHeight; /** - MXKeyVerification object associated to key verifcation event when using key verification by direct message. + A link if the textMessage contains one, otherwise nil. + */ +@property (nonatomic) NSURL *link; + +/** + MXKeyVerification object associated to key verification event when using key verification by direct message. */ @property(nonatomic, strong) MXKeyVerification *keyVerification; diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index 423b84426c..98943172ab 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -132,6 +132,12 @@ - (instancetype)initWithEvent:(MXEvent *)event andRoomState:(MXRoomState *)roomS self.collapsed = YES; } break; + case MXEventTypeRoomMessage: + { + // If the message contains a URL, store it in the cell data. + self.link = [event vc_firstURLInBody]; + } + break; case MXEventTypeCallInvite: case MXEventTypeCallAnswer: case MXEventTypeCallHangup: @@ -788,6 +794,41 @@ - (BOOL)addEvent:(MXEvent*)event andRoomState:(MXRoomState*)roomState if ([messageType isEqualToString:kMXMessageTypeKeyVerificationRequest]) { shouldAddEvent = NO; + break; + } + + NSDate *eventDate = [NSDate dateWithTimeIntervalSince1970:(double)event.originServerTs/1000]; + + if (self.mostRecentComponentIndex != NSNotFound) + { + MXKRoomBubbleComponent *lastComponent = self.bubbleComponents[self.mostRecentComponentIndex]; + // If the new event comes after the last bubble component + if ([lastComponent.date earlierDate:eventDate] == lastComponent.date) + { + // FIXME: This should be for all event types, not just messages. + // Don't add it if there is already a link in the cell data + if (self.link) + { + shouldAddEvent = NO; + } + break; + } + } + + if (self.oldestComponentIndex != NSNotFound) + { + MXKRoomBubbleComponent *firstComponent = self.bubbleComponents[self.oldestComponentIndex]; + // If the new event event comes before the first bubble component + if ([firstComponent.date laterDate:eventDate] == firstComponent.date) + { + // Don't add it to the cell data if it contains a link + NSString *messageBody = event.content[@"body"]; + if (messageBody && [messageBody vc_containsURL]) + { + shouldAddEvent = NO; + } + break; + } } } break; @@ -844,6 +885,11 @@ - (BOOL)addEvent:(MXEvent*)event andRoomState:(MXRoomState*)roomState shouldAddEvent = [super addEvent:event andRoomState:roomState]; } + if (shouldAddEvent) + { + self.link = [event vc_firstURLInBody]; + } + return shouldAddEvent; } From 29758d1aa7ffcd2ffdf2aab2942baea13e9099a2 Mon Sep 17 00:00:00 2001 From: Doug Date: Mon, 23 Aug 2021 17:56:24 +0100 Subject: [PATCH 02/28] Add PreviewManger with Core Data cache and a URLPreviewView with a view model. Changes to RoomDataSource still to come. --- Riot/Categories/MXEvent.swift | 26 +++- .../Managers/URLPreviews/PreviewManager.swift | 72 +++++++-- .../URLPreviews/URLPreviewCache.swift | 138 +++++++++++++++++ .../URLPreviewCache.xcdatamodel/contents | 14 ++ .../URLPreviews/URLPreviewCacheData.swift | 43 ++++++ .../URLPreviewImageTransformer.swift | 45 ++++++ Riot/Modules/Application/LegacyAppDelegate.h | 4 + Riot/Modules/Application/LegacyAppDelegate.m | 7 + .../Room/CellData/RoomBubbleCellData.m | 12 +- .../Encryption/RoomBubbleCellLayout.swift | 6 + .../Views/URLPreviews/URLPreviewView.swift | 145 ++++++++++++++++++ .../Room/Views/URLPreviews/URLPreviewView.xib | 118 ++++++++++++++ .../URLPreviews/URLPreviewViewAction.swift | 24 +++ .../URLPreviews/URLPreviewViewData.swift | 42 +++++ .../URLPreviews/URLPreviewViewModel.swift | 89 +++++++++++ .../URLPreviews/URLPreviewViewModelType.swift | 29 ++++ .../URLPreviews/URLPreviewViewState.swift | 25 +++ RiotTests/URLPreviewCacheTests.swift | 132 ++++++++++++++++ 18 files changed, 952 insertions(+), 19 deletions(-) create mode 100644 Riot/Managers/URLPreviews/URLPreviewCache.swift create mode 100644 Riot/Managers/URLPreviews/URLPreviewCache.xcdatamodeld/URLPreviewCache.xcdatamodel/contents create mode 100644 Riot/Managers/URLPreviews/URLPreviewCacheData.swift create mode 100644 Riot/Managers/URLPreviews/URLPreviewImageTransformer.swift create mode 100644 Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift create mode 100644 Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib create mode 100644 Riot/Modules/Room/Views/URLPreviews/URLPreviewViewAction.swift create mode 100644 Riot/Modules/Room/Views/URLPreviews/URLPreviewViewData.swift create mode 100644 Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift create mode 100644 Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModelType.swift create mode 100644 Riot/Modules/Room/Views/URLPreviews/URLPreviewViewState.swift create mode 100644 RiotTests/URLPreviewCacheTests.swift diff --git a/Riot/Categories/MXEvent.swift b/Riot/Categories/MXEvent.swift index 8c3a42dcb5..eb6f9b440d 100644 --- a/Riot/Categories/MXEvent.swift +++ b/Riot/Categories/MXEvent.swift @@ -20,15 +20,27 @@ extension MXEvent { /// Gets the first URL contained in a text message event's body. /// - Returns: A URL if detected, otherwise nil. @objc func vc_firstURLInBody() -> NSURL? { - guard - type == kMXEventTypeStringRoomMessage, - content["msgtype"] as? String == kMXMessageTypeText, - let textMessage = content["body"] as? String, - let url = textMessage.vc_firstURLDetected() - else { - return nil + // Only get URLs for un-redacted message events that are text, notice or emote. + guard isRedactedEvent() == false, + eventType == .roomMessage, + let messageType = content["msgtype"] as? String, + messageType == kMXMessageTypeText || messageType == kMXMessageTypeNotice || messageType == kMXMessageTypeEmote + else { return nil } + + // Make sure not to parse any quoted reply text. + let body: String? + if isReply() { + body = MXReplyEventParser().parse(self).bodyParts.replyText + } else { + body = content["body"] as? String } + // Find the first url and make sure it's https or http. + guard let textMessage = body, + let url = textMessage.vc_firstURLDetected(), + url.scheme == "https" || url.scheme == "http" + else { return nil } + return url } } diff --git a/Riot/Managers/URLPreviews/PreviewManager.swift b/Riot/Managers/URLPreviews/PreviewManager.swift index fefc6ef991..e642c5e274 100644 --- a/Riot/Managers/URLPreviews/PreviewManager.swift +++ b/Riot/Managers/URLPreviews/PreviewManager.swift @@ -19,27 +19,81 @@ import Foundation @objcMembers class PreviewManager: NSObject { let restClient: MXRestClient + let mediaManager: MXMediaManager - // temporary in memory cache for now - let cache = NSCache() + // Core Data cache to reduce network requests + let cache = URLPreviewCache() - init(restClient: MXRestClient) { + init(restClient: MXRestClient, mediaManager: MXMediaManager) { self.restClient = restClient - cache.countLimit = 20 + self.mediaManager = mediaManager } - func preview(for url: URL, success: @escaping (MXURLPreview?) -> Void, failure: @escaping (Error?) -> Void) { - if let preview = cache.object(forKey: url as NSURL) { + func preview(for url: URL, success: @escaping (URLPreviewViewData) -> Void, failure: @escaping (Error?) -> Void) { + // Sanitize the URL before checking cache or performing lookup + let sanitizedURL = sanitize(url) + + if let preview = cache.preview(for: sanitizedURL) { + MXLog.debug("[PreviewManager] Using preview from cache") success(preview) return } - restClient.preview(for: url, success: { preview in + restClient.preview(for: sanitizedURL, success: { preview in + MXLog.debug("[PreviewManager] Preview not found in cache. Requesting from homeserver.") + if let preview = preview { - self.cache.setObject(preview, forKey: url as NSURL) + self.makePreviewData(for: sanitizedURL, from: preview) { previewData in + self.cache.store(previewData) + success(previewData) + } } - success(preview) }, failure: failure) } + + func makePreviewData(for url: URL, from preview: MXURLPreview, completion: @escaping (URLPreviewViewData) -> Void) { + let previewData = URLPreviewViewData(url: url, siteName: preview.siteName, title: preview.title, text: preview.text) + + guard let imageURL = preview.imageURL else { + completion(previewData) + return + } + + if let cachePath = MXMediaManager.cachePath(forMatrixContentURI: imageURL, andType: preview.imageType, inFolder: nil), + let image = MXMediaManager.loadThroughCache(withFilePath: cachePath) { + previewData.image = image + completion(previewData) + return + } + + // Don't de-dupe image downloads as the manager should de-dupe preview generation. + + mediaManager.downloadMedia(fromMatrixContentURI: imageURL, withType: preview.imageType, inFolder: nil) { path in + guard let image = MXMediaManager.loadThroughCache(withFilePath: path) else { + completion(previewData) + return + } + previewData.image = image + completion(previewData) + } failure: { error in + completion(previewData) + } + } + + func sanitize(_ url: URL) -> URL { + // Remove the fragment from the URL. + var components = URLComponents(url: url, resolvingAgainstBaseURL: false) + components?.fragment = nil + + return components?.url ?? url + } + + func removeExpiredItemsFromCache() { + cache.removeExpiredItems() + } + + func clearCache() { + cache.clear() + } } diff --git a/Riot/Managers/URLPreviews/URLPreviewCache.swift b/Riot/Managers/URLPreviews/URLPreviewCache.swift new file mode 100644 index 0000000000..f443b8bf9e --- /dev/null +++ b/Riot/Managers/URLPreviews/URLPreviewCache.swift @@ -0,0 +1,138 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import CoreData + +/// A cache for URL previews backed by Core Data. +class URLPreviewCache { + + // MARK: - Properties + + /// The Core Data container for persisting the cache to disk. + private let container: NSPersistentContainer + + /// The Core Data context used to store and load data on. + private var context: NSManagedObjectContext { + container.viewContext + } + + /// A time interval that represents how long an item in the cache is valid for. + private let dataValidityTime: TimeInterval = 60 * 60 * 24 + + /// The oldest `creationDate` allowed for valid data. + private var expiryDate: Date { + Date().addingTimeInterval(-dataValidityTime) + } + + // MARK: - Lifecycle + + /// Create a URLPreview Cache optionally storing the data in memory. + /// - Parameter inMemory: Whether to store the data in memory. + init(inMemory: Bool = false) { + // Register the transformer for the `image` field. + ValueTransformer.setValueTransformer(URLPreviewImageTransformer(), forName: .urlPreviewImageTransformer) + + // Create the container, updating it's path if storing the data in memory. + container = NSPersistentContainer(name: "URLPreviewCache") + + if inMemory { + container.persistentStoreDescriptions.first?.url = URL(fileURLWithPath: "/dev/null") + } + + // Load the persistent stores into the container + container.loadPersistentStores { storeDescription, error in + if let error = error { + MXLog.error("[URLPreviewCache] Core Data container error: \(error.localizedDescription)") + } + } + } + + // MARK: - Public + + /// Store a preview in the cache. If a preview already exists with the same URL it will be updated from the new preview. + /// - Parameter preview: The preview to add to the cache. + /// - Parameter date: Optional: The date the preview was generated. + func store(_ preview: URLPreviewViewData, generatedOn generationDate: Date? = nil) { + // Create a fetch request for an existing preview. + let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() + request.predicate = NSPredicate(format: "url == %@", preview.url as NSURL) + + // Use the custom date if supplied (currently this is for testing purposes) + let date = generationDate ?? Date() + + // Update existing data if found otherwise create new data. + if let cachedPreview = try? context.fetch(request).first { + cachedPreview.update(from: preview, on: date) + } else { + _ = URLPreviewCacheData(context: context, preview: preview, creationDate: date) + } + + save() + } + + /// Fetches the preview from the cache for the supplied URL. If a preview doesn't exist or + /// if the preview is older than the ``dataValidityTime`` the returned value will be nil. + /// - Parameter url: The URL to fetch the preview for. + /// - Returns: The preview if found, otherwise nil. + func preview(for url: URL) -> URLPreviewViewData? { + // Create a request for the url excluding any expired items + let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() + request.predicate = NSCompoundPredicate(type: .and, subpredicates: [ + NSPredicate(format: "url == %@", url as NSURL), + NSPredicate(format: "creationDate > %@", expiryDate as NSDate) + ]) + + // Fetch the request, returning nil if nothing was found + guard + let cachedPreview = try? context.fetch(request).first + else { return nil } + + // Convert and return + return cachedPreview.preview() + } + + func count() -> Int { + let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() + return (try? context.count(for: request)) ?? 0 + } + + func removeExpiredItems() { + let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() + request.predicate = NSPredicate(format: "creationDate < %@", expiryDate as NSDate) + + do { + try context.execute(NSBatchDeleteRequest(fetchRequest: request)) + } catch { + MXLog.error("[URLPreviewCache] Error executing batch delete request: \(error.localizedDescription)") + } + } + + func clear() { + do { + _ = try context.execute(NSBatchDeleteRequest(fetchRequest: URLPreviewCacheData.fetchRequest())) + } catch { + MXLog.error("[URLPreviewCache] Error executing batch delete request: \(error.localizedDescription)") + } + } + + // MARK: - Private + + /// Saves any changes that are found on the context + private func save() { + guard context.hasChanges else { return } + try? context.save() + } +} diff --git a/Riot/Managers/URLPreviews/URLPreviewCache.xcdatamodeld/URLPreviewCache.xcdatamodel/contents b/Riot/Managers/URLPreviews/URLPreviewCache.xcdatamodeld/URLPreviewCache.xcdatamodel/contents new file mode 100644 index 0000000000..cdda6f4a4a --- /dev/null +++ b/Riot/Managers/URLPreviews/URLPreviewCache.xcdatamodeld/URLPreviewCache.xcdatamodel/contents @@ -0,0 +1,14 @@ + + + + + + + + + + + + + + \ No newline at end of file diff --git a/Riot/Managers/URLPreviews/URLPreviewCacheData.swift b/Riot/Managers/URLPreviews/URLPreviewCacheData.swift new file mode 100644 index 0000000000..4b7b0fd90f --- /dev/null +++ b/Riot/Managers/URLPreviews/URLPreviewCacheData.swift @@ -0,0 +1,43 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import CoreData + +extension URLPreviewCacheData { + convenience init(context: NSManagedObjectContext, preview: URLPreviewViewData, creationDate: Date) { + self.init(context: context) + update(from: preview, on: creationDate) + } + + func update(from preview: URLPreviewViewData, on date: Date) { + url = preview.url + siteName = preview.siteName + title = preview.title + text = preview.text + image = preview.image + + creationDate = date + } + + func preview() -> URLPreviewViewData? { + guard let url = url else { return nil } + + let viewData = URLPreviewViewData(url: url, siteName: siteName, title: title, text: text) + viewData.image = image as? UIImage + + return viewData + } +} diff --git a/Riot/Managers/URLPreviews/URLPreviewImageTransformer.swift b/Riot/Managers/URLPreviews/URLPreviewImageTransformer.swift new file mode 100644 index 0000000000..565cc96e98 --- /dev/null +++ b/Riot/Managers/URLPreviews/URLPreviewImageTransformer.swift @@ -0,0 +1,45 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import CoreData + +/// A `ValueTransformer` for ``URLPreviewCacheData``'s `image` field. +/// This class transforms between `UIImage` and it's `pngData()` representation. +class URLPreviewImageTransformer: ValueTransformer { + override class func transformedValueClass() -> AnyClass { + UIImage.self + } + + override class func allowsReverseTransformation() -> Bool { + true + } + + /// Transforms a `UIImage` into it's `pngData()` representation. + override func transformedValue(_ value: Any?) -> Any? { + guard let image = value as? UIImage else { return nil } + return image.pngData() + } + + /// Transforms `Data` into a `UIImage` + override func reverseTransformedValue(_ value: Any?) -> Any? { + guard let data = value as? Data else { return nil } + return UIImage(data: data) + } +} + +extension NSValueTransformerName { + static let urlPreviewImageTransformer = NSValueTransformerName("URLPreviewImageTransformer") +} diff --git a/Riot/Modules/Application/LegacyAppDelegate.h b/Riot/Modules/Application/LegacyAppDelegate.h index 1b06d8cbeb..555c1ace4d 100644 --- a/Riot/Modules/Application/LegacyAppDelegate.h +++ b/Riot/Modules/Application/LegacyAppDelegate.h @@ -31,6 +31,7 @@ @protocol LegacyAppDelegateDelegate; @class CallBar; @class CallPresenter; +@class PreviewManager; #pragma mark - Notifications /** @@ -104,6 +105,9 @@ UINavigationControllerDelegate // Associated matrix sessions (empty by default). @property (nonatomic, readonly) NSArray *mxSessions; +#warning Move this elsewhere. +@property (nonatomic, readonly) PreviewManager *previewManager; + // Current selected room id. nil if no room is presently visible. @property (strong, nonatomic) NSString *visibleRoomId; diff --git a/Riot/Modules/Application/LegacyAppDelegate.m b/Riot/Modules/Application/LegacyAppDelegate.m index a6df6cb486..1133de1a55 100644 --- a/Riot/Modules/Application/LegacyAppDelegate.m +++ b/Riot/Modules/Application/LegacyAppDelegate.m @@ -547,6 +547,9 @@ - (void)applicationDidEnterBackground:(UIApplication *)application // check if some media must be released to reduce the cache size [MXMediaManager reduceCacheSizeToInsert:0]; + // Remove expired URL previews from the cache + [self.previewManager removeExpiredItemsFromCache]; + // Hide potential notification if (self.mxInAppNotification) { @@ -1980,6 +1983,10 @@ - (void)addMatrixSession:(MXSession *)mxSession [self checkDeviceId:mxSession]; [self.delegate legacyAppDelegate:self didAddMatrixSession:mxSession]; + + #warning Move this elsewhere + self->_previewManager = [[PreviewManager alloc] initWithRestClient:mxSession.matrixRestClient + mediaManager:mxSession.mediaManager]; } } diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index 43a8139d27..d301fb3b6b 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -135,7 +135,10 @@ - (instancetype)initWithEvent:(MXEvent *)event andRoomState:(MXRoomState *)roomS case MXEventTypeRoomMessage: { // If the message contains a URL, store it in the cell data. - self.link = [event vc_firstURLInBody]; + if (!self.isEncryptedRoom) + { + self.link = [event vc_firstURLInBody]; + } } break; case MXEventTypeCallInvite: @@ -807,7 +810,7 @@ - (BOOL)addEvent:(MXEvent*)event andRoomState:(MXRoomState*)roomState { // FIXME: This should be for all event types, not just messages. // Don't add it if there is already a link in the cell data - if (self.link) + if (self.link && !self.isEncryptedRoom) { shouldAddEvent = NO; } @@ -885,7 +888,10 @@ - (BOOL)addEvent:(MXEvent*)event andRoomState:(MXRoomState*)roomState shouldAddEvent = [super addEvent:event andRoomState:roomState]; } - if (shouldAddEvent) + // When adding events, if there is a link they should be going before and + // so not have links in. If there isn't a link the could come after and + // contain a link, so update the link property if necessary + if (shouldAddEvent && !self.link && !self.isEncryptedRoom) { self.link = [event vc_firstURLInBody]; } diff --git a/Riot/Modules/Room/Views/BubbleCells/Encryption/RoomBubbleCellLayout.swift b/Riot/Modules/Room/Views/BubbleCells/Encryption/RoomBubbleCellLayout.swift index ce636a9899..3ba0a58a31 100644 --- a/Riot/Modules/Room/Views/BubbleCells/Encryption/RoomBubbleCellLayout.swift +++ b/Riot/Modules/Room/Views/BubbleCells/Encryption/RoomBubbleCellLayout.swift @@ -20,6 +20,12 @@ import Foundation @objcMembers final class RoomBubbleCellLayout: NSObject { + // URL Previews + + static let urlPreviewViewTopMargin: CGFloat = 8.0 + static let urlPreviewViewHeight: CGFloat = 247.0 + static let urlPreviewViewWidth: CGFloat = 267.0 + // Reactions static let reactionsViewTopMargin: CGFloat = 1.0 diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift new file mode 100644 index 0000000000..ce5925b8fd --- /dev/null +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift @@ -0,0 +1,145 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import UIKit +import Reusable + +@objcMembers +class URLPreviewView: UIView, NibLoadable, Themable { + // MARK: - Constants + + private enum Constants { } + + // MARK: - Properties + + var viewModel: URLPreviewViewModel! { + didSet { + viewModel.viewDelegate = self + } + } + + @IBOutlet weak var imageView: UIImageView! + @IBOutlet weak var faviconImageView: UIImageView! + + @IBOutlet weak var siteNameLabel: UILabel! + @IBOutlet weak var titleLabel: UILabel! + @IBOutlet weak var descriptionLabel: UILabel! + + override var intrinsicContentSize: CGSize { + CGSize(width: RoomBubbleCellLayout.urlPreviewViewWidth, height: RoomBubbleCellLayout.urlPreviewViewHeight) + } + + // MARK: - Setup + + static func instantiate(viewModel: URLPreviewViewModel) -> Self { + let view = Self.loadFromNib() + view.update(theme: ThemeService.shared().theme) + + view.viewModel = viewModel + viewModel.process(viewAction: .loadData) + + return view + } + + // MARK: - Life cycle + + override func awakeFromNib() { + super.awakeFromNib() + + layer.cornerRadius = 8 + layer.masksToBounds = true + + imageView.contentMode = .scaleAspectFill + faviconImageView.layer.cornerRadius = 6 + + siteNameLabel.isUserInteractionEnabled = false + titleLabel.isUserInteractionEnabled = false + descriptionLabel.isUserInteractionEnabled = false + + #warning("Debugging for previews - to be removed") + faviconImageView.backgroundColor = .systemBlue.withAlphaComponent(0.7) + } + + // MARK: - Public + + func update(theme: Theme) { + backgroundColor = theme.colors.navigation + + siteNameLabel.textColor = theme.colors.secondaryContent + siteNameLabel.font = theme.fonts.caption2SB + + titleLabel.textColor = theme.colors.primaryContent + titleLabel.font = theme.fonts.calloutSB + + descriptionLabel.textColor = theme.colors.secondaryContent + descriptionLabel.font = theme.fonts.caption1 + } + + // MARK: - Private + private func renderLoading(_ url: URL) { + imageView.image = nil + + siteNameLabel.text = url.host + titleLabel.text = "Loading..." + descriptionLabel.text = "" + } + + private func renderLoaded(_ preview: URLPreviewViewData) { + imageView.image = preview.image + + siteNameLabel.text = preview.siteName ?? preview.url.host + titleLabel.text = preview.title + descriptionLabel.text = preview.text + } + + private func renderError(_ error: Error) { + imageView.image = nil + + siteNameLabel.text = "Error" + titleLabel.text = descriptionLabel.text + descriptionLabel.text = error.localizedDescription + } + + + // MARK: - Action + @IBAction private func openURL(_ sender: Any) { + MXLog.debug("[URLPreviewView] Link was tapped.") + viewModel.process(viewAction: .openURL) + } + + @IBAction private func close(_ sender: Any) { + + } +} + + +// MARK: URLPreviewViewModelViewDelegate +extension URLPreviewView: URLPreviewViewModelViewDelegate { + func urlPreviewViewModel(_ viewModel: URLPreviewViewModelType, didUpdateViewState viewState: URLPreviewViewState) { + DispatchQueue.main.async { + switch viewState { + case .loading(let url): + self.renderLoading(url) + case .loaded(let preview): + self.renderLoaded(preview) + case .error(let error): + self.renderError(error) + case .hidden: + self.frame.size.height = 0 + } + } + } +} diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib new file mode 100644 index 0000000000..53ce2c0d60 --- /dev/null +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib @@ -0,0 +1,118 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewAction.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewAction.swift new file mode 100644 index 0000000000..a442f0d414 --- /dev/null +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewAction.swift @@ -0,0 +1,24 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +/// URLPreviewView actions exposed to view model +enum URLPreviewViewAction { + case loadData + case openURL + case close +} diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewData.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewData.swift new file mode 100644 index 0000000000..0d93ede22c --- /dev/null +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewData.swift @@ -0,0 +1,42 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +@objc +class URLPreviewViewData: NSObject { + /// The URL that's represented by the preview data. + let url: URL + + /// The OpenGraph site name for the URL. + let siteName: String? + + /// The OpenGraph title for the URL. + let title: String? + + /// The OpenGraph description for the URL. + let text: String? + + /// The OpenGraph image for the URL. + var image: UIImage? + + init(url: URL, siteName: String?, title: String?, text: String?) { + self.url = url + self.siteName = siteName + self.title = title + self.text = text + } +} diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift new file mode 100644 index 0000000000..2b7e567357 --- /dev/null +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift @@ -0,0 +1,89 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import MatrixSDK + +@objcMembers +class URLPreviewViewModel: NSObject, URLPreviewViewModelType { + + // MARK: - Properties + + // MARK: Private + + private let url: URL + private let session: MXSession + + private var currentOperation: MXHTTPOperation? + private var urlPreview: MXURLPreview? + + // MARK: Public + + weak var viewDelegate: URLPreviewViewModelViewDelegate? + + // MARK: - Setup + + init(url: URL, session: MXSession) { + self.url = url + self.session = session + } + + deinit { + cancelOperations() + } + + // MARK: - Public + + func process(viewAction: URLPreviewViewAction) { + switch viewAction { + case .loadData: + loadData() + case .openURL: + openURL() + case .close: + cancelOperations() + } + } + + // MARK: - Private + + private func loadData() { + update(viewState: .loading(url)) + + AppDelegate.theDelegate().previewManager.preview(for: url) { [weak self] preview in + guard let self = self else { return } + + self.update(viewState: .loaded(preview)) + } failure: { error in + #warning("REALLY?!") + if let error = error { + self.update(viewState: .error(error)) + } + } + } + + private func openURL() { + UIApplication.shared.open(url) + } + + private func update(viewState: URLPreviewViewState) { + viewDelegate?.urlPreviewViewModel(self, didUpdateViewState: viewState) + } + + private func cancelOperations() { + currentOperation?.cancel() + } +} diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModelType.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModelType.swift new file mode 100644 index 0000000000..15fa1a78f9 --- /dev/null +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModelType.swift @@ -0,0 +1,29 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +protocol URLPreviewViewModelViewDelegate: AnyObject { + func urlPreviewViewModel(_ viewModel: URLPreviewViewModelType, didUpdateViewState viewState: URLPreviewViewState) +} + +/// Protocol describing the view model used by `URLPreviewView` +protocol URLPreviewViewModelType { + + var viewDelegate: URLPreviewViewModelViewDelegate? { get set } + + func process(viewAction: URLPreviewViewAction) +} diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewState.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewState.swift new file mode 100644 index 0000000000..1059220018 --- /dev/null +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewState.swift @@ -0,0 +1,25 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +/// URLPreviewView state +enum URLPreviewViewState { + case loading(_ url: URL) + case loaded(_ preview: URLPreviewViewData) + case error(Error) + case hidden +} diff --git a/RiotTests/URLPreviewCacheTests.swift b/RiotTests/URLPreviewCacheTests.swift new file mode 100644 index 0000000000..8d89ef02f8 --- /dev/null +++ b/RiotTests/URLPreviewCacheTests.swift @@ -0,0 +1,132 @@ +// +// Copyright 2021 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +@testable import Riot + +class URLPreviewCacheTests: XCTestCase { + var cache: URLPreviewCache! + + func matrixPreview() -> URLPreviewViewData { + let preview = URLPreviewViewData(url: URL(string: "https://www.matrix.org/")!, + siteName: "Matrix", + title: "Home", + text: "An open network for secure, decentralized communication") + preview.image = Asset.Images.appSymbol.image + + return preview + } + + func elementPreview() -> URLPreviewViewData { + URLPreviewViewData(url: URL(string: "https://element.io/")!, + siteName: "Element", + title: "Home", + text: "Secure and independent communication, connected via Matrix") + } + + override func setUpWithError() throws { + // Create a fresh in-memory cache for each test. + cache = URLPreviewCache(inMemory: true) + } + + func testStoreAndRetrieve() { + // Given a URL preview + let preview = matrixPreview() + + // When storing and retrieving that preview. + cache.store(preview) + + guard let cachedPreview = cache.preview(for: preview.url) else { + XCTFail("The cache should return a preview after storing one with the same URL.") + return + } + + // Then the content in the retrieved preview should match the original preview. + XCTAssertEqual(cachedPreview.url, preview.url, "The url should match.") + XCTAssertEqual(cachedPreview.siteName, preview.siteName, "The site name should match.") + XCTAssertEqual(cachedPreview.title, preview.title, "The title should match.") + XCTAssertEqual(cachedPreview.text, preview.text, "The text should match.") + XCTAssertEqual(cachedPreview.image == nil, preview.image == nil, "The cached preview should have an image if the original did.") + } + + func testUpdating() { + // Given a preview stored in the cache. + let preview = matrixPreview() + cache.store(preview) + + guard let cachedPreview = cache.preview(for: preview.url) else { + XCTFail("The cache should return a preview after storing one with the same URL.") + return + } + XCTAssertEqual(cachedPreview.text, preview.text, "The text should match the original preview's text.") + XCTAssertEqual(cache.count(), 1, "There should be 1 item in the cache.") + + // When storing an updated version of that preview. + let updatedPreview = URLPreviewViewData(url: preview.url, siteName: "Matrix", title: "Home", text: "We updated our website.") + cache.store(updatedPreview) + + // Then the store should update the original preview. + guard let updatedCachedPreview = cache.preview(for: preview.url) else { + XCTFail("The cache should return a preview after storing one with the same URL.") + return + } + XCTAssertEqual(updatedCachedPreview.text, updatedPreview.text, "The text should match the updated preview's text.") + XCTAssertEqual(cache.count(), 1, "There should still only be 1 item in the cache.") + } + + func testPreviewExpiry() { + // Given a preview generated 30 days ago. + let preview = matrixPreview() + cache.store(preview, generatedOn: Date().addingTimeInterval(-60 * 60 * 24 * 30)) + + // When retrieving that today. + let cachedPreview = cache.preview(for: preview.url) + + // Then no preview should be returned. + XCTAssertNil(cachedPreview, "The expired preview should not be returned.") + } + + func testRemovingExpiredItems() { + // Given a cache with 2 items, one of which has expired. + testPreviewExpiry() + let preview = elementPreview() + cache.store(preview) + XCTAssertEqual(cache.count(), 2, "There should be 2 items in the cache.") + + // When removing expired items. + cache.removeExpiredItems() + + // Then only the expired item should have been removed. + XCTAssertEqual(cache.count(), 1, "Only 1 item should have been removed from the cache.") + if cache.preview(for: preview.url) == nil { + XCTFail("The valid preview should still be in the cache.") + } + } + + func testClearingTheCache() { + // Given a cache with 2 items. + testStoreAndRetrieve() + let preview = elementPreview() + cache.store(preview) + XCTAssertEqual(cache.count(), 2, "There should be 2 items in the cache.") + + // When clearing the cache. + cache.clear() + + // Then no items should be left in the cache + XCTAssertEqual(cache.count(), 0, "The cache should be empty.") + } +} From 5f598a918c18be9d203a02d996caebe69a8a7d9f Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 24 Aug 2021 09:42:55 +0100 Subject: [PATCH 03/28] Add comments about the un-sanitized URL. --- Riot/Modules/Room/Views/URLPreviews/URLPreviewViewData.swift | 3 ++- Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewData.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewData.swift index 0d93ede22c..8ab7efd1b0 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewData.swift +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewData.swift @@ -18,7 +18,8 @@ import Foundation @objc class URLPreviewViewData: NSObject { - /// The URL that's represented by the preview data. + /// The URL that's represented by the preview data. This may have been sanitized. + /// Note: The original URL, is stored in ``URLPreviewViewModel``. let url: URL /// The OpenGraph site name for the URL. diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift index 2b7e567357..ddf11f2606 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift @@ -23,7 +23,8 @@ class URLPreviewViewModel: NSObject, URLPreviewViewModelType { // MARK: - Properties // MARK: Private - + + /// The original (un-sanitized) URL to be previewed. private let url: URL private let session: MXSession @@ -76,6 +77,7 @@ class URLPreviewViewModel: NSObject, URLPreviewViewModelType { } private func openURL() { + // Open the original (un-sanitized) URL stored in the view model. UIApplication.shared.open(url) } From 59e541667eb55594b8a81d7f5c7294252af445ac Mon Sep 17 00:00:00 2001 From: Doug Date: Wed, 1 Sep 2021 10:37:37 +0100 Subject: [PATCH 04/28] Load and store URLPreviewViewData in RoomBubbleCellData. Implement close button and store the action in Core Data. Hide the preview image view when no image is received. Remove line breaks in description text. --- .../Room/URLPreviews/Contents.json | 6 + .../url_preview_close.imageset/Contents.json | 15 +++ .../url_preview_close.pdf | Bin 0 -> 2120 bytes .../Contents.json | 15 +++ .../url_preview_close_dark.pdf | Bin 0 -> 2120 bytes Riot/Categories/MXEvent.swift | 46 ------- Riot/Categories/NSString.swift | 49 -------- Riot/Generated/Images.swift | 2 + .../URLPreviews/ClosedURLPreview.swift} | 8 +- .../Managers/URLPreviews/PreviewManager.swift | 45 ++++--- .../URLPreviews/URLPreviewCache.swift | 23 +++- .../URLPreviewCache.xcdatamodel/contents | 13 +- .../URLPreviews/URLPreviewCacheData.swift | 9 +- .../Room/CellData/RoomBubbleCellData.h | 5 +- .../Room/CellData/RoomBubbleCellData.m | 89 +++++++++----- .../Modules/Room/DataSources/RoomDataSource.m | 115 +++++++++++++++++- .../Encryption/RoomBubbleCellLayout.swift | 7 +- .../RoomIncomingTextMsgBubbleCell.m | 13 ++ ...mingTextMsgWithPaginationTitleBubbleCell.m | 13 ++ ...comingTextMsgWithoutSenderInfoBubbleCell.m | 13 ++ .../RoomOutgoingTextMsgBubbleCell.m | 13 ++ ...tgoingTextMsgWithoutSenderInfoBubbleCell.m | 13 ++ .../Views/URLPreviews/URLPreviewView.swift | 100 +++++++++------ .../Room/Views/URLPreviews/URLPreviewView.xib | 101 ++++++++------- .../URLPreviews/URLPreviewViewData.swift | 17 ++- ...ate.swift => URLPreviewViewDelegate.swift} | 10 +- .../URLPreviews/URLPreviewViewModel.swift | 91 -------------- .../URLPreviews/URLPreviewViewModelType.swift | 29 ----- 28 files changed, 484 insertions(+), 376 deletions(-) create mode 100644 Riot/Assets/Images.xcassets/Room/URLPreviews/Contents.json create mode 100644 Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close.imageset/Contents.json create mode 100644 Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close.imageset/url_preview_close.pdf create mode 100644 Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close_dark.imageset/Contents.json create mode 100644 Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close_dark.imageset/url_preview_close_dark.pdf delete mode 100644 Riot/Categories/MXEvent.swift delete mode 100644 Riot/Categories/NSString.swift rename Riot/{Modules/Room/Views/URLPreviews/URLPreviewViewAction.swift => Managers/URLPreviews/ClosedURLPreview.swift} (80%) rename Riot/Modules/Room/Views/URLPreviews/{URLPreviewViewState.swift => URLPreviewViewDelegate.swift} (70%) delete mode 100644 Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModel.swift delete mode 100644 Riot/Modules/Room/Views/URLPreviews/URLPreviewViewModelType.swift diff --git a/Riot/Assets/Images.xcassets/Room/URLPreviews/Contents.json b/Riot/Assets/Images.xcassets/Room/URLPreviews/Contents.json new file mode 100644 index 0000000000..73c00596a7 --- /dev/null +++ b/Riot/Assets/Images.xcassets/Room/URLPreviews/Contents.json @@ -0,0 +1,6 @@ +{ + "info" : { + "author" : "xcode", + "version" : 1 + } +} diff --git a/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close.imageset/Contents.json b/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close.imageset/Contents.json new file mode 100644 index 0000000000..41e31d6f97 --- /dev/null +++ b/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close.imageset/Contents.json @@ -0,0 +1,15 @@ +{ + "images" : [ + { + "filename" : "url_preview_close.pdf", + "idiom" : "universal" + } + ], + "info" : { + "author" : "xcode", + "version" : 1 + }, + "properties" : { + "preserves-vector-representation" : true + } +} diff --git a/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close.imageset/url_preview_close.pdf b/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close.imageset/url_preview_close.pdf new file mode 100644 index 0000000000000000000000000000000000000000..05f7a69500e1aef66c827662d0aa7d0fa5f8da46 GIT binary patch literal 2120 zcmcIlO>f&U487}D@KT^X)D~^al7OPXntou|hIQ#~#SWgMrWq1@i`}8yuP-HAmQx!T zFl<86C{H5!NWG9px0lza7)izuIMiRiF@Uo(h_9a8o4dAXP4^AmM_B-m6t@e`&$~Zn zs+Bck$$|ZQQ?1)aNC{lhMb@-?Q|wrLX?|`>v%I;0#ohQ@*{~lhzCu_x3?mnUQ?Pz> zI_#!r!%?|o#7=Y6IazA170>J@LzFo*WyO-rJH*8Ab|X|osZ4}Q0acVIGD8_C1d9-m z4F&@?Y^F!SLrFyY%tEBWV2)+LN)}848g_H1nKH{rYfVJ~K@TrD zX5h-430q#OPUh<^XqRwTW*$i{kxM}P3`v?Ruuqd>?rMusq7$VPpl}XL0LR$GxMZYI zu4SI0btw>4mO;RDq@>EFvJQJ=9c4&`6HS<7qG3Q1Pgiv`1SDNeG}%o#SY|U>SZc0lBf48Qb-A6-cE+O( z>6R_vWH%gQZVxHxf2D<2?>?uG$`8Qgy&Aji_zOCC1-i-y!QayBg>a3$_@usTkwt@RE zG(Bj=pMQNf5Z@MS(*qyOYExX)&u||)>Y%mIyu!|VTAHSQ+LxvQJD--lyG32KrlN>; z=y>q9*b@e#hiK^d94r(@+q=z*sDjO42|muEej;0(@NWnid7{ZO^3_7Q8#^cOXigIp`l8HVY@Fj zTeD|&*1x@^avV4H&YZyUW_8jPC0^Eb3pQa_;P`Iy+Yoy<%Zq*M(m|yaJ36}hc>Mx6 CaJxwW literal 0 HcmV?d00001 diff --git a/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close_dark.imageset/Contents.json b/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close_dark.imageset/Contents.json new file mode 100644 index 0000000000..9aa8a10fc5 --- /dev/null +++ b/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close_dark.imageset/Contents.json @@ -0,0 +1,15 @@ +{ + "images" : [ + { + "filename" : "url_preview_close_dark.pdf", + "idiom" : "universal" + } + ], + "info" : { + "author" : "xcode", + "version" : 1 + }, + "properties" : { + "preserves-vector-representation" : true + } +} diff --git a/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close_dark.imageset/url_preview_close_dark.pdf b/Riot/Assets/Images.xcassets/Room/URLPreviews/url_preview_close_dark.imageset/url_preview_close_dark.pdf new file mode 100644 index 0000000000000000000000000000000000000000..c2b155fabe7d66d787f0f815c44be24be3231931 GIT binary patch literal 2120 zcmcIl-*3|}5PtVxaW9j$hb6WX$F_tfvF?WuAV$aA#6w8EuBc6*$!=anXL5 zD_3O3463Fjx!$ zSuhA#SWLGD4+RqKG8-ZVgE^K1R^GHPvlj4w-`{Dd1uWvDTA)**I(mM+U0(}S zsfV?INLLFAx{K2@wJ_rFr&>5TI(7LswLs0h>(qT)rw-nZ1(7B;bo(^GG%a42W!2R1 z@ENBEKJllY-yFnu`Py~Bd$-!;m(>eA3>`Jp+R(g)Rqtu(>gsu4xEg3aEjxFMs%%_| z5ozf3;9b5)46rMC2!~Fe13|-Rd%Ia7RiF%((8pO+&uA+q{A)x;p0Wc^ZJLX`$+y+I zGo_^KIT9~>_sBSC*8aoj8#1L51qgSKz=1^%lmXJn^8gYm!+v`NVK+U3bcR_Uo-)*^ zJE+;`o2}b3y1h47D934C?c5ohZdPZlDDkqY8lZ%|!14X&mqYe`mgoD%rz4Ct?BwM7 G!_9A{`@3ZT literal 0 HcmV?d00001 diff --git a/Riot/Categories/MXEvent.swift b/Riot/Categories/MXEvent.swift deleted file mode 100644 index eb6f9b440d..0000000000 --- a/Riot/Categories/MXEvent.swift +++ /dev/null @@ -1,46 +0,0 @@ -// -// Copyright 2021 New Vector Ltd -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -import Foundation - -extension MXEvent { - /// Gets the first URL contained in a text message event's body. - /// - Returns: A URL if detected, otherwise nil. - @objc func vc_firstURLInBody() -> NSURL? { - // Only get URLs for un-redacted message events that are text, notice or emote. - guard isRedactedEvent() == false, - eventType == .roomMessage, - let messageType = content["msgtype"] as? String, - messageType == kMXMessageTypeText || messageType == kMXMessageTypeNotice || messageType == kMXMessageTypeEmote - else { return nil } - - // Make sure not to parse any quoted reply text. - let body: String? - if isReply() { - body = MXReplyEventParser().parse(self).bodyParts.replyText - } else { - body = content["body"] as? String - } - - // Find the first url and make sure it's https or http. - guard let textMessage = body, - let url = textMessage.vc_firstURLDetected(), - url.scheme == "https" || url.scheme == "http" - else { return nil } - - return url - } -} diff --git a/Riot/Categories/NSString.swift b/Riot/Categories/NSString.swift deleted file mode 100644 index e593c1a263..0000000000 --- a/Riot/Categories/NSString.swift +++ /dev/null @@ -1,49 +0,0 @@ -// -// Copyright 2021 New Vector Ltd -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -import Foundation - -extension NSString { - /// Check if the string contains a URL. - /// - Returns: True if the string contains at least one URL, otherwise false. - @objc func vc_containsURL() -> Bool { - guard let linkDetector = try? NSDataDetector(types: NSTextCheckingResult.CheckingType.link.rawValue) else { - MXLog.debug("[NSString+URLDetector]: Unable to create link detector.") - return false - } - - // return true if there is at least one match - return linkDetector.numberOfMatches(in: self as String, options: [], range: NSRange(location: 0, length: self.length)) > 0 - } - - /// Gets the first URL contained in the string. - /// - Returns: A URL if detected, otherwise nil. - @objc func vc_firstURLDetected() -> NSURL? { - guard let linkDetector = try? NSDataDetector(types: NSTextCheckingResult.CheckingType.link.rawValue) else { - MXLog.debug("[NSString+URLDetector]: Unable to create link detector.") - return nil - } - - // find the first match, otherwise return nil - guard let match = linkDetector.firstMatch(in: self as String, options: [], range: NSRange(location: 0, length: self.length)) else { - return nil - } - - // create a url and return it. - let urlString = self.substring(with: match.range) - return NSURL(string: urlString) - } -} diff --git a/Riot/Generated/Images.swift b/Riot/Generated/Images.swift index c6b056c71e..559b3adfb9 100644 --- a/Riot/Generated/Images.swift +++ b/Riot/Generated/Images.swift @@ -137,6 +137,8 @@ internal enum Asset { internal static let videoCall = ImageAsset(name: "video_call") internal static let voiceCallHangonIcon = ImageAsset(name: "voice_call_hangon_icon") internal static let voiceCallHangupIcon = ImageAsset(name: "voice_call_hangup_icon") + internal static let urlPreviewClose = ImageAsset(name: "url_preview_close") + internal static let urlPreviewCloseDark = ImageAsset(name: "url_preview_close_dark") internal static let voiceMessageCancelGradient = ImageAsset(name: "voice_message_cancel_gradient") internal static let voiceMessageLockChevron = ImageAsset(name: "voice_message_lock_chevron") internal static let voiceMessageLockIconLocked = ImageAsset(name: "voice_message_lock_icon_locked") diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewAction.swift b/Riot/Managers/URLPreviews/ClosedURLPreview.swift similarity index 80% rename from Riot/Modules/Room/Views/URLPreviews/URLPreviewViewAction.swift rename to Riot/Managers/URLPreviews/ClosedURLPreview.swift index a442f0d414..c47434550c 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewViewAction.swift +++ b/Riot/Managers/URLPreviews/ClosedURLPreview.swift @@ -16,9 +16,7 @@ import Foundation -/// URLPreviewView actions exposed to view model -enum URLPreviewViewAction { - case loadData - case openURL - case close +extension ClosedURLPreview { + // Nothing to extend, however having this file stops Xcode + // complaining that it can't find ClosedURLPreview. } diff --git a/Riot/Managers/URLPreviews/PreviewManager.swift b/Riot/Managers/URLPreviews/PreviewManager.swift index e642c5e274..29236f988d 100644 --- a/Riot/Managers/URLPreviews/PreviewManager.swift +++ b/Riot/Managers/URLPreviews/PreviewManager.swift @@ -18,22 +18,22 @@ import Foundation @objcMembers class PreviewManager: NSObject { - let restClient: MXRestClient - let mediaManager: MXMediaManager + private let restClient: MXRestClient + private let mediaManager: MXMediaManager // Core Data cache to reduce network requests - let cache = URLPreviewCache() + private let cache = URLPreviewCache() init(restClient: MXRestClient, mediaManager: MXMediaManager) { self.restClient = restClient self.mediaManager = mediaManager } - func preview(for url: URL, success: @escaping (URLPreviewViewData) -> Void, failure: @escaping (Error?) -> Void) { + func preview(for url: URL, and event: MXEvent, success: @escaping (URLPreviewViewData) -> Void, failure: @escaping (Error?) -> Void) { // Sanitize the URL before checking cache or performing lookup let sanitizedURL = sanitize(url) - if let preview = cache.preview(for: sanitizedURL) { + if let preview = cache.preview(for: sanitizedURL, and: event) { MXLog.debug("[PreviewManager] Using preview from cache") success(preview) return @@ -43,7 +43,7 @@ class PreviewManager: NSObject { MXLog.debug("[PreviewManager] Preview not found in cache. Requesting from homeserver.") if let preview = preview { - self.makePreviewData(for: sanitizedURL, from: preview) { previewData in + self.makePreviewData(for: sanitizedURL, and: event, from: preview) { previewData in self.cache.store(previewData) success(previewData) } @@ -52,8 +52,13 @@ class PreviewManager: NSObject { }, failure: failure) } - func makePreviewData(for url: URL, from preview: MXURLPreview, completion: @escaping (URLPreviewViewData) -> Void) { - let previewData = URLPreviewViewData(url: url, siteName: preview.siteName, title: preview.title, text: preview.text) + func makePreviewData(for url: URL, and event: MXEvent, from preview: MXURLPreview, completion: @escaping (URLPreviewViewData) -> Void) { + let previewData = URLPreviewViewData(url: url, + eventID: event.eventId, + roomID: event.roomId, + siteName: preview.siteName, + title: preview.title, + text: preview.text) guard let imageURL = preview.imageURL else { completion(previewData) @@ -81,14 +86,6 @@ class PreviewManager: NSObject { } } - func sanitize(_ url: URL) -> URL { - // Remove the fragment from the URL. - var components = URLComponents(url: url, resolvingAgainstBaseURL: false) - components?.fragment = nil - - return components?.url ?? url - } - func removeExpiredItemsFromCache() { cache.removeExpiredItems() } @@ -96,4 +93,20 @@ class PreviewManager: NSObject { func clearCache() { cache.clear() } + + func closePreview(for eventID: String, in roomID: String) { + cache.closePreview(for: eventID, in: roomID) + } + + func hasClosedPreview(from event: MXEvent) -> Bool { + cache.hasClosedPreview(for: event.eventId, in: event.roomId) + } + + private func sanitize(_ url: URL) -> URL { + // Remove the fragment from the URL. + var components = URLComponents(url: url, resolvingAgainstBaseURL: false) + components?.fragment = nil + + return components?.url ?? url + } } diff --git a/Riot/Managers/URLPreviews/URLPreviewCache.swift b/Riot/Managers/URLPreviews/URLPreviewCache.swift index f443b8bf9e..33711e5f28 100644 --- a/Riot/Managers/URLPreviews/URLPreviewCache.swift +++ b/Riot/Managers/URLPreviews/URLPreviewCache.swift @@ -87,7 +87,7 @@ class URLPreviewCache { /// if the preview is older than the ``dataValidityTime`` the returned value will be nil. /// - Parameter url: The URL to fetch the preview for. /// - Returns: The preview if found, otherwise nil. - func preview(for url: URL) -> URLPreviewViewData? { + func preview(for url: URL, and event: MXEvent) -> URLPreviewViewData? { // Create a request for the url excluding any expired items let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() request.predicate = NSCompoundPredicate(type: .and, subpredicates: [ @@ -101,7 +101,7 @@ class URLPreviewCache { else { return nil } // Convert and return - return cachedPreview.preview() + return cachedPreview.preview(for: event) } func count() -> Int { @@ -123,11 +123,30 @@ class URLPreviewCache { func clear() { do { _ = try context.execute(NSBatchDeleteRequest(fetchRequest: URLPreviewCacheData.fetchRequest())) + _ = try context.execute(NSBatchDeleteRequest(fetchRequest: ClosedURLPreview.fetchRequest())) } catch { MXLog.error("[URLPreviewCache] Error executing batch delete request: \(error.localizedDescription)") } } + func closePreview(for eventID: String, in roomID: String) { + let closedPreview = ClosedURLPreview(context: context) + closedPreview.eventID = eventID + closedPreview.roomID = roomID + save() + } + + func hasClosedPreview(for eventID: String, in roomID: String) -> Bool { + // Create a request for the url excluding any expired items + let request: NSFetchRequest = ClosedURLPreview.fetchRequest() + request.predicate = NSCompoundPredicate(type: .and, subpredicates: [ + NSPredicate(format: "eventID == %@", eventID), + NSPredicate(format: "roomID == %@", roomID) + ]) + + return (try? context.count(for: request)) ?? 0 > 0 + } + // MARK: - Private /// Saves any changes that are found on the context diff --git a/Riot/Managers/URLPreviews/URLPreviewCache.xcdatamodeld/URLPreviewCache.xcdatamodel/contents b/Riot/Managers/URLPreviews/URLPreviewCache.xcdatamodeld/URLPreviewCache.xcdatamodel/contents index cdda6f4a4a..cade6911fc 100644 --- a/Riot/Managers/URLPreviews/URLPreviewCache.xcdatamodeld/URLPreviewCache.xcdatamodel/contents +++ b/Riot/Managers/URLPreviews/URLPreviewCache.xcdatamodeld/URLPreviewCache.xcdatamodel/contents @@ -1,5 +1,15 @@ + + + + + + + + + + @@ -9,6 +19,7 @@ - + + \ No newline at end of file diff --git a/Riot/Managers/URLPreviews/URLPreviewCacheData.swift b/Riot/Managers/URLPreviews/URLPreviewCacheData.swift index 4b7b0fd90f..f32d276b54 100644 --- a/Riot/Managers/URLPreviews/URLPreviewCacheData.swift +++ b/Riot/Managers/URLPreviews/URLPreviewCacheData.swift @@ -32,10 +32,15 @@ extension URLPreviewCacheData { creationDate = date } - func preview() -> URLPreviewViewData? { + func preview(for event: MXEvent) -> URLPreviewViewData? { guard let url = url else { return nil } - let viewData = URLPreviewViewData(url: url, siteName: siteName, title: title, text: text) + let viewData = URLPreviewViewData(url: url, + eventID: event.eventId, + roomID: event.roomId, + siteName: siteName, + title: title, + text: text) viewData.image = image as? UIImage return viewData diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.h b/Riot/Modules/Room/CellData/RoomBubbleCellData.h index f77d99adc1..9af568e2c3 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.h +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.h @@ -15,6 +15,9 @@ */ #import +@class URLPreviewViewData; + +extern NSString *const URLPreviewDidUpdateNotification; // Custom tags for MXKRoomBubbleCellDataStoring.tag typedef NS_ENUM(NSInteger, RoomBubbleCellDataTag) @@ -81,7 +84,7 @@ typedef NS_ENUM(NSInteger, RoomBubbleCellDataTag) /** A link if the textMessage contains one, otherwise nil. */ -@property (nonatomic) NSURL *link; +@property (nonatomic) URLPreviewViewData *urlPreviewData; /** MXKeyVerification object associated to key verification event when using key verification by direct message. diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index d301fb3b6b..aba1eb55a5 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -24,9 +24,12 @@ #import "BubbleReactionsViewSizer.h" #import "Riot-Swift.h" +#import static NSAttributedString *timestampVerticalWhitespace = nil; +NSString *const URLPreviewDidUpdateNotification = @"URLPreviewDidUpdateNotification"; + @interface RoomBubbleCellData() @property(nonatomic, readonly) BOOL addVerticalWhitespaceForSelectedComponentTimestamp; @@ -132,15 +135,6 @@ - (instancetype)initWithEvent:(MXEvent *)event andRoomState:(MXRoomState *)roomS self.collapsed = YES; } break; - case MXEventTypeRoomMessage: - { - // If the message contains a URL, store it in the cell data. - if (!self.isEncryptedRoom) - { - self.link = [event vc_firstURLInBody]; - } - } - break; case MXEventTypeCallInvite: case MXEventTypeCallAnswer: case MXEventTypeCallHangup: @@ -185,6 +179,12 @@ - (instancetype)initWithEvent:(MXEvent *)event andRoomState:(MXRoomState *)roomS // Reset attributedTextMessage to force reset MXKRoomCellData parameters self.attributedTextMessage = nil; + + // Load a url preview if a link was detected + if (self.hasLink) + { + [self loadURLPreview]; + } } return self; @@ -592,8 +592,8 @@ - (CGFloat)reactionHeightForEventId:(NSString*)eventId }); BOOL showAllReactions = [self.eventsToShowAllReactions containsObject:eventId]; - BubbleReactionsViewModel *viemModel = [[BubbleReactionsViewModel alloc] initWithAggregatedReactions:aggregatedReactions eventId:eventId showAll:showAllReactions]; - height = [bubbleReactionsViewSizer heightForViewModel:viemModel fittingWidth:bubbleReactionsViewWidth] + RoomBubbleCellLayout.reactionsViewTopMargin; + BubbleReactionsViewModel *viewModel = [[BubbleReactionsViewModel alloc] initWithAggregatedReactions:aggregatedReactions eventId:eventId showAll:showAllReactions]; + height = [bubbleReactionsViewSizer heightForViewModel:viewModel fittingWidth:bubbleReactionsViewWidth] + RoomBubbleCellLayout.reactionsViewTopMargin; } return height; @@ -800,17 +800,15 @@ - (BOOL)addEvent:(MXEvent*)event andRoomState:(MXRoomState*)roomState break; } - NSDate *eventDate = [NSDate dateWithTimeIntervalSince1970:(double)event.originServerTs/1000]; - - if (self.mostRecentComponentIndex != NSNotFound) + if (self.bubbleComponents.lastObject) { - MXKRoomBubbleComponent *lastComponent = self.bubbleComponents[self.mostRecentComponentIndex]; + MXKRoomBubbleComponent *lastComponent = self.bubbleComponents.lastObject; // If the new event comes after the last bubble component - if ([lastComponent.date earlierDate:eventDate] == lastComponent.date) + if (event.originServerTs > lastComponent.event.originServerTs) { // FIXME: This should be for all event types, not just messages. // Don't add it if there is already a link in the cell data - if (self.link && !self.isEncryptedRoom) + if (self.hasLink && !self.isEncryptedRoom) { shouldAddEvent = NO; } @@ -818,15 +816,15 @@ - (BOOL)addEvent:(MXEvent*)event andRoomState:(MXRoomState*)roomState } } - if (self.oldestComponentIndex != NSNotFound) + if (self.bubbleComponents.firstObject) { - MXKRoomBubbleComponent *firstComponent = self.bubbleComponents[self.oldestComponentIndex]; + MXKRoomBubbleComponent *firstComponent = self.bubbleComponents.firstObject; // If the new event event comes before the first bubble component - if ([firstComponent.date laterDate:eventDate] == firstComponent.date) + if (event.originServerTs < firstComponent.event.originServerTs) { // Don't add it to the cell data if it contains a link NSString *messageBody = event.content[@"body"]; - if (messageBody && [messageBody vc_containsURL]) + if (messageBody && [messageBody mxk_firstURLDetected]) { shouldAddEvent = NO; } @@ -885,15 +883,15 @@ - (BOOL)addEvent:(MXEvent*)event andRoomState:(MXRoomState*)roomState if (shouldAddEvent) { + BOOL hadLink = self.hasLink; + shouldAddEvent = [super addEvent:event andRoomState:roomState]; - } - - // When adding events, if there is a link they should be going before and - // so not have links in. If there isn't a link the could come after and - // contain a link, so update the link property if necessary - if (shouldAddEvent && !self.link && !self.isEncryptedRoom) - { - self.link = [event vc_firstURLInBody]; + + // If the cell data now contains a link, set the preview data. + if (shouldAddEvent && self.hasLink && !hadLink) + { + [self loadURLPreview]; + } } return shouldAddEvent; @@ -1061,4 +1059,37 @@ - (NSString*)accessibilityLabelForAttachmentType:(MXKAttachmentType)attachmentTy return accessibilityLabel; } +#pragma mark - URL Previews + +- (void)loadURLPreview +{ + // Get the last bubble component as that contains the link. + MXKRoomBubbleComponent *lastComponent = bubbleComponents.lastObject; + if (!lastComponent) + { + return; + } + + // Check that the preview hasn't been dismissed already. + if ([LegacyAppDelegate.theDelegate.previewManager hasClosedPreviewFrom:lastComponent.event]) + { + return; + } + + // Set the preview data. + MXWeakify(self); + + [LegacyAppDelegate.theDelegate.previewManager previewFor:lastComponent.link and:lastComponent.event success:^(URLPreviewViewData * _Nonnull urlPreviewData) { + MXStrongifyAndReturnIfNil(self); + + // Update the preview data and send a notification for refresh + self.urlPreviewData = urlPreviewData; + [NSNotificationCenter.defaultCenter postNotificationName:URLPreviewDidUpdateNotification object:self]; + + } failure:^(NSError * _Nullable error) { + MXLogDebug(@"[RoomBubbleCellData] Failed to get url preview") + }]; +} + + @end diff --git a/Riot/Modules/Room/DataSources/RoomDataSource.m b/Riot/Modules/Room/DataSources/RoomDataSource.m index 434aee6ac4..1f4ab111be 100644 --- a/Riot/Modules/Room/DataSources/RoomDataSource.m +++ b/Riot/Modules/Room/DataSources/RoomDataSource.m @@ -29,10 +29,13 @@ const CGFloat kTypingCellHeight = 24; -@interface RoomDataSource() +@interface RoomDataSource() { // Observe kThemeServiceDidChangeThemeNotification to handle user interface theme change. id kThemeServiceDidChangeThemeNotificationObserver; + + // Observe URL preview updates to refresh cells. + id kURLPreviewDidUpdateNotificationObserver; } // Observe key verification request changes @@ -71,7 +74,7 @@ - (instancetype)initWithRoomId:(NSString *)roomId andMatrixSession:(MXSession *) // Replace the event formatter [self updateEventFormatter]; - // Handle timestamp and read receips display at Vector app level (see [tableView: cellForRowAtIndexPath:]) + // Handle timestamp and read receipts display at Vector app level (see [tableView: cellForRowAtIndexPath:]) self.useCustomDateTimeLabel = YES; self.useCustomReceipts = YES; self.useCustomUnsentButton = YES; @@ -93,6 +96,13 @@ - (instancetype)initWithRoomId:(NSString *)roomId andMatrixSession:(MXSession *) }]; + // Observe URL preview updates. + kURLPreviewDidUpdateNotificationObserver = [NSNotificationCenter.defaultCenter addObserverForName:URLPreviewDidUpdateNotification object:nil queue:NSOperationQueue.mainQueue usingBlock:^(NSNotification * _Nonnull note) { + + // Refresh all cells. + [self refreshCells]; + }]; + [self registerKeyVerificationRequestNotification]; [self registerKeyVerificationTransactionNotification]; [self registerTrustLevelDidChangeNotifications]; @@ -161,6 +171,12 @@ - (void)destroy kThemeServiceDidChangeThemeNotificationObserver = nil; } + if (kURLPreviewDidUpdateNotificationObserver) + { + [NSNotificationCenter.defaultCenter removeObserver:kURLPreviewDidUpdateNotificationObserver]; + kURLPreviewDidUpdateNotificationObserver = nil; + } + if (self.keyVerificationRequestDidChangeNotificationObserver) { [[NSNotificationCenter defaultCenter] removeObserver:self.keyVerificationRequestDidChangeNotificationObserver]; @@ -343,7 +359,7 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N // Handle read receipts and read marker display. // Ignore the read receipts on the bubble without actual display. // Ignore the read receipts on collapsed bubbles - if ((((self.showBubbleReceipts && cellData.readReceipts.count) || cellData.reactions.count) && !isCollapsableCellCollapsed) || self.showReadMarker) + if ((((self.showBubbleReceipts && cellData.readReceipts.count) || cellData.reactions.count || cellData.hasLink) && !isCollapsableCellCollapsed) || self.showReadMarker) { // Read receipts container are inserted here on the right side into the content view. // Some vertical whitespaces are added in message text view (see RoomBubbleCellData class) to insert correctly multiple receipts. @@ -368,7 +384,41 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N { continue; } - + + NSURL *link = component.link; + URLPreviewView *urlPreviewView; + + // Encrypted rooms must not show URL previews. + if (link && cellData.urlPreviewData && !self.room.summary.isEncrypted) + { + urlPreviewView = [URLPreviewView instantiate]; + urlPreviewView.preview = cellData.urlPreviewData; + urlPreviewView.delegate = self; + + [temporaryViews addObject:urlPreviewView]; + + if (!bubbleCell.tmpSubviews) + { + bubbleCell.tmpSubviews = [NSMutableArray array]; + } + [bubbleCell.tmpSubviews addObject:urlPreviewView]; + + urlPreviewView.translatesAutoresizingMaskIntoConstraints = NO; + [bubbleCell.contentView addSubview:urlPreviewView]; + + CGFloat leftMargin = RoomBubbleCellLayout.reactionsViewLeftMargin; + if (roomBubbleCellData.containsBubbleComponentWithEncryptionBadge) + { + leftMargin+= RoomBubbleCellLayout.encryptedContentLeftMargin; + } + + // Set the preview view's origin + [NSLayoutConstraint activateConstraints: @[ + [urlPreviewView.leadingAnchor constraintEqualToAnchor:urlPreviewView.superview.leadingAnchor constant:leftMargin], + [urlPreviewView.topAnchor constraintEqualToAnchor:urlPreviewView.superview.topAnchor constant:bottomPositionY + RoomBubbleCellLayout.urlPreviewViewTopMargin + RoomBubbleCellLayout.reactionsViewTopMargin], + ]]; + } + MXAggregatedReactions* reactions = cellData.reactions[componentEventId].aggregatedReactionsWithNonZeroCount; BubbleReactionsView *reactionsView; @@ -411,12 +461,23 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N leftMargin+= RoomBubbleCellLayout.encryptedContentLeftMargin; } + // The top constraint may need to include the URL preview view + CGFloat topConstraintConstant; + if (urlPreviewView) + { + topConstraintConstant = bottomPositionY + RoomBubbleCellLayout.urlPreviewViewTopMargin + urlPreviewView.frame.size.height + RoomBubbleCellLayout.reactionsViewTopMargin; + } + else + { + topConstraintConstant = bottomPositionY + RoomBubbleCellLayout.reactionsViewTopMargin; + } + // Force receipts container size [NSLayoutConstraint activateConstraints: @[ [reactionsView.leadingAnchor constraintEqualToAnchor:reactionsView.superview.leadingAnchor constant:leftMargin], [reactionsView.trailingAnchor constraintEqualToAnchor:reactionsView.superview.trailingAnchor constant:-RoomBubbleCellLayout.reactionsViewRightMargin], - [reactionsView.topAnchor constraintEqualToAnchor:reactionsView.superview.topAnchor constant:bottomPositionY + RoomBubbleCellLayout.reactionsViewTopMargin] + [reactionsView.topAnchor constraintEqualToAnchor:reactionsView.superview.topAnchor constant:topConstraintConstant] ]]; } } @@ -522,12 +583,16 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N multiplier:1.0 constant:-RoomBubbleCellLayout.readReceiptsViewRightMargin]; - // At the bottom, we have reactions or nothing + // At the bottom, we either have reactions, a URL preview or nothing NSLayoutConstraint *topConstraint; if (reactionsView) { topConstraint = [avatarsContainer.topAnchor constraintEqualToAnchor:reactionsView.bottomAnchor constant:RoomBubbleCellLayout.readReceiptsViewTopMargin]; } + else if (urlPreviewView) + { + topConstraint = [avatarsContainer.topAnchor constraintEqualToAnchor:urlPreviewView.bottomAnchor constant:RoomBubbleCellLayout.readReceiptsViewTopMargin]; + } else { topConstraint = [avatarsContainer.topAnchor constraintEqualToAnchor:avatarsContainer.superview.topAnchor constant:bottomPositionY + RoomBubbleCellLayout.readReceiptsViewTopMargin]; @@ -1163,4 +1228,42 @@ - (NSUInteger)roomBubbleDataIndexWithTag:(RoomBubbleCellDataTag)tag } } +#pragma mark - URLPreviewViewDelegate + +- (void)didOpenURLFromPreviewView:(URLPreviewView *)previewView for:(NSString *)eventID in:(NSString *)roomID +{ + RoomBubbleCellData *cellData = [self cellDataOfEventWithEventId:eventID]; + + if (!cellData) + { + return; + } + + MXKRoomBubbleComponent *lastComponent = cellData.bubbleComponents.lastObject; + + if (!lastComponent) + { + return; + } + + [UIApplication.sharedApplication vc_open:lastComponent.link completionHandler:nil]; +} + +- (void)didCloseURLPreviewView:(URLPreviewView *)previewView for:(NSString *)eventID in:(NSString *)roomID +{ + RoomBubbleCellData *cellData = [self cellDataOfEventWithEventId:eventID]; + + if (!cellData) + { + return; + } + + // Remember that the user closed the preview so it isn't shown again. + [LegacyAppDelegate.theDelegate.previewManager closePreviewFor:eventID in:roomID]; + + // Remove the preview data and refresh the cells. + cellData.urlPreviewData = nil; + [self refreshCells]; +} + @end diff --git a/Riot/Modules/Room/Views/BubbleCells/Encryption/RoomBubbleCellLayout.swift b/Riot/Modules/Room/Views/BubbleCells/Encryption/RoomBubbleCellLayout.swift index 3ba0a58a31..b9d1db6e7d 100644 --- a/Riot/Modules/Room/Views/BubbleCells/Encryption/RoomBubbleCellLayout.swift +++ b/Riot/Modules/Room/Views/BubbleCells/Encryption/RoomBubbleCellLayout.swift @@ -20,12 +20,6 @@ import Foundation @objcMembers final class RoomBubbleCellLayout: NSObject { - // URL Previews - - static let urlPreviewViewTopMargin: CGFloat = 8.0 - static let urlPreviewViewHeight: CGFloat = 247.0 - static let urlPreviewViewWidth: CGFloat = 267.0 - // Reactions static let reactionsViewTopMargin: CGFloat = 1.0 @@ -51,4 +45,5 @@ final class RoomBubbleCellLayout: NSObject { // Others static let encryptedContentLeftMargin: CGFloat = 15.0 + static let urlPreviewViewTopMargin: CGFloat = 8.0 } diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m index d308fcfea0..e7bb2fbce6 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m @@ -39,4 +39,17 @@ - (void)render:(MXKCellData *)cellData [self updateUserNameColor]; } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + if (bubbleData && bubbleData.urlPreviewData) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m index 2e8bdeb342..3d88949e40 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m @@ -45,4 +45,17 @@ - (void)render:(MXKCellData *)cellData } } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + if (bubbleData && bubbleData.urlPreviewData) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m index 56b0db94c3..3b8c32635f 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m @@ -29,4 +29,17 @@ - (void)customizeTableViewCellRendering self.messageTextView.tintColor = ThemeService.shared.theme.tintColor; } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + if (bubbleData && bubbleData.urlPreviewData) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m index 374ed6da00..9a1a6efa4d 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m @@ -40,4 +40,17 @@ - (void)render:(MXKCellData *)cellData [self updateUserNameColor]; } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + if (bubbleData && bubbleData.urlPreviewData) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m index 1e0ffeb61b..82b54239cc 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m @@ -29,4 +29,17 @@ - (void)customizeTableViewCellRendering self.messageTextView.tintColor = ThemeService.shared.theme.tintColor; } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + if (bubbleData && bubbleData.urlPreviewData) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift index ce5925b8fd..910855b6c1 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift @@ -21,36 +21,49 @@ import Reusable class URLPreviewView: UIView, NibLoadable, Themable { // MARK: - Constants - private enum Constants { } + private static let sizingView = URLPreviewView.instantiate() + + private enum Constants { + // URL Previews + + static let maxHeight: CGFloat = 247.0 + static let width: CGFloat = 267.0 + } // MARK: - Properties - var viewModel: URLPreviewViewModel! { + var preview: URLPreviewViewData? { didSet { - viewModel.viewDelegate = self + guard let preview = preview else { return } + renderLoaded(preview) } } + weak var delegate: URLPreviewViewDelegate? + + @IBOutlet weak var imageContainer: UIView! @IBOutlet weak var imageView: UIImageView! - @IBOutlet weak var faviconImageView: UIImageView! + @IBOutlet weak var closeButton: UIButton! @IBOutlet weak var siteNameLabel: UILabel! @IBOutlet weak var titleLabel: UILabel! @IBOutlet weak var descriptionLabel: UILabel! + /// The constraint that pins the top of the text container to the top of the view. + @IBOutlet weak var textContainerViewConstraint: NSLayoutConstraint! + /// The constraint that pins the top of the text container to the bottom of the image container. + @IBOutlet weak var textContainerImageConstraint: NSLayoutConstraint! + override var intrinsicContentSize: CGSize { - CGSize(width: RoomBubbleCellLayout.urlPreviewViewWidth, height: RoomBubbleCellLayout.urlPreviewViewHeight) + CGSize(width: Constants.width, height: Constants.maxHeight) } // MARK: - Setup - static func instantiate(viewModel: URLPreviewViewModel) -> Self { + static func instantiate() -> Self { let view = Self.loadFromNib() view.update(theme: ThemeService.shared().theme) - view.viewModel = viewModel - viewModel.process(viewAction: .loadData) - return view } @@ -63,14 +76,10 @@ class URLPreviewView: UIView, NibLoadable, Themable { layer.masksToBounds = true imageView.contentMode = .scaleAspectFill - faviconImageView.layer.cornerRadius = 6 siteNameLabel.isUserInteractionEnabled = false titleLabel.isUserInteractionEnabled = false descriptionLabel.isUserInteractionEnabled = false - - #warning("Debugging for previews - to be removed") - faviconImageView.backgroundColor = .systemBlue.withAlphaComponent(0.7) } // MARK: - Public @@ -86,9 +95,19 @@ class URLPreviewView: UIView, NibLoadable, Themable { descriptionLabel.textColor = theme.colors.secondaryContent descriptionLabel.font = theme.fonts.caption1 + + let closeButtonAsset = ThemeService.shared().isCurrentThemeDark() ? Asset.Images.urlPreviewCloseDark : Asset.Images.urlPreviewClose + closeButton.setImage(closeButtonAsset.image, for: .normal) + } + + static func contentViewHeight(for preview: URLPreviewViewData) -> CGFloat { + sizingView.renderLoaded(preview) + + return sizingView.systemLayoutSizeFitting(sizingView.intrinsicContentSize).height } // MARK: - Private + #warning("Check whether we should show a loading state.") private func renderLoading(_ url: URL) { imageView.image = nil @@ -98,48 +117,49 @@ class URLPreviewView: UIView, NibLoadable, Themable { } private func renderLoaded(_ preview: URLPreviewViewData) { - imageView.image = preview.image + if let image = preview.image { + imageView.image = image + showImageContainer() + } else { + imageView.image = nil + hideImageContainer() + } siteNameLabel.text = preview.siteName ?? preview.url.host titleLabel.text = preview.title descriptionLabel.text = preview.text } - private func renderError(_ error: Error) { - imageView.image = nil + private func showImageContainer() { + // When the image container has a superview it is already visible + guard imageContainer.superview == nil else { return } + + textContainerViewConstraint.isActive = false + addSubview(imageContainer) + textContainerImageConstraint.isActive = true - siteNameLabel.text = "Error" - titleLabel.text = descriptionLabel.text - descriptionLabel.text = error.localizedDescription + // Ensure the close button remains visible + bringSubviewToFront(closeButton) } + private func hideImageContainer() { + textContainerImageConstraint.isActive = false + imageContainer.removeFromSuperview() + textContainerViewConstraint.isActive = true + } // MARK: - Action @IBAction private func openURL(_ sender: Any) { MXLog.debug("[URLPreviewView] Link was tapped.") - viewModel.process(viewAction: .openURL) + guard let preview = preview else { return } + + // Ask the delegate to open the URL for the event, as the bubble component + // has the original un-sanitized URL that needs to be opened. + delegate?.didOpenURLFromPreviewView(self, for: preview.eventID, in: preview.roomID) } @IBAction private func close(_ sender: Any) { - - } -} - - -// MARK: URLPreviewViewModelViewDelegate -extension URLPreviewView: URLPreviewViewModelViewDelegate { - func urlPreviewViewModel(_ viewModel: URLPreviewViewModelType, didUpdateViewState viewState: URLPreviewViewState) { - DispatchQueue.main.async { - switch viewState { - case .loading(let url): - self.renderLoading(url) - case .loaded(let preview): - self.renderLoaded(preview) - case .error(let error): - self.renderError(error) - case .hidden: - self.frame.size.height = 0 - } - } + guard let preview = preview else { return } + delegate?.didCloseURLPreviewView(self, for: preview.eventID, in: preview.roomID) } } diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib index 53ce2c0d60..cdb7e7ce57 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib @@ -14,7 +14,7 @@ - + @@ -23,7 +23,6 @@ - @@ -34,35 +33,48 @@ - - + + + + + + + + - - + + + + + + + + + + + + - - - - - - - - - - - - - - - - + + + + + + - - - - - - - - - - - - - - + + + + + + - + + - + - - - From 9fb13b7c5a0079e61f605af2ebdfdfab96ec0c4c Mon Sep 17 00:00:00 2001 From: Doug Date: Thu, 2 Sep 2021 12:41:55 +0100 Subject: [PATCH 07/28] Update layout for text only previews. --- .../Views/URLPreviews/URLPreviewView.swift | 42 +++++++++++-------- .../Room/Views/URLPreviews/URLPreviewView.xib | 20 +++++---- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift index e5b02a52d2..f627b0062a 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift @@ -51,6 +51,15 @@ class URLPreviewView: UIView, NibLoadable, Themable { @IBOutlet weak var titleLabel: UILabel! @IBOutlet weak var descriptionLabel: UILabel! + // Matches the label's height with the close button. + // Use a strong reference to keep it around when deactivating. + @IBOutlet var siteNameLabelHeightConstraint: NSLayoutConstraint! + + private var hasTitle: Bool { + guard let title = titleLabel.text else { return false } + return !title.isEmpty + } + // MARK: - Setup static func instantiate() -> Self { @@ -118,29 +127,28 @@ class URLPreviewView: UIView, NibLoadable, Themable { } private func renderLoaded(_ preview: URLPreviewData) { - if let image = preview.image { - imageView.image = image - showImageContainer() - } else { - imageView.image = nil - hideImageContainer() - } - + imageView.image = preview.image siteNameLabel.text = preview.siteName ?? preview.url.host titleLabel.text = preview.title descriptionLabel.text = preview.text - } - - private func showImageContainer() { - imageView.isHidden = false - // TODO: Adjust spacing of site name label + updateLayout() } - private func hideImageContainer() { - imageView.isHidden = true - - // TODO: Adjust spacing of site name label + private func updateLayout() { + if imageView.image == nil { + imageView.isHidden = true + + // tweak the layout of labels + siteNameLabelHeightConstraint.isActive = true + descriptionLabel.numberOfLines = hasTitle ? 3 : 5 + } else { + imageView.isHidden = false + + // tweak the layout of labels + siteNameLabelHeightConstraint.isActive = false + descriptionLabel.numberOfLines = 2 + } } // MARK: - Action diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib index defe897197..05b19e57bd 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib @@ -10,11 +10,11 @@ - + - + @@ -23,25 +23,28 @@ - + - + From 1831b61b126edec222429122410c2b03f3ab5301 Mon Sep 17 00:00:00 2001 From: Doug Date: Thu, 2 Sep 2021 17:37:48 +0100 Subject: [PATCH 08/28] Show an activity indicator until the preview has loaded. --- .../Room/CellData/RoomBubbleCellData.h | 7 ++- .../Room/CellData/RoomBubbleCellData.m | 5 +- .../Modules/Room/DataSources/RoomDataSource.m | 7 +-- .../RoomIncomingTextMsgBubbleCell.m | 2 +- ...mingTextMsgWithPaginationTitleBubbleCell.m | 2 +- ...comingTextMsgWithoutSenderInfoBubbleCell.m | 2 +- .../RoomOutgoingTextMsgBubbleCell.m | 2 +- ...tgoingTextMsgWithoutSenderInfoBubbleCell.m | 2 +- .../Views/URLPreviews/URLPreviewView.swift | 42 ++++++++++++---- .../Room/Views/URLPreviews/URLPreviewView.xib | 49 ++++++++++++++----- 10 files changed, 88 insertions(+), 32 deletions(-) diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.h b/Riot/Modules/Room/CellData/RoomBubbleCellData.h index f4f7e8078c..1e55e84468 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.h +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.h @@ -82,10 +82,15 @@ typedef NS_ENUM(NSInteger, RoomBubbleCellDataTag) @property(nonatomic, readonly) CGFloat additionalContentHeight; /** - A link if the textMessage contains one, otherwise nil. + The data necessary to show a URL preview. */ @property (nonatomic) URLPreviewData *urlPreviewData; +/** + Whether a URL preview should be displayed for this cell. + */ +@property (nonatomic) BOOL showURLPreview; + /** MXKeyVerification object associated to key verification event when using key verification by direct message. */ diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index 5ddb7c4e8b..0ef615ad1c 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -1070,8 +1070,9 @@ - (void)loadURLPreview return; } - // Check that the preview hasn't been dismissed already. - if ([URLPreviewManager.shared hasClosedPreviewFrom:lastComponent.event]) + // Don't show the preview if it has been dismissed already. + self.showURLPreview = ![URLPreviewManager.shared hasClosedPreviewFrom:lastComponent.event]; + if (!self.showURLPreview) { return; } diff --git a/Riot/Modules/Room/DataSources/RoomDataSource.m b/Riot/Modules/Room/DataSources/RoomDataSource.m index b379830c91..c4fce1cdd1 100644 --- a/Riot/Modules/Room/DataSources/RoomDataSource.m +++ b/Riot/Modules/Room/DataSources/RoomDataSource.m @@ -388,8 +388,8 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N NSURL *link = component.link; URLPreviewView *urlPreviewView; - // Encrypted rooms must not show URL previews. - if (link && cellData.urlPreviewData && !self.room.summary.isEncrypted) + // Show a URL preview if the component has a link that should be previewed. + if (link && cellData.showURLPreview) { urlPreviewView = [URLPreviewView instantiate]; urlPreviewView.preview = cellData.urlPreviewData; @@ -1261,7 +1261,8 @@ - (void)didCloseURLPreviewView:(URLPreviewView *)previewView for:(NSString *)eve // Remember that the user closed the preview so it isn't shown again. [URLPreviewManager.shared closePreviewFor:eventID in:roomID]; - // Remove the preview data and refresh the cells. + // Hide the preview, remove its data and refresh the cells. + cellData.showURLPreview = NO; cellData.urlPreviewData = nil; [self refreshCells]; } diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m index e7bb2fbce6..cb6bef798e 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m @@ -43,7 +43,7 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.urlPreviewData) + if (bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m index 3d88949e40..a1cf37c789 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m @@ -49,7 +49,7 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.urlPreviewData) + if (bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m index 3b8c32635f..219c18f773 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m @@ -33,7 +33,7 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.urlPreviewData) + if (bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m index 9a1a6efa4d..55210fcd5b 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m @@ -44,7 +44,7 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.urlPreviewData) + if (bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m index 82b54239cc..b8de02021d 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m @@ -33,7 +33,7 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.urlPreviewData) + if (bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift index f627b0062a..a34b06e678 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift @@ -37,7 +37,10 @@ class URLPreviewView: UIView, NibLoadable, Themable { var preview: URLPreviewData? { didSet { - guard let preview = preview else { return } + guard let preview = preview else { + renderLoading() + return + } renderLoaded(preview) } } @@ -47,10 +50,15 @@ class URLPreviewView: UIView, NibLoadable, Themable { @IBOutlet weak var imageView: UIImageView! @IBOutlet weak var closeButton: UIButton! + @IBOutlet weak var textContainerView: UIView! @IBOutlet weak var siteNameLabel: UILabel! @IBOutlet weak var titleLabel: UILabel! @IBOutlet weak var descriptionLabel: UILabel! + @IBOutlet weak var loadingView: UIView! + @IBOutlet weak var loadingLabel: UILabel! + @IBOutlet weak var loadingActivityIndicator: UIActivityIndicatorView! + // Matches the label's height with the close button. // Use a strong reference to keep it around when deactivating. @IBOutlet var siteNameLabelHeightConstraint: NSLayoutConstraint! @@ -98,14 +106,21 @@ class URLPreviewView: UIView, NibLoadable, Themable { descriptionLabel.textColor = theme.colors.secondaryContent descriptionLabel.font = theme.fonts.caption1 + loadingLabel.textColor = siteNameLabel.textColor + let closeButtonAsset = ThemeService.shared().isCurrentThemeDark() ? Asset.Images.urlPreviewCloseDark : Asset.Images.urlPreviewClose closeButton.setImage(closeButtonAsset.image, for: .normal) } - static func contentViewHeight(for preview: URLPreviewData) -> CGFloat { + static func contentViewHeight(for preview: URLPreviewData?) -> CGFloat { sizingView.frame = CGRect(x: 0, y: 0, width: Constants.width, height: 1) - sizingView.renderLoaded(preview) + // Call render directly to avoid storing the preview data in the sizing view + if let preview = preview { + sizingView.renderLoaded(preview) + } else { + sizingView.renderLoading() + } sizingView.setNeedsLayout() sizingView.layoutIfNeeded() @@ -117,16 +132,18 @@ class URLPreviewView: UIView, NibLoadable, Themable { } // MARK: - Private - #warning("Check whether we should show a loading state.") - private func renderLoading(_ url: URL) { - imageView.image = nil + private func renderLoading() { + // hide the content + imageView.isHidden = true + textContainerView.isHidden = true - siteNameLabel.text = url.host - titleLabel.text = "Loading..." - descriptionLabel.text = "" + // show the loading interface + loadingView.isHidden = false + loadingActivityIndicator.startAnimating() } private func renderLoaded(_ preview: URLPreviewData) { + // update preview content imageView.image = preview.image siteNameLabel.text = preview.siteName ?? preview.url.host titleLabel.text = preview.title @@ -136,6 +153,13 @@ class URLPreviewView: UIView, NibLoadable, Themable { } private func updateLayout() { + // hide the loading interface + loadingView.isHidden = true + loadingActivityIndicator.stopAnimating() + + // show the content + textContainerView.isHidden = false + if imageView.image == nil { imageView.isHidden = true diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib index 05b19e57bd..1675f2a6d8 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib @@ -10,11 +10,11 @@ - + - + @@ -22,11 +22,11 @@ - - + + - + - - - - @@ -64,6 +60,31 @@ + + + + + + + + + + + + + + + + + @@ -92,12 +113,16 @@ + + + + - + From 6a5b12ab8bbc2f6b67e5010f69154a5160562fcc Mon Sep 17 00:00:00 2001 From: Doug Date: Thu, 2 Sep 2021 18:08:35 +0100 Subject: [PATCH 09/28] Ensure correct font is used. --- Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift index a34b06e678..824549cb47 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift @@ -107,6 +107,7 @@ class URLPreviewView: UIView, NibLoadable, Themable { descriptionLabel.font = theme.fonts.caption1 loadingLabel.textColor = siteNameLabel.textColor + loadingLabel.font = siteNameLabel.font let closeButtonAsset = ThemeService.shared().isCurrentThemeDark() ? Asset.Images.urlPreviewCloseDark : Asset.Images.urlPreviewClose closeButton.setImage(closeButtonAsset.image, for: .normal) From 2e041233371bb3b8a291252f9baa0a1f3c0a5485 Mon Sep 17 00:00:00 2001 From: Doug Date: Fri, 3 Sep 2021 10:19:26 +0100 Subject: [PATCH 10/28] Add setting to disable URL previews. Using a temporary position in the settings screen whilst waiting for feedback. --- Config/CommonConfiguration.swift | 3 +++ Riot/Assets/en.lproj/Vector.strings | 2 ++ Riot/Generated/Strings.swift | 4 ++++ Riot/Managers/Settings/RiotSettings.swift | 3 +++ .../Modules/Room/DataSources/RoomDataSource.m | 2 +- .../RoomIncomingTextMsgBubbleCell.m | 3 ++- ...mingTextMsgWithPaginationTitleBubbleCell.m | 3 ++- ...comingTextMsgWithoutSenderInfoBubbleCell.m | 3 ++- .../RoomOutgoingTextMsgBubbleCell.m | 3 ++- ...tgoingTextMsgWithoutSenderInfoBubbleCell.m | 3 ++- .../Modules/Settings/SettingsViewController.m | 19 +++++++++++++++++++ 11 files changed, 42 insertions(+), 6 deletions(-) diff --git a/Config/CommonConfiguration.swift b/Config/CommonConfiguration.swift index e582f546af..b6aa77dade 100644 --- a/Config/CommonConfiguration.swift +++ b/Config/CommonConfiguration.swift @@ -49,6 +49,9 @@ class CommonConfiguration: NSObject, Configurable { settings.messageDetailsAllowCopyingMedia = BuildSettings.messageDetailsAllowCopyMedia settings.messageDetailsAllowPastingMedia = BuildSettings.messageDetailsAllowPasteMedia + // Enable link detection if url preview are enabled + settings.enableBubbleComponentLinkDetection = true + MXKContactManager.shared().allowLocalContactsAccess = BuildSettings.allowLocalContactsAccess } diff --git a/Riot/Assets/en.lproj/Vector.strings b/Riot/Assets/en.lproj/Vector.strings index d7cad47ca3..34138c74d8 100644 --- a/Riot/Assets/en.lproj/Vector.strings +++ b/Riot/Assets/en.lproj/Vector.strings @@ -537,6 +537,8 @@ Tap the + to start adding people."; "settings_ui_theme_picker_message_invert_colours" = "\"Auto\" uses your device's \"Invert Colours\" settings"; "settings_ui_theme_picker_message_match_system_theme" = "\"Auto\" matches your device's system theme"; +"settings_show_url_previews" = "Show inline URL previews"; + "settings_unignore_user" = "Show all messages from %@?"; "settings_contacts_discover_matrix_users" = "Use emails and phone numbers to discover users"; diff --git a/Riot/Generated/Strings.swift b/Riot/Generated/Strings.swift index d0ec69c4fa..4cc79276fd 100644 --- a/Riot/Generated/Strings.swift +++ b/Riot/Generated/Strings.swift @@ -4554,6 +4554,10 @@ internal enum VectorL10n { internal static var settingsShowNSFWPublicRooms: String { return VectorL10n.tr("Vector", "settings_show_NSFW_public_rooms") } + /// Show inline URL previews + internal static var settingsShowUrlPreviews: String { + return VectorL10n.tr("Vector", "settings_show_url_previews") + } /// Sign Out internal static var settingsSignOut: String { return VectorL10n.tr("Vector", "settings_sign_out") diff --git a/Riot/Managers/Settings/RiotSettings.swift b/Riot/Managers/Settings/RiotSettings.swift index a268bd52b0..b33ba0a759 100644 --- a/Riot/Managers/Settings/RiotSettings.swift +++ b/Riot/Managers/Settings/RiotSettings.swift @@ -160,6 +160,9 @@ final class RiotSettings: NSObject { @UserDefault(key: "roomScreenAllowFilesAction", defaultValue: BuildSettings.roomScreenAllowFilesAction, storage: defaults) var roomScreenAllowFilesAction + @UserDefault(key: "roomScreenShowsURLPreviews", defaultValue: true, storage: defaults) + var roomScreenShowsURLPreviews + // MARK: - Room Contextual Menu @UserDefault(key: "roomContextualMenuShowMoreOptionForMessages", defaultValue: BuildSettings.roomContextualMenuShowMoreOptionForMessages, storage: defaults) diff --git a/Riot/Modules/Room/DataSources/RoomDataSource.m b/Riot/Modules/Room/DataSources/RoomDataSource.m index c4fce1cdd1..85607fede8 100644 --- a/Riot/Modules/Room/DataSources/RoomDataSource.m +++ b/Riot/Modules/Room/DataSources/RoomDataSource.m @@ -389,7 +389,7 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N URLPreviewView *urlPreviewView; // Show a URL preview if the component has a link that should be previewed. - if (link && cellData.showURLPreview) + if (link && RiotSettings.shared.roomScreenShowsURLPreviews && cellData.showURLPreview) { urlPreviewView = [URLPreviewView instantiate]; urlPreviewView.preview = cellData.urlPreviewData; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m index cb6bef798e..542f20a445 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m @@ -43,7 +43,8 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.showURLPreview) + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m index a1cf37c789..af187513bf 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m @@ -49,7 +49,8 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.showURLPreview) + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m index 219c18f773..19fb4a5cd3 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m @@ -33,7 +33,8 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.showURLPreview) + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m index 55210fcd5b..94bd5c3101 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m @@ -44,7 +44,8 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.showURLPreview) + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m index b8de02021d..0c444d51fb 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m @@ -33,7 +33,8 @@ + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)m { RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - if (bubbleData && bubbleData.showURLPreview) + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) { CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; diff --git a/Riot/Modules/Settings/SettingsViewController.m b/Riot/Modules/Settings/SettingsViewController.m index 34e0aa3291..178075fd1e 100644 --- a/Riot/Modules/Settings/SettingsViewController.m +++ b/Riot/Modules/Settings/SettingsViewController.m @@ -120,6 +120,7 @@ { USER_INTERFACE_LANGUAGE_INDEX = 0, USER_INTERFACE_THEME_INDEX, + USER_INTERFACE_SHOW_URL_PREVIEWS_INDEX, }; enum @@ -461,6 +462,7 @@ - (void)updateSections Section *sectionUserInterface = [Section sectionWithTag:SECTION_TAG_USER_INTERFACE]; [sectionUserInterface addRowWithTag:USER_INTERFACE_LANGUAGE_INDEX]; [sectionUserInterface addRowWithTag:USER_INTERFACE_THEME_INDEX]; + [sectionUserInterface addRowWithTag:USER_INTERFACE_SHOW_URL_PREVIEWS_INDEX]; sectionUserInterface.headerTitle = NSLocalizedStringFromTable(@"settings_user_interface", @"Vector", nil); [tmpSections addObject: sectionUserInterface]; @@ -2104,6 +2106,18 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N [cell vc_setAccessoryDisclosureIndicatorWithCurrentTheme]; cell.selectionStyle = UITableViewCellSelectionStyleDefault; } + else if (row == USER_INTERFACE_SHOW_URL_PREVIEWS_INDEX) + { + MXKTableViewCellWithLabelAndSwitch *labelAndSwitchCell = [self getLabelAndSwitchCell:tableView forIndexPath:indexPath]; + + labelAndSwitchCell.mxkLabel.text = NSLocalizedStringFromTable(@"settings_show_url_previews", @"Vector", nil); + labelAndSwitchCell.mxkSwitch.on = RiotSettings.shared.roomScreenShowsURLPreviews; + labelAndSwitchCell.mxkSwitch.onTintColor = ThemeService.shared.theme.tintColor; + + [labelAndSwitchCell.mxkSwitch addTarget:self action:@selector(toggleEnableURLPreviews:) forControlEvents:UIControlEventValueChanged]; + + cell = labelAndSwitchCell; + } } else if (section == SECTION_TAG_IGNORED_USERS) { @@ -3038,6 +3052,11 @@ - (void)toggleLocalContactsSync:(UISwitch *)sender } } +- (void)toggleEnableURLPreviews:(UISwitch *)sender +{ + RiotSettings.shared.roomScreenShowsURLPreviews = sender.on; +} + - (void)toggleSendCrashReport:(id)sender { BOOL enable = RiotSettings.shared.enableCrashReport; From be83d8e2f6fb0b295190196bae41ece1819486e1 Mon Sep 17 00:00:00 2001 From: Doug Date: Fri, 3 Sep 2021 11:21:07 +0100 Subject: [PATCH 11/28] Fix edits to previewable links not working. --- .../Room/CellData/RoomBubbleCellData.m | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index 0ef615ad1c..4c5a747153 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -190,6 +190,19 @@ - (instancetype)initWithEvent:(MXEvent *)event andRoomState:(MXRoomState *)roomS return self; } +- (NSUInteger)updateEvent:(NSString *)eventId withEvent:(MXEvent *)event +{ + NSUInteger retVal = [super updateEvent:eventId withEvent:event]; + + // Update any URL preview data too. + if (self.hasLink) + { + [self loadURLPreview]; + } + + return retVal; +} + - (void)prepareBubbleComponentsPosition { if (shouldUpdateComponentsPosition) @@ -1077,6 +1090,13 @@ - (void)loadURLPreview return; } + // If there is existing preview data, the message has been edited + // Clear the data to show the loading state when the preview isn't cached + if (self.urlPreviewData) + { + self.urlPreviewData = nil; + } + // Set the preview data. MXWeakify(self); @@ -1088,7 +1108,10 @@ - (void)loadURLPreview // Update the preview data and send a notification for refresh self.urlPreviewData = urlPreviewData; - [NSNotificationCenter.defaultCenter postNotificationName:URLPreviewDidUpdateNotification object:self]; + + dispatch_async(dispatch_get_main_queue(), ^{ + [NSNotificationCenter.defaultCenter postNotificationName:URLPreviewDidUpdateNotification object:self]; + }); } failure:^(NSError * _Nullable error) { MXLogDebug(@"[RoomBubbleCellData] Failed to get url preview") From 80f8cc6dac8eeabd4509c5b38a1f61a57f431e66 Mon Sep 17 00:00:00 2001 From: Doug Date: Fri, 3 Sep 2021 11:32:09 +0100 Subject: [PATCH 12/28] Hide the loading state on error. --- Riot/Modules/Room/CellData/RoomBubbleCellData.m | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index 4c5a747153..bdbb241abb 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -1115,6 +1115,13 @@ - (void)loadURLPreview } failure:^(NSError * _Nullable error) { MXLogDebug(@"[RoomBubbleCellData] Failed to get url preview") + + // Don't show a preview and send a notification for refresh + self.showURLPreview = NO; + + dispatch_async(dispatch_get_main_queue(), ^{ + [NSNotificationCenter.defaultCenter postNotificationName:URLPreviewDidUpdateNotification object:self]; + }); }]; } From 7db81ccf2aa10740a31084207d20d5e9c291e06c Mon Sep 17 00:00:00 2001 From: Doug Date: Fri, 3 Sep 2021 11:52:57 +0100 Subject: [PATCH 13/28] Break-up cell data after a link even if the new event isn't a message. --- .../Room/CellData/RoomBubbleCellData.m | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index bdbb241abb..5a9890b310 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -767,6 +767,19 @@ - (BOOL)addEvent:(MXEvent*)event andRoomState:(MXRoomState*)roomState { BOOL shouldAddEvent = YES; + // For unencrypted rooms, don't allow any events to be added + // after a bubble component that contains a link so than any URL + // preview is for the last bubble component in the cell. + if (!self.isEncryptedRoom && self.hasLink && self.bubbleComponents.lastObject) + { + MXKRoomBubbleComponent *lastComponent = self.bubbleComponents.lastObject; + + if (event.originServerTs > lastComponent.event.originServerTs) + { + shouldAddEvent = NO; + } + } + switch (self.tag) { case RoomBubbleCellDataTagKeyVerificationNoDisplay: @@ -813,29 +826,14 @@ - (BOOL)addEvent:(MXEvent*)event andRoomState:(MXRoomState*)roomState break; } - if (self.bubbleComponents.lastObject) - { - MXKRoomBubbleComponent *lastComponent = self.bubbleComponents.lastObject; - // If the new event comes after the last bubble component - if (event.originServerTs > lastComponent.event.originServerTs) - { - // FIXME: This should be for all event types, not just messages. - // Don't add it if there is already a link in the cell data - if (self.hasLink && !self.isEncryptedRoom) - { - shouldAddEvent = NO; - } - break; - } - } - - if (self.bubbleComponents.firstObject) + // If the message contains a link and comes before this cell data, don't add it to + // ensure that a URL preview is only shown for the last component on some new cell data. + if (!self.isEncryptedRoom && self.bubbleComponents.firstObject) { MXKRoomBubbleComponent *firstComponent = self.bubbleComponents.firstObject; - // If the new event event comes before the first bubble component + if (event.originServerTs < firstComponent.event.originServerTs) { - // Don't add it to the cell data if it contains a link NSString *messageBody = event.content[@"body"]; if (messageBody && [messageBody mxk_firstURLDetected]) { From 434657ea6b2e5a96078e5bf6e2c8d8f88026182c Mon Sep 17 00:00:00 2001 From: Doug Date: Fri, 3 Sep 2021 12:07:29 +0100 Subject: [PATCH 14/28] Fix reactions beneath URL previews. --- Riot/Modules/Room/DataSources/RoomDataSource.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Riot/Modules/Room/DataSources/RoomDataSource.m b/Riot/Modules/Room/DataSources/RoomDataSource.m index 85607fede8..4b23664404 100644 --- a/Riot/Modules/Room/DataSources/RoomDataSource.m +++ b/Riot/Modules/Room/DataSources/RoomDataSource.m @@ -462,14 +462,14 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N } // The top constraint may need to include the URL preview view - CGFloat topConstraintConstant; + NSLayoutConstraint *topConstraint; if (urlPreviewView) { - topConstraintConstant = bottomPositionY + RoomBubbleCellLayout.urlPreviewViewTopMargin + urlPreviewView.frame.size.height + RoomBubbleCellLayout.reactionsViewTopMargin; + topConstraint = [reactionsView.topAnchor constraintEqualToAnchor:urlPreviewView.bottomAnchor constant:RoomBubbleCellLayout.reactionsViewTopMargin]; } else { - topConstraintConstant = bottomPositionY + RoomBubbleCellLayout.reactionsViewTopMargin; + topConstraint = [reactionsView.topAnchor constraintEqualToAnchor:reactionsView.superview.topAnchor constant:bottomPositionY + RoomBubbleCellLayout.reactionsViewTopMargin]; } // Force receipts container size @@ -477,7 +477,7 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N @[ [reactionsView.leadingAnchor constraintEqualToAnchor:reactionsView.superview.leadingAnchor constant:leftMargin], [reactionsView.trailingAnchor constraintEqualToAnchor:reactionsView.superview.trailingAnchor constant:-RoomBubbleCellLayout.reactionsViewRightMargin], - [reactionsView.topAnchor constraintEqualToAnchor:reactionsView.superview.topAnchor constant:topConstraintConstant] + topConstraint ]]; } } From cf3733c8b94d435771f765ce1b966f881d8dcd5b Mon Sep 17 00:00:00 2001 From: Doug Date: Fri, 3 Sep 2021 12:12:44 +0100 Subject: [PATCH 15/28] Clear the URL preview manager's store when clearing caches. --- Riot/Modules/Application/LegacyAppDelegate.m | 1 + 1 file changed, 1 insertion(+) diff --git a/Riot/Modules/Application/LegacyAppDelegate.m b/Riot/Modules/Application/LegacyAppDelegate.m index 917e13f8a6..c6e2a175d4 100644 --- a/Riot/Modules/Application/LegacyAppDelegate.m +++ b/Riot/Modules/Application/LegacyAppDelegate.m @@ -4328,6 +4328,7 @@ - (void)clearCache [MXMediaManager clearCache]; [MXKAttachment clearCache]; [VoiceMessageAttachmentCacheManagerBridge clearCache]; + [URLPreviewManager.shared clearStore]; } @end From 55df9303de56c97c6d35eddaabe355e3cec55224 Mon Sep 17 00:00:00 2001 From: Doug Date: Fri, 3 Sep 2021 18:18:36 +0100 Subject: [PATCH 16/28] Fix potentially redundant table reloading. --- Riot/Modules/Room/CellData/RoomBubbleCellData.m | 9 +++++++-- Riot/Modules/Room/DataSources/RoomDataSource.m | 13 ++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index 5a9890b310..7339db8e86 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -1098,6 +1098,11 @@ - (void)loadURLPreview // Set the preview data. MXWeakify(self); + NSDictionary *userInfo = @{ + @"eventId": lastComponent.event.eventId, + @"roomId": self.roomId + }; + [URLPreviewManager.shared previewFor:lastComponent.link and:lastComponent.event with:self.mxSession @@ -1108,7 +1113,7 @@ - (void)loadURLPreview self.urlPreviewData = urlPreviewData; dispatch_async(dispatch_get_main_queue(), ^{ - [NSNotificationCenter.defaultCenter postNotificationName:URLPreviewDidUpdateNotification object:self]; + [NSNotificationCenter.defaultCenter postNotificationName:URLPreviewDidUpdateNotification object:nil userInfo:userInfo]; }); } failure:^(NSError * _Nullable error) { @@ -1118,7 +1123,7 @@ - (void)loadURLPreview self.showURLPreview = NO; dispatch_async(dispatch_get_main_queue(), ^{ - [NSNotificationCenter.defaultCenter postNotificationName:URLPreviewDidUpdateNotification object:self]; + [NSNotificationCenter.defaultCenter postNotificationName:URLPreviewDidUpdateNotification object:nil userInfo:userInfo]; }); }]; } diff --git a/Riot/Modules/Room/DataSources/RoomDataSource.m b/Riot/Modules/Room/DataSources/RoomDataSource.m index 4b23664404..b121536736 100644 --- a/Riot/Modules/Room/DataSources/RoomDataSource.m +++ b/Riot/Modules/Room/DataSources/RoomDataSource.m @@ -97,10 +97,17 @@ - (instancetype)initWithRoomId:(NSString *)roomId andMatrixSession:(MXSession *) }]; // Observe URL preview updates. - kURLPreviewDidUpdateNotificationObserver = [NSNotificationCenter.defaultCenter addObserverForName:URLPreviewDidUpdateNotification object:nil queue:NSOperationQueue.mainQueue usingBlock:^(NSNotification * _Nonnull note) { + kURLPreviewDidUpdateNotificationObserver = [NSNotificationCenter.defaultCenter addObserverForName:URLPreviewDidUpdateNotification object:nil queue:NSOperationQueue.mainQueue usingBlock:^(NSNotification * _Nonnull notification) { - // Refresh all cells. - [self refreshCells]; + if (![(NSString*)notification.userInfo[@"roomId"] isEqualToString:self.roomId] || !self.delegate) + { + return; + } + + // Refresh the updated cell. + // Note - it doesn't appear as though MXKRoomViewController actually uses the index path. + NSInteger index = [self indexOfCellDataWithEventId:(NSString*)notification.userInfo[@"eventId"]]; + [self.delegate dataSource:self didCellChange:[NSIndexPath indexPathWithIndex:index]]; }]; [self registerKeyVerificationRequestNotification]; From 7448ca1002fe6e082637647a34eded35ea25494b Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 7 Sep 2021 11:20:59 +0100 Subject: [PATCH 17/28] Observe URL preview update notification in RoomViewController. Update bubbleTableView's content offset when a preview above the bottom most visible cell changes the height of the table's content. --- .../Modules/Room/DataSources/RoomDataSource.m | 23 --------- Riot/Modules/Room/RoomViewController.m | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/Riot/Modules/Room/DataSources/RoomDataSource.m b/Riot/Modules/Room/DataSources/RoomDataSource.m index b121536736..c76303fb6b 100644 --- a/Riot/Modules/Room/DataSources/RoomDataSource.m +++ b/Riot/Modules/Room/DataSources/RoomDataSource.m @@ -33,9 +33,6 @@ @interface RoomDataSource() Date: Tue, 7 Sep 2021 12:58:45 +0100 Subject: [PATCH 18/28] Fix unsatisfiable constraints messages. --- Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift index 824549cb47..2dc06e7a66 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift @@ -73,6 +73,7 @@ class URLPreviewView: UIView, NibLoadable, Themable { static func instantiate() -> Self { let view = Self.loadFromNib() view.update(theme: ThemeService.shared().theme) + view.translatesAutoresizingMaskIntoConstraints = false // fixes unsatisfiable constraints encountered by the sizing view return view } From 0094add5a4de61ea97c8509fa0564755f1b209ca Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 7 Sep 2021 16:00:12 +0100 Subject: [PATCH 19/28] Move url preview setting under labs section. --- Riot/Assets/en.lproj/Vector.strings | 1 + Riot/Generated/Strings.swift | 4 ++ Riot/Managers/Settings/RiotSettings.swift | 3 +- .../Modules/Settings/SettingsViewController.m | 41 ++++++++++++------- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/Riot/Assets/en.lproj/Vector.strings b/Riot/Assets/en.lproj/Vector.strings index 34138c74d8..2671873d81 100644 --- a/Riot/Assets/en.lproj/Vector.strings +++ b/Riot/Assets/en.lproj/Vector.strings @@ -538,6 +538,7 @@ Tap the + to start adding people."; "settings_ui_theme_picker_message_match_system_theme" = "\"Auto\" matches your device's system theme"; "settings_show_url_previews" = "Show inline URL previews"; +"settings_show_url_previews_description" = "Previews will only be shown in unencrypted rooms."; "settings_unignore_user" = "Show all messages from %@?"; diff --git a/Riot/Generated/Strings.swift b/Riot/Generated/Strings.swift index 4cc79276fd..83a3afc120 100644 --- a/Riot/Generated/Strings.swift +++ b/Riot/Generated/Strings.swift @@ -4558,6 +4558,10 @@ internal enum VectorL10n { internal static var settingsShowUrlPreviews: String { return VectorL10n.tr("Vector", "settings_show_url_previews") } + /// Previews will only be shown in unencrypted rooms. + internal static var settingsShowUrlPreviewsDescription: String { + return VectorL10n.tr("Vector", "settings_show_url_previews_description") + } /// Sign Out internal static var settingsSignOut: String { return VectorL10n.tr("Vector", "settings_sign_out") diff --git a/Riot/Managers/Settings/RiotSettings.swift b/Riot/Managers/Settings/RiotSettings.swift index b33ba0a759..b850114acf 100644 --- a/Riot/Managers/Settings/RiotSettings.swift +++ b/Riot/Managers/Settings/RiotSettings.swift @@ -160,7 +160,8 @@ final class RiotSettings: NSObject { @UserDefault(key: "roomScreenAllowFilesAction", defaultValue: BuildSettings.roomScreenAllowFilesAction, storage: defaults) var roomScreenAllowFilesAction - @UserDefault(key: "roomScreenShowsURLPreviews", defaultValue: true, storage: defaults) + // labs prefix added to the key can be dropped when default value becomes true + @UserDefault(key: "labsRoomScreenShowsURLPreviews", defaultValue: false, storage: defaults) var roomScreenShowsURLPreviews // MARK: - Room Contextual Menu diff --git a/Riot/Modules/Settings/SettingsViewController.m b/Riot/Modules/Settings/SettingsViewController.m index 178075fd1e..5ecaac55b0 100644 --- a/Riot/Modules/Settings/SettingsViewController.m +++ b/Riot/Modules/Settings/SettingsViewController.m @@ -119,8 +119,7 @@ enum { USER_INTERFACE_LANGUAGE_INDEX = 0, - USER_INTERFACE_THEME_INDEX, - USER_INTERFACE_SHOW_URL_PREVIEWS_INDEX, + USER_INTERFACE_THEME_INDEX }; enum @@ -148,6 +147,8 @@ enum { LABS_ENABLE_RINGING_FOR_GROUP_CALLS_INDEX = 0, + LABS_SHOW_URL_PREVIEWS_INDEX, + LABS_SHOW_URL_PREVIEWS_DESCRIPTION_INDEX }; enum @@ -462,7 +463,6 @@ - (void)updateSections Section *sectionUserInterface = [Section sectionWithTag:SECTION_TAG_USER_INTERFACE]; [sectionUserInterface addRowWithTag:USER_INTERFACE_LANGUAGE_INDEX]; [sectionUserInterface addRowWithTag:USER_INTERFACE_THEME_INDEX]; - [sectionUserInterface addRowWithTag:USER_INTERFACE_SHOW_URL_PREVIEWS_INDEX]; sectionUserInterface.headerTitle = NSLocalizedStringFromTable(@"settings_user_interface", @"Vector", nil); [tmpSections addObject: sectionUserInterface]; @@ -516,6 +516,8 @@ - (void)updateSections { Section *sectionLabs = [Section sectionWithTag:SECTION_TAG_LABS]; [sectionLabs addRowWithTag:LABS_ENABLE_RINGING_FOR_GROUP_CALLS_INDEX]; + [sectionLabs addRowWithTag:LABS_SHOW_URL_PREVIEWS_INDEX]; + [sectionLabs addRowWithTag:LABS_SHOW_URL_PREVIEWS_DESCRIPTION_INDEX]; sectionLabs.headerTitle = NSLocalizedStringFromTable(@"settings_labs", @"Vector", nil); if (sectionLabs.hasAnyRows) { @@ -2106,18 +2108,6 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N [cell vc_setAccessoryDisclosureIndicatorWithCurrentTheme]; cell.selectionStyle = UITableViewCellSelectionStyleDefault; } - else if (row == USER_INTERFACE_SHOW_URL_PREVIEWS_INDEX) - { - MXKTableViewCellWithLabelAndSwitch *labelAndSwitchCell = [self getLabelAndSwitchCell:tableView forIndexPath:indexPath]; - - labelAndSwitchCell.mxkLabel.text = NSLocalizedStringFromTable(@"settings_show_url_previews", @"Vector", nil); - labelAndSwitchCell.mxkSwitch.on = RiotSettings.shared.roomScreenShowsURLPreviews; - labelAndSwitchCell.mxkSwitch.onTintColor = ThemeService.shared.theme.tintColor; - - [labelAndSwitchCell.mxkSwitch addTarget:self action:@selector(toggleEnableURLPreviews:) forControlEvents:UIControlEventValueChanged]; - - cell = labelAndSwitchCell; - } } else if (section == SECTION_TAG_IGNORED_USERS) { @@ -2366,6 +2356,27 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N cell = labelAndSwitchCell; } + else if (row == LABS_SHOW_URL_PREVIEWS_INDEX) + { + MXKTableViewCellWithLabelAndSwitch *labelAndSwitchCell = [self getLabelAndSwitchCell:tableView forIndexPath:indexPath]; + + labelAndSwitchCell.mxkLabel.text = NSLocalizedStringFromTable(@"settings_show_url_previews", @"Vector", nil); + labelAndSwitchCell.mxkSwitch.on = RiotSettings.shared.roomScreenShowsURLPreviews; + labelAndSwitchCell.mxkSwitch.onTintColor = ThemeService.shared.theme.tintColor; + + [labelAndSwitchCell.mxkSwitch addTarget:self action:@selector(toggleEnableURLPreviews:) forControlEvents:UIControlEventValueChanged]; + + cell = labelAndSwitchCell; + } + else if (row == LABS_SHOW_URL_PREVIEWS_DESCRIPTION_INDEX) + { + MXKTableViewCell *descriptionCell = [self getDefaultTableViewCell:tableView]; + descriptionCell.textLabel.text = NSLocalizedStringFromTable(@"settings_show_url_previews_description", @"Vector", nil); + descriptionCell.textLabel.numberOfLines = 0; + descriptionCell.selectionStyle = UITableViewCellSelectionStyleNone; + + cell = descriptionCell; + } } else if (section == SECTION_TAG_FLAIR) { From 4ad041622ce26f3797962198f4ee7be7e810a70d Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 7 Sep 2021 16:12:12 +0100 Subject: [PATCH 20/28] Remove "Loading preview..." label. --- .../Views/URLPreviews/URLPreviewView.swift | 4 ---- .../Room/Views/URLPreviews/URLPreviewView.xib | 19 ++++--------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift index 2dc06e7a66..ae7742af44 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift @@ -56,7 +56,6 @@ class URLPreviewView: UIView, NibLoadable, Themable { @IBOutlet weak var descriptionLabel: UILabel! @IBOutlet weak var loadingView: UIView! - @IBOutlet weak var loadingLabel: UILabel! @IBOutlet weak var loadingActivityIndicator: UIActivityIndicatorView! // Matches the label's height with the close button. @@ -107,9 +106,6 @@ class URLPreviewView: UIView, NibLoadable, Themable { descriptionLabel.textColor = theme.colors.secondaryContent descriptionLabel.font = theme.fonts.caption1 - loadingLabel.textColor = siteNameLabel.textColor - loadingLabel.font = siteNameLabel.font - let closeButtonAsset = ThemeService.shared().isCurrentThemeDark() ? Asset.Images.urlPreviewCloseDark : Asset.Images.urlPreviewClose closeButton.setImage(closeButtonAsset.image, for: .normal) } diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib index 1675f2a6d8..a65c9d1926 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.xib @@ -63,26 +63,16 @@ - - + - - - - + + + @@ -114,7 +104,6 @@ - From c007bc5b4df69a856ce4310925b123f16e7b082c Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 7 Sep 2021 16:12:37 +0100 Subject: [PATCH 21/28] Fix settings toggle not enabled. --- Riot/Modules/Settings/SettingsViewController.m | 1 + 1 file changed, 1 insertion(+) diff --git a/Riot/Modules/Settings/SettingsViewController.m b/Riot/Modules/Settings/SettingsViewController.m index 5ecaac55b0..bc1e800941 100644 --- a/Riot/Modules/Settings/SettingsViewController.m +++ b/Riot/Modules/Settings/SettingsViewController.m @@ -2363,6 +2363,7 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N labelAndSwitchCell.mxkLabel.text = NSLocalizedStringFromTable(@"settings_show_url_previews", @"Vector", nil); labelAndSwitchCell.mxkSwitch.on = RiotSettings.shared.roomScreenShowsURLPreviews; labelAndSwitchCell.mxkSwitch.onTintColor = ThemeService.shared.theme.tintColor; + labelAndSwitchCell.mxkSwitch.enabled = YES; [labelAndSwitchCell.mxkSwitch addTarget:self action:@selector(toggleEnableURLPreviews:) forControlEvents:UIControlEventValueChanged]; From 08d548c51a8f5a7bbe5aae030fadee228a285848 Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 7 Sep 2021 17:10:14 +0100 Subject: [PATCH 22/28] Add changelog entry. --- changelog.d/888.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/888.feature diff --git a/changelog.d/888.feature b/changelog.d/888.feature new file mode 100644 index 0000000000..5c869283d3 --- /dev/null +++ b/changelog.d/888.feature @@ -0,0 +1 @@ +Timeline: Add URL previews under a labs setting. From ea14ed961666a43e51ccadedc7294c4c4c19fff5 Mon Sep 17 00:00:00 2001 From: Doug Date: Wed, 8 Sep 2021 09:51:47 +0100 Subject: [PATCH 23/28] Add more docs and comments. Rename store.store(_:) to store.cache(_:). --- .../URLPreviews/URLPreviewManager.swift | 42 ++++++++++++++++--- .../URLPreviews/URLPreviewStore.swift | 20 +++++---- .../Modules/Room/DataSources/RoomDataSource.m | 2 + .../Views/URLPreviews/URLPreviewView.swift | 13 +++--- RiotTests/URLPreviewStoreTests.swift | 14 +++---- 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/Riot/Managers/URLPreviews/URLPreviewManager.swift b/Riot/Managers/URLPreviews/URLPreviewManager.swift index 6d46e2fe46..29357509b1 100644 --- a/Riot/Managers/URLPreviews/URLPreviewManager.swift +++ b/Riot/Managers/URLPreviews/URLPreviewManager.swift @@ -17,14 +17,26 @@ import Foundation @objcMembers +/// A manager for URL preview data to handle fetching, caching and clean-up +/// as well as remembering which previews have been closed by the user. class URLPreviewManager: NSObject { + /// The shared manager object. static let shared = URLPreviewManager() - // Core Data store to reduce network requests + /// A persistent store backed by Core Data to reduce network requests private let store = URLPreviewStore() private override init() { } + /// Generates preview data for a URL to be previewed as part of the supplied event, + /// first checking the cache, and if necessary making a request to the homeserver. + /// You should call `hasClosedPreview` first to ensure that a preview is required. + /// - Parameters: + /// - url: The URL to generate the preview for. + /// - event: The event that the preview is for. + /// - session: The session to use to contact the homeserver. + /// - success: The closure called when the operation complete. The generated preview data is passed in. + /// - failure: The closure called when something goes wrong. The error that occured is passed in. func preview(for url: URL, and event: MXEvent, with session: MXSession, @@ -33,18 +45,21 @@ class URLPreviewManager: NSObject { // Sanitize the URL before checking the store or performing lookup let sanitizedURL = sanitize(url) + // Check for a valid preview in the store, and use this if found if let preview = store.preview(for: sanitizedURL, and: event) { MXLog.debug("[URLPreviewManager] Using cached preview.") success(preview) return } + // Otherwise make a request to the homeserver to generate a preview session.matrixRestClient.preview(for: sanitizedURL, success: { previewResponse in MXLog.debug("[URLPreviewManager] Cached preview not found. Requesting from homeserver.") if let previewResponse = previewResponse { + // Convert the response to preview data, fetching the image if provided. self.makePreviewData(from: previewResponse, for: sanitizedURL, and: event, with: session) { previewData in - self.store.store(previewData) + self.store.cache(previewData) success(previewData) } } @@ -52,11 +67,19 @@ class URLPreviewManager: NSObject { }, failure: failure) } - func makePreviewData(from previewResponse: MXURLPreview, + /// Convert an `MXURLPreview` object into `URLPreviewData` whilst also getting the image via the media manager. + /// - Parameters: + /// - previewResponse: The `MXURLPreview` object to convert. + /// - url: The URL that response was for. + /// - event: The event that the URL preview is for. + /// - session: The session to use to for media management. + /// - completion: A closure called when the operation completes. This contains the preview data. + private func makePreviewData(from previewResponse: MXURLPreview, for url: URL, and event: MXEvent, with session: MXSession, completion: @escaping (URLPreviewData) -> Void) { + // Create the preview data and return if no image is needed. let previewData = URLPreviewData(url: url, eventID: event.eventId, roomID: event.roomId, @@ -69,6 +92,7 @@ class URLPreviewManager: NSObject { return } + // Check for an image in the media cache and use this if found. if let cachePath = MXMediaManager.cachePath(forMatrixContentURI: imageURL, andType: previewResponse.imageType, inFolder: nil), let image = MXMediaManager.loadThroughCache(withFilePath: cachePath) { previewData.image = image @@ -78,6 +102,7 @@ class URLPreviewManager: NSObject { // Don't de-dupe image downloads as the manager should de-dupe preview generation. + // Otherwise download the image from the homeserver, treating an error as a preview without an image. session.mediaManager.downloadMedia(fromMatrixContentURI: imageURL, withType: previewResponse.imageType, inFolder: nil) { path in guard let image = MXMediaManager.loadThroughCache(withFilePath: path) else { completion(previewData) @@ -90,22 +115,29 @@ class URLPreviewManager: NSObject { } } + /// Removes any cached preview data that has expired. func removeExpiredCacheData() { store.removeExpiredItems() } + /// Deletes all cached preview data and closed previews from the store. func clearStore() { store.deleteAll() } - func closePreview(for eventID: String, in roomID: String) { - store.closePreview(for: eventID, in: roomID) + + /// Store the `eventId` and `roomId` of a closed preview. + func closePreview(for eventId: String, in roomId: String) { + store.closePreview(for: eventId, in: roomId) } + /// Whether a preview for the given event has been closed or not. func hasClosedPreview(from event: MXEvent) -> Bool { store.hasClosedPreview(for: event.eventId, in: event.roomId) } + /// Returns a URL created from the URL passed in, with sanitizations applied to reduce + /// queries and duplicate cache data for URLs that will return the same preview data. private func sanitize(_ url: URL) -> URL { // Remove the fragment from the URL. var components = URLComponents(url: url, resolvingAgainstBaseURL: false) diff --git a/Riot/Managers/URLPreviews/URLPreviewStore.swift b/Riot/Managers/URLPreviews/URLPreviewStore.swift index 5748844f83..3ff6075e4c 100644 --- a/Riot/Managers/URLPreviews/URLPreviewStore.swift +++ b/Riot/Managers/URLPreviews/URLPreviewStore.swift @@ -62,10 +62,10 @@ class URLPreviewStore { // MARK: - Public - /// Store a preview in the cache. If a preview already exists with the same URL it will be updated from the new preview. - /// - Parameter preview: The preview to add to the cache. - /// - Parameter date: Optional: The date the preview was generated. - func store(_ preview: URLPreviewData, generatedOn generationDate: Date? = nil) { + /// Cache a preview in the store. If a preview already exists with the same URL it will be updated from the new preview. + /// - Parameter preview: The preview to add to the store. + /// - Parameter date: Optional: The date the preview was generated. When nil, the current date is assigned. + func cache(_ preview: URLPreviewData, generatedOn generationDate: Date? = nil) { // Create a fetch request for an existing preview. let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() request.predicate = NSPredicate(format: "url == %@", preview.url as NSURL) @@ -132,17 +132,19 @@ class URLPreviewStore { } } - func closePreview(for eventID: String, in roomID: String) { - _ = ClosedURLPreview(context: context, eventID: eventID, roomID: roomID) + /// Store the `eventId` and `roomId` of a closed preview. + func closePreview(for eventId: String, in roomId: String) { + _ = ClosedURLPreview(context: context, eventID: eventId, roomID: roomId) save() } - func hasClosedPreview(for eventID: String, in roomID: String) -> Bool { + /// Whether a preview for an event with the given `eventId` and `roomId` has been closed or not. + func hasClosedPreview(for eventId: String, in roomId: String) -> Bool { // Create a request for the url excluding any expired items let request: NSFetchRequest = ClosedURLPreview.fetchRequest() request.predicate = NSCompoundPredicate(type: .and, subpredicates: [ - NSPredicate(format: "eventID == %@", eventID), - NSPredicate(format: "roomID == %@", roomID) + NSPredicate(format: "eventID == %@", eventId), + NSPredicate(format: "roomID == %@", roomId) ]) return (try? context.count(for: request)) ?? 0 > 0 diff --git a/Riot/Modules/Room/DataSources/RoomDataSource.m b/Riot/Modules/Room/DataSources/RoomDataSource.m index c76303fb6b..97171a7ae7 100644 --- a/Riot/Modules/Room/DataSources/RoomDataSource.m +++ b/Riot/Modules/Room/DataSources/RoomDataSource.m @@ -1216,6 +1216,8 @@ - (NSUInteger)roomBubbleDataIndexWithTag:(RoomBubbleCellDataTag)tag - (void)didOpenURLFromPreviewView:(URLPreviewView *)previewView for:(NSString *)eventID in:(NSString *)roomID { + // Use the link stored in the bubble component when opening the URL as we only + // store the sanitized URL in the preview data which may differ to the message content. RoomBubbleCellData *cellData = [self cellDataOfEventWithEventId:eventID]; if (!cellData) diff --git a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift index ae7742af44..e36759bc5e 100644 --- a/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift +++ b/Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift @@ -24,17 +24,20 @@ protocol URLPreviewViewDelegate: AnyObject { } @objcMembers +/// A view to display `URLPreviewData` generated by the `URLPreviewManager`. class URLPreviewView: UIView, NibLoadable, Themable { // MARK: - Constants private static let sizingView = URLPreviewView.instantiate() private enum Constants { + /// The fixed width of the preview view. static let width: CGFloat = 267.0 } // MARK: - Properties + /// The preview data to display in the view. var preview: URLPreviewData? { didSet { guard let preview = preview else { @@ -62,6 +65,7 @@ class URLPreviewView: UIView, NibLoadable, Themable { // Use a strong reference to keep it around when deactivating. @IBOutlet var siteNameLabelHeightConstraint: NSLayoutConstraint! + /// Returns true when `titleLabel` has a non-empty string. private var hasTitle: Bool { guard let title = titleLabel.text else { return false } return !title.isEmpty @@ -130,6 +134,7 @@ class URLPreviewView: UIView, NibLoadable, Themable { } // MARK: - Private + /// Tells the view to show in it's loading state. private func renderLoading() { // hide the content imageView.isHidden = true @@ -140,6 +145,7 @@ class URLPreviewView: UIView, NibLoadable, Themable { loadingActivityIndicator.startAnimating() } + /// Tells the view to display it's loaded state for the supplied data. private func renderLoaded(_ preview: URLPreviewData) { // update preview content imageView.image = preview.image @@ -147,10 +153,6 @@ class URLPreviewView: UIView, NibLoadable, Themable { titleLabel.text = preview.title descriptionLabel.text = preview.text - updateLayout() - } - - private func updateLayout() { // hide the loading interface loadingView.isHidden = true loadingActivityIndicator.stopAnimating() @@ -158,16 +160,15 @@ class URLPreviewView: UIView, NibLoadable, Themable { // show the content textContainerView.isHidden = false + // tweak the layout depending on the content if imageView.image == nil { imageView.isHidden = true - // tweak the layout of labels siteNameLabelHeightConstraint.isActive = true descriptionLabel.numberOfLines = hasTitle ? 3 : 5 } else { imageView.isHidden = false - // tweak the layout of labels siteNameLabelHeightConstraint.isActive = false descriptionLabel.numberOfLines = 2 } diff --git a/RiotTests/URLPreviewStoreTests.swift b/RiotTests/URLPreviewStoreTests.swift index 606b2d5c38..a995aa1c60 100644 --- a/RiotTests/URLPreviewStoreTests.swift +++ b/RiotTests/URLPreviewStoreTests.swift @@ -61,8 +61,8 @@ class URLPreviewStoreTests: XCTestCase { // Given a URL preview let preview = matrixPreview() - // When storing and retrieving that preview. - store.store(preview) + // When caching and retrieving that preview. + store.cache(preview) guard let cachedPreview = store.preview(for: preview.url, and: fakeEvent()) else { XCTFail("The cache should return a preview after storing one with the same URL.") @@ -80,7 +80,7 @@ class URLPreviewStoreTests: XCTestCase { func testUpdating() { // Given a preview stored in the cache. let preview = matrixPreview() - store.store(preview) + store.cache(preview) guard let cachedPreview = store.preview(for: preview.url, and: fakeEvent()) else { XCTFail("The cache should return a preview after storing one with the same URL.") @@ -96,7 +96,7 @@ class URLPreviewStoreTests: XCTestCase { siteName: "Matrix", title: "Home", text: "We updated our website.") - store.store(updatedPreview) + store.cache(updatedPreview) // Then the store should update the original preview. guard let updatedCachedPreview = store.preview(for: preview.url, and: fakeEvent()) else { @@ -110,7 +110,7 @@ class URLPreviewStoreTests: XCTestCase { func testPreviewExpiry() { // Given a preview generated 30 days ago. let preview = matrixPreview() - store.store(preview, generatedOn: Date().addingTimeInterval(-60 * 60 * 24 * 30)) + store.cache(preview, generatedOn: Date().addingTimeInterval(-60 * 60 * 24 * 30)) // When retrieving that today. let cachedPreview = store.preview(for: preview.url, and: fakeEvent()) @@ -123,7 +123,7 @@ class URLPreviewStoreTests: XCTestCase { // Given a cache with 2 items, one of which has expired. testPreviewExpiry() let preview = elementPreview() - store.store(preview) + store.cache(preview) XCTAssertEqual(store.cacheCount(), 2, "There should be 2 items in the cache.") // When removing expired items. @@ -140,7 +140,7 @@ class URLPreviewStoreTests: XCTestCase { // Given a cache with 2 items. testStoreAndRetrieve() let preview = elementPreview() - store.store(preview) + store.cache(preview) XCTAssertEqual(store.cacheCount(), 2, "There should be 2 items in the cache.") // When clearing the cache. From 24afc7af6c06a28fba43eed9b38098d2681351d3 Mon Sep 17 00:00:00 2001 From: Doug Date: Wed, 8 Sep 2021 15:10:13 +0100 Subject: [PATCH 24/28] Update for PR feedback. URLPreviewManager becomes URLPreviewService. addVerticalWhitespaceToString used instead of heightForCellData multiple times. All newline characters removed. --- .../Managers/URLPreviews/URLPreviewData.swift | 2 +- ...wManager.swift => URLPreviewService.swift} | 81 +++++++++++-------- .../URLPreviews/URLPreviewStore.swift | 6 +- Riot/Modules/Application/LegacyAppDelegate.h | 1 - Riot/Modules/Application/LegacyAppDelegate.m | 4 +- .../Room/CellData/RoomBubbleCellData.m | 10 ++- .../Modules/Room/DataSources/RoomDataSource.m | 2 +- Riot/Modules/Room/RoomViewController.m | 79 +++++++++--------- .../RoomIncomingTextMsgBubbleCell.m | 14 ---- ...mingTextMsgWithPaginationTitleBubbleCell.m | 14 ---- ...comingTextMsgWithoutSenderInfoBubbleCell.m | 14 ---- .../RoomOutgoingTextMsgBubbleCell.m | 14 ---- ...tgoingTextMsgWithoutSenderInfoBubbleCell.m | 14 ---- 13 files changed, 105 insertions(+), 150 deletions(-) rename Riot/Managers/URLPreviews/{URLPreviewManager.swift => URLPreviewService.swift} (85%) diff --git a/Riot/Managers/URLPreviews/URLPreviewData.swift b/Riot/Managers/URLPreviews/URLPreviewData.swift index 74e5c5125c..849b40d9f7 100644 --- a/Riot/Managers/URLPreviews/URLPreviewData.swift +++ b/Riot/Managers/URLPreviews/URLPreviewData.swift @@ -47,6 +47,6 @@ class URLPreviewData: NSObject { self.siteName = siteName self.title = title // Remove line breaks from the description text - self.text = text?.replacingOccurrences(of: "\n", with: " ") + self.text = text?.components(separatedBy: .newlines).joined(separator: " ") } } diff --git a/Riot/Managers/URLPreviews/URLPreviewManager.swift b/Riot/Managers/URLPreviews/URLPreviewService.swift similarity index 85% rename from Riot/Managers/URLPreviews/URLPreviewManager.swift rename to Riot/Managers/URLPreviews/URLPreviewService.swift index 29357509b1..d897286213 100644 --- a/Riot/Managers/URLPreviews/URLPreviewManager.swift +++ b/Riot/Managers/URLPreviews/URLPreviewService.swift @@ -16,17 +16,24 @@ import Foundation +enum URLPreviewServiceError: Error { + case missingResponse +} + @objcMembers -/// A manager for URL preview data to handle fetching, caching and clean-up +/// A service for URL preview data that handles fetching, caching and clean-up /// as well as remembering which previews have been closed by the user. -class URLPreviewManager: NSObject { - /// The shared manager object. - static let shared = URLPreviewManager() +class URLPreviewService: NSObject { + + // MARK: - Properties + + /// The shared service object. + static let shared = URLPreviewService() /// A persistent store backed by Core Data to reduce network requests private let store = URLPreviewStore() - private override init() { } + // MARK: - Public /// Generates preview data for a URL to be previewed as part of the supplied event, /// first checking the cache, and if necessary making a request to the homeserver. @@ -47,26 +54,51 @@ class URLPreviewManager: NSObject { // Check for a valid preview in the store, and use this if found if let preview = store.preview(for: sanitizedURL, and: event) { - MXLog.debug("[URLPreviewManager] Using cached preview.") + MXLog.debug("[URLPreviewService] Using cached preview.") success(preview) return } // Otherwise make a request to the homeserver to generate a preview session.matrixRestClient.preview(for: sanitizedURL, success: { previewResponse in - MXLog.debug("[URLPreviewManager] Cached preview not found. Requesting from homeserver.") + MXLog.debug("[URLPreviewService] Cached preview not found. Requesting from homeserver.") + + guard let previewResponse = previewResponse else { + failure(URLPreviewServiceError.missingResponse) + return + } - if let previewResponse = previewResponse { - // Convert the response to preview data, fetching the image if provided. - self.makePreviewData(from: previewResponse, for: sanitizedURL, and: event, with: session) { previewData in - self.store.cache(previewData) - success(previewData) - } + // Convert the response to preview data, fetching the image if provided. + self.makePreviewData(from: previewResponse, for: sanitizedURL, and: event, with: session) { previewData in + self.store.cache(previewData) + success(previewData) } }, failure: failure) } + /// Removes any cached preview data that has expired. + func removeExpiredCacheData() { + store.removeExpiredItems() + } + + /// Deletes all cached preview data and closed previews from the store. + func clearStore() { + store.deleteAll() + } + + /// Store the `eventId` and `roomId` of a closed preview. + func closePreview(for eventId: String, in roomId: String) { + store.closePreview(for: eventId, in: roomId) + } + + /// Whether a preview for the given event has been closed or not. + func hasClosedPreview(from event: MXEvent) -> Bool { + store.hasClosedPreview(for: event.eventId, in: event.roomId) + } + + // MARK: - Private + /// Convert an `MXURLPreview` object into `URLPreviewData` whilst also getting the image via the media manager. /// - Parameters: /// - previewResponse: The `MXURLPreview` object to convert. @@ -100,7 +132,7 @@ class URLPreviewManager: NSObject { return } - // Don't de-dupe image downloads as the manager should de-dupe preview generation. + // Don't de-dupe image downloads as the service should de-dupe preview generation. // Otherwise download the image from the homeserver, treating an error as a preview without an image. session.mediaManager.downloadMedia(fromMatrixContentURI: imageURL, withType: previewResponse.imageType, inFolder: nil) { path in @@ -115,27 +147,6 @@ class URLPreviewManager: NSObject { } } - /// Removes any cached preview data that has expired. - func removeExpiredCacheData() { - store.removeExpiredItems() - } - - /// Deletes all cached preview data and closed previews from the store. - func clearStore() { - store.deleteAll() - } - - - /// Store the `eventId` and `roomId` of a closed preview. - func closePreview(for eventId: String, in roomId: String) { - store.closePreview(for: eventId, in: roomId) - } - - /// Whether a preview for the given event has been closed or not. - func hasClosedPreview(from event: MXEvent) -> Bool { - store.hasClosedPreview(for: event.eventId, in: event.roomId) - } - /// Returns a URL created from the URL passed in, with sanitizations applied to reduce /// queries and duplicate cache data for URLs that will return the same preview data. private func sanitize(_ url: URL) -> URL { diff --git a/Riot/Managers/URLPreviews/URLPreviewStore.swift b/Riot/Managers/URLPreviews/URLPreviewStore.swift index 3ff6075e4c..cacc846247 100644 --- a/Riot/Managers/URLPreviews/URLPreviewStore.swift +++ b/Riot/Managers/URLPreviews/URLPreviewStore.swift @@ -49,7 +49,11 @@ class URLPreviewStore { container = NSPersistentContainer(name: "URLPreviewStore") if inMemory { - container.persistentStoreDescriptions.first?.url = URL(fileURLWithPath: "/dev/null") + if let storeDescription = container.persistentStoreDescriptions.first { + storeDescription.url = URL(fileURLWithPath: "/dev/null") + } else { + MXLog.error("[URLPreviewStore] persistentStoreDescription not found.") + } } // Load the persistent stores into the container diff --git a/Riot/Modules/Application/LegacyAppDelegate.h b/Riot/Modules/Application/LegacyAppDelegate.h index df279fc146..1b06d8cbeb 100644 --- a/Riot/Modules/Application/LegacyAppDelegate.h +++ b/Riot/Modules/Application/LegacyAppDelegate.h @@ -31,7 +31,6 @@ @protocol LegacyAppDelegateDelegate; @class CallBar; @class CallPresenter; -@class URLPreviewManager; #pragma mark - Notifications /** diff --git a/Riot/Modules/Application/LegacyAppDelegate.m b/Riot/Modules/Application/LegacyAppDelegate.m index 26f5e03ab9..f18bdadf81 100644 --- a/Riot/Modules/Application/LegacyAppDelegate.m +++ b/Riot/Modules/Application/LegacyAppDelegate.m @@ -548,7 +548,7 @@ - (void)applicationDidEnterBackground:(UIApplication *)application [MXMediaManager reduceCacheSizeToInsert:0]; // Remove expired URL previews from the cache - [URLPreviewManager.shared removeExpiredCacheData]; + [URLPreviewService.shared removeExpiredCacheData]; // Hide potential notification if (self.mxInAppNotification) @@ -4331,7 +4331,7 @@ - (void)clearCache [MXMediaManager clearCache]; [MXKAttachment clearCache]; [VoiceMessageAttachmentCacheManagerBridge clearCache]; - [URLPreviewManager.shared clearStore]; + [URLPreviewService.shared clearStore]; } @end diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index 7339db8e86..28750bea5e 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -527,6 +527,12 @@ - (void)addVerticalWhitespaceToString:(NSMutableAttributedString *)attributedStr { CGFloat additionalVerticalHeight = 0; + // Add vertical whitespace in case of a URL preview. + if (RiotSettings.shared.roomScreenShowsURLPreviews && self.showURLPreview) + { + additionalVerticalHeight += RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:self.urlPreviewData]; + } + // Add vertical whitespace in case of reactions. additionalVerticalHeight+= [self reactionHeightForEventId:eventId]; // Add vertical whitespace in case of read receipts. @@ -1082,7 +1088,7 @@ - (void)loadURLPreview } // Don't show the preview if it has been dismissed already. - self.showURLPreview = ![URLPreviewManager.shared hasClosedPreviewFrom:lastComponent.event]; + self.showURLPreview = ![URLPreviewService.shared hasClosedPreviewFrom:lastComponent.event]; if (!self.showURLPreview) { return; @@ -1103,7 +1109,7 @@ - (void)loadURLPreview @"roomId": self.roomId }; - [URLPreviewManager.shared previewFor:lastComponent.link + [URLPreviewService.shared previewFor:lastComponent.link and:lastComponent.event with:self.mxSession success:^(URLPreviewData * _Nonnull urlPreviewData) { diff --git a/Riot/Modules/Room/DataSources/RoomDataSource.m b/Riot/Modules/Room/DataSources/RoomDataSource.m index 97171a7ae7..b8b58d61ca 100644 --- a/Riot/Modules/Room/DataSources/RoomDataSource.m +++ b/Riot/Modules/Room/DataSources/RoomDataSource.m @@ -1245,7 +1245,7 @@ - (void)didCloseURLPreviewView:(URLPreviewView *)previewView for:(NSString *)eve } // Remember that the user closed the preview so it isn't shown again. - [URLPreviewManager.shared closePreviewFor:eventID in:roomID]; + [URLPreviewService.shared closePreviewFor:eventID in:roomID]; // Hide the preview, remove its data and refresh the cells. cellData.showURLPreview = NO; diff --git a/Riot/Modules/Room/RoomViewController.m b/Riot/Modules/Room/RoomViewController.m index 80164bcaa4..312902949e 100644 --- a/Riot/Modules/Room/RoomViewController.m +++ b/Riot/Modules/Room/RoomViewController.m @@ -444,43 +444,7 @@ - (void)viewDidLoad [self userInterfaceThemeDidChange]; // Observe URL preview updates. - URLPreviewDidUpdateNotificationObserver = [NSNotificationCenter.defaultCenter addObserverForName:URLPreviewDidUpdateNotification object:nil queue:NSOperationQueue.mainQueue usingBlock:^(NSNotification * _Nonnull notification) { - - // Ensure this is the correct room - if (![(NSString*)notification.userInfo[@"roomId"] isEqualToString:self.roomDataSource.roomId]) - { - return; - } - - // Get the indexPath for the updated cell. - NSString *updatedEventId = notification.userInfo[@"eventId"]; - NSInteger updatedEventIndex = [self.roomDataSource indexOfCellDataWithEventId:updatedEventId]; - NSIndexPath *updatedIndexPath = [NSIndexPath indexPathForRow:updatedEventIndex inSection:0]; - - // Store the content size and offset before reloading the cell - CGFloat originalContentSize = self.bubblesTableView.contentSize.height; - CGPoint contentOffset = self.bubblesTableView.contentOffset; - - // Only update the content offset if the cell is visible or above the current visible cells. - BOOL shouldUpdateContentOffset = NO; - NSIndexPath *lastVisibleIndexPath = [self.bubblesTableView indexPathsForVisibleRows].lastObject; - if (lastVisibleIndexPath && updatedIndexPath.row < lastVisibleIndexPath.row) - { - shouldUpdateContentOffset = YES; - } - - // Note: Despite passing in the index path, this reloads the whole table. - [self dataSource:self.roomDataSource didCellChange:updatedIndexPath]; - - // Update the content offset to include any changes to the scroll view's height. - if (shouldUpdateContentOffset) - { - CGFloat delta = self.bubblesTableView.contentSize.height - originalContentSize; - contentOffset.y += delta; - - self.bubblesTableView.contentOffset = contentOffset; - } - }]; + [self registerURLPreviewNotifications]; [self setupActions]; } @@ -1556,6 +1520,47 @@ - (BOOL)canEditJitsiWidget return myPower >= requiredPower; } +- (void)registerURLPreviewNotifications +{ + URLPreviewDidUpdateNotificationObserver = [NSNotificationCenter.defaultCenter addObserverForName:URLPreviewDidUpdateNotification object:nil queue:NSOperationQueue.mainQueue usingBlock:^(NSNotification * _Nonnull notification) { + + // Ensure this is the correct room + if (![(NSString*)notification.userInfo[@"roomId"] isEqualToString:self.roomDataSource.roomId]) + { + return; + } + + // Get the indexPath for the updated cell. + NSString *updatedEventId = notification.userInfo[@"eventId"]; + NSInteger updatedEventIndex = [self.roomDataSource indexOfCellDataWithEventId:updatedEventId]; + NSIndexPath *updatedIndexPath = [NSIndexPath indexPathForRow:updatedEventIndex inSection:0]; + + // Store the content size and offset before reloading the cell + CGFloat originalContentSize = self.bubblesTableView.contentSize.height; + CGPoint contentOffset = self.bubblesTableView.contentOffset; + + // Only update the content offset if the cell is visible or above the current visible cells. + BOOL shouldUpdateContentOffset = NO; + NSIndexPath *lastVisibleIndexPath = [self.bubblesTableView indexPathsForVisibleRows].lastObject; + if (lastVisibleIndexPath && updatedIndexPath.row < lastVisibleIndexPath.row) + { + shouldUpdateContentOffset = YES; + } + + // Note: Despite passing in the index path, this reloads the whole table. + [self dataSource:self.roomDataSource didCellChange:updatedIndexPath]; + + // Update the content offset to include any changes to the scroll view's height. + if (shouldUpdateContentOffset) + { + CGFloat delta = self.bubblesTableView.contentSize.height - originalContentSize; + contentOffset.y += delta; + + self.bubblesTableView.contentOffset = contentOffset; + } + }]; +} + - (void)refreshRoomTitle { NSMutableArray *rightBarButtonItems = nil; diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m index 542f20a445..d308fcfea0 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m @@ -39,18 +39,4 @@ - (void)render:(MXKCellData *)cellData [self updateUserNameColor]; } -+ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth -{ - RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - - // Include the URL preview in the height if necessary. - if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) - { - CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; - return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; - } - - return [super heightForCellData:cellData withMaximumWidth:maxWidth]; -} - @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m index af187513bf..2e8bdeb342 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m @@ -45,18 +45,4 @@ - (void)render:(MXKCellData *)cellData } } -+ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth -{ - RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - - // Include the URL preview in the height if necessary. - if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) - { - CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; - return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; - } - - return [super heightForCellData:cellData withMaximumWidth:maxWidth]; -} - @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m index 19fb4a5cd3..56b0db94c3 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m @@ -29,18 +29,4 @@ - (void)customizeTableViewCellRendering self.messageTextView.tintColor = ThemeService.shared.theme.tintColor; } -+ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth -{ - RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - - // Include the URL preview in the height if necessary. - if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) - { - CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; - return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; - } - - return [super heightForCellData:cellData withMaximumWidth:maxWidth]; -} - @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m index 94bd5c3101..374ed6da00 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m @@ -40,18 +40,4 @@ - (void)render:(MXKCellData *)cellData [self updateUserNameColor]; } -+ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth -{ - RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - - // Include the URL preview in the height if necessary. - if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) - { - CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; - return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; - } - - return [super heightForCellData:cellData withMaximumWidth:maxWidth]; -} - @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m index 0c444d51fb..1e0ffeb61b 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m @@ -29,18 +29,4 @@ - (void)customizeTableViewCellRendering self.messageTextView.tintColor = ThemeService.shared.theme.tintColor; } -+ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth -{ - RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; - - // Include the URL preview in the height if necessary. - if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) - { - CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; - return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; - } - - return [super heightForCellData:cellData withMaximumWidth:maxWidth]; -} - @end From 206017c01fa500bfb5d32922c55d10bc5f062801 Mon Sep 17 00:00:00 2001 From: Doug Date: Wed, 8 Sep 2021 15:47:14 +0100 Subject: [PATCH 25/28] Rename Core Data objects. URLPreviewCacheData becomes URLPreviewData in the model with a class name of URLPreviewDataMO ClosedURLData becomes URLPreviewUserData in the model with a class name of URLPreviewUserDataMO --- .../URLPreviewDataMO.swift} | 2 +- .../URLPreviewImageTransformer.swift | 0 .../{ => Core Data}/URLPreviewStore.swift | 23 ++++++++++--------- .../URLPreviewStore.xcdatamodel/contents | 23 ++++++++++--------- .../URLPreviewUserDataMO.swift} | 5 ++-- 5 files changed, 28 insertions(+), 25 deletions(-) rename Riot/Managers/URLPreviews/{URLPreviewCacheData.swift => Core Data/URLPreviewDataMO.swift} (98%) rename Riot/Managers/URLPreviews/{ => Core Data}/URLPreviewImageTransformer.swift (100%) rename Riot/Managers/URLPreviews/{ => Core Data}/URLPreviewStore.swift (88%) rename Riot/Managers/URLPreviews/{ => Core Data}/URLPreviewStore.xcdatamodeld/URLPreviewStore.xcdatamodel/contents (69%) rename Riot/Managers/URLPreviews/{ClosedURLPreview.swift => Core Data/URLPreviewUserDataMO.swift} (87%) diff --git a/Riot/Managers/URLPreviews/URLPreviewCacheData.swift b/Riot/Managers/URLPreviews/Core Data/URLPreviewDataMO.swift similarity index 98% rename from Riot/Managers/URLPreviews/URLPreviewCacheData.swift rename to Riot/Managers/URLPreviews/Core Data/URLPreviewDataMO.swift index 099f1ebb14..31b18c5c5d 100644 --- a/Riot/Managers/URLPreviews/URLPreviewCacheData.swift +++ b/Riot/Managers/URLPreviews/Core Data/URLPreviewDataMO.swift @@ -16,7 +16,7 @@ import CoreData -extension URLPreviewCacheData { +extension URLPreviewDataMO { convenience init(context: NSManagedObjectContext, preview: URLPreviewData, creationDate: Date) { self.init(context: context) update(from: preview, on: creationDate) diff --git a/Riot/Managers/URLPreviews/URLPreviewImageTransformer.swift b/Riot/Managers/URLPreviews/Core Data/URLPreviewImageTransformer.swift similarity index 100% rename from Riot/Managers/URLPreviews/URLPreviewImageTransformer.swift rename to Riot/Managers/URLPreviews/Core Data/URLPreviewImageTransformer.swift diff --git a/Riot/Managers/URLPreviews/URLPreviewStore.swift b/Riot/Managers/URLPreviews/Core Data/URLPreviewStore.swift similarity index 88% rename from Riot/Managers/URLPreviews/URLPreviewStore.swift rename to Riot/Managers/URLPreviews/Core Data/URLPreviewStore.swift index cacc846247..dc02a66bb5 100644 --- a/Riot/Managers/URLPreviews/URLPreviewStore.swift +++ b/Riot/Managers/URLPreviews/Core Data/URLPreviewStore.swift @@ -71,7 +71,7 @@ class URLPreviewStore { /// - Parameter date: Optional: The date the preview was generated. When nil, the current date is assigned. func cache(_ preview: URLPreviewData, generatedOn generationDate: Date? = nil) { // Create a fetch request for an existing preview. - let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() + let request: NSFetchRequest = URLPreviewDataMO.fetchRequest() request.predicate = NSPredicate(format: "url == %@", preview.url as NSURL) // Use the custom date if supplied (currently this is for testing purposes) @@ -81,7 +81,7 @@ class URLPreviewStore { if let cachedPreview = try? context.fetch(request).first { cachedPreview.update(from: preview, on: date) } else { - _ = URLPreviewCacheData(context: context, preview: preview, creationDate: date) + _ = URLPreviewDataMO(context: context, preview: preview, creationDate: date) } save() @@ -93,7 +93,7 @@ class URLPreviewStore { /// - Returns: The preview if found, otherwise nil. func preview(for url: URL, and event: MXEvent) -> URLPreviewData? { // Create a request for the url excluding any expired items - let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() + let request: NSFetchRequest = URLPreviewDataMO.fetchRequest() request.predicate = NSCompoundPredicate(type: .and, subpredicates: [ NSPredicate(format: "url == %@", url as NSURL), NSPredicate(format: "creationDate > %@", expiryDate as NSDate) @@ -110,13 +110,13 @@ class URLPreviewStore { /// Returns the number of URL previews cached in the store. func cacheCount() -> Int { - let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() + let request: NSFetchRequest = URLPreviewDataMO.fetchRequest() return (try? context.count(for: request)) ?? 0 } /// Removes any expired cache data from the store. func removeExpiredItems() { - let request: NSFetchRequest = URLPreviewCacheData.fetchRequest() + let request: NSFetchRequest = URLPreviewDataMO.fetchRequest() request.predicate = NSPredicate(format: "creationDate < %@", expiryDate as NSDate) do { @@ -129,26 +129,27 @@ class URLPreviewStore { /// Deletes all cache data and all closed previews from the store. func deleteAll() { do { - _ = try context.execute(NSBatchDeleteRequest(fetchRequest: URLPreviewCacheData.fetchRequest())) - _ = try context.execute(NSBatchDeleteRequest(fetchRequest: ClosedURLPreview.fetchRequest())) + _ = try context.execute(NSBatchDeleteRequest(fetchRequest: URLPreviewDataMO.fetchRequest())) + _ = try context.execute(NSBatchDeleteRequest(fetchRequest: URLPreviewUserDataMO.fetchRequest())) } catch { MXLog.error("[URLPreviewStore] Error executing batch delete request: \(error.localizedDescription)") } } - /// Store the `eventId` and `roomId` of a closed preview. + /// Store the dismissal of a preview from the event with `eventId` and `roomId`. func closePreview(for eventId: String, in roomId: String) { - _ = ClosedURLPreview(context: context, eventID: eventId, roomID: roomId) + _ = URLPreviewUserDataMO(context: context, eventID: eventId, roomID: roomId, dismissed: true) save() } /// Whether a preview for an event with the given `eventId` and `roomId` has been closed or not. func hasClosedPreview(for eventId: String, in roomId: String) -> Bool { // Create a request for the url excluding any expired items - let request: NSFetchRequest = ClosedURLPreview.fetchRequest() + let request: NSFetchRequest = URLPreviewUserDataMO.fetchRequest() request.predicate = NSCompoundPredicate(type: .and, subpredicates: [ NSPredicate(format: "eventID == %@", eventId), - NSPredicate(format: "roomID == %@", roomId) + NSPredicate(format: "roomID == %@", roomId), + NSPredicate(format: "dismissed == true") ]) return (try? context.count(for: request)) ?? 0 > 0 diff --git a/Riot/Managers/URLPreviews/URLPreviewStore.xcdatamodeld/URLPreviewStore.xcdatamodel/contents b/Riot/Managers/URLPreviews/Core Data/URLPreviewStore.xcdatamodeld/URLPreviewStore.xcdatamodel/contents similarity index 69% rename from Riot/Managers/URLPreviews/URLPreviewStore.xcdatamodeld/URLPreviewStore.xcdatamodel/contents rename to Riot/Managers/URLPreviews/Core Data/URLPreviewStore.xcdatamodeld/URLPreviewStore.xcdatamodel/contents index cade6911fc..2064295435 100644 --- a/Riot/Managers/URLPreviews/URLPreviewStore.xcdatamodeld/URLPreviewStore.xcdatamodel/contents +++ b/Riot/Managers/URLPreviews/Core Data/URLPreviewStore.xcdatamodeld/URLPreviewStore.xcdatamodel/contents @@ -1,6 +1,15 @@ - + + + + + + + + + + @@ -10,16 +19,8 @@ - - - - - - - - - - + + \ No newline at end of file diff --git a/Riot/Managers/URLPreviews/ClosedURLPreview.swift b/Riot/Managers/URLPreviews/Core Data/URLPreviewUserDataMO.swift similarity index 87% rename from Riot/Managers/URLPreviews/ClosedURLPreview.swift rename to Riot/Managers/URLPreviews/Core Data/URLPreviewUserDataMO.swift index 37f778d794..6b1aa34011 100644 --- a/Riot/Managers/URLPreviews/ClosedURLPreview.swift +++ b/Riot/Managers/URLPreviews/Core Data/URLPreviewUserDataMO.swift @@ -16,10 +16,11 @@ import CoreData -extension ClosedURLPreview { - convenience init(context: NSManagedObjectContext, eventID: String, roomID: String) { +extension URLPreviewUserDataMO { + convenience init(context: NSManagedObjectContext, eventID: String, roomID: String, dismissed: Bool) { self.init(context: context) self.eventID = eventID self.roomID = roomID + self.dismissed = dismissed } } From 26567464e4d558314929781bb6ad42ac122e55de Mon Sep 17 00:00:00 2001 From: Doug Date: Wed, 8 Sep 2021 15:59:30 +0100 Subject: [PATCH 26/28] Revert height computation for now. --- Riot/Modules/Room/CellData/RoomBubbleCellData.m | 6 ------ .../BubbleCells/RoomIncomingTextMsgBubbleCell.m | 14 ++++++++++++++ ...mIncomingTextMsgWithPaginationTitleBubbleCell.m | 14 ++++++++++++++ ...oomIncomingTextMsgWithoutSenderInfoBubbleCell.m | 14 ++++++++++++++ .../BubbleCells/RoomOutgoingTextMsgBubbleCell.m | 14 ++++++++++++++ ...oomOutgoingTextMsgWithoutSenderInfoBubbleCell.m | 14 ++++++++++++++ 6 files changed, 70 insertions(+), 6 deletions(-) diff --git a/Riot/Modules/Room/CellData/RoomBubbleCellData.m b/Riot/Modules/Room/CellData/RoomBubbleCellData.m index 28750bea5e..33a34821f2 100644 --- a/Riot/Modules/Room/CellData/RoomBubbleCellData.m +++ b/Riot/Modules/Room/CellData/RoomBubbleCellData.m @@ -527,12 +527,6 @@ - (void)addVerticalWhitespaceToString:(NSMutableAttributedString *)attributedStr { CGFloat additionalVerticalHeight = 0; - // Add vertical whitespace in case of a URL preview. - if (RiotSettings.shared.roomScreenShowsURLPreviews && self.showURLPreview) - { - additionalVerticalHeight += RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:self.urlPreviewData]; - } - // Add vertical whitespace in case of reactions. additionalVerticalHeight+= [self reactionHeightForEventId:eventId]; // Add vertical whitespace in case of read receipts. diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m index d308fcfea0..542f20a445 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgBubbleCell.m @@ -39,4 +39,18 @@ - (void)render:(MXKCellData *)cellData [self updateUserNameColor]; } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m index 2e8bdeb342..af187513bf 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithPaginationTitleBubbleCell.m @@ -45,4 +45,18 @@ - (void)render:(MXKCellData *)cellData } } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m index 56b0db94c3..19fb4a5cd3 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m @@ -29,4 +29,18 @@ - (void)customizeTableViewCellRendering self.messageTextView.tintColor = ThemeService.shared.theme.tintColor; } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m index 374ed6da00..94bd5c3101 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgBubbleCell.m @@ -40,4 +40,18 @@ - (void)render:(MXKCellData *)cellData [self updateUserNameColor]; } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end diff --git a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m index 1e0ffeb61b..0c444d51fb 100644 --- a/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m +++ b/Riot/Modules/Room/Views/BubbleCells/RoomOutgoingTextMsgWithoutSenderInfoBubbleCell.m @@ -29,4 +29,18 @@ - (void)customizeTableViewCellRendering self.messageTextView.tintColor = ThemeService.shared.theme.tintColor; } ++ (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth +{ + RoomBubbleCellData *bubbleData = (RoomBubbleCellData*)cellData; + + // Include the URL preview in the height if necessary. + if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview) + { + CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth]; + return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData]; + } + + return [super heightForCellData:cellData withMaximumWidth:maxWidth]; +} + @end From ad618b463952260cfebf6b2d967f49bc992cba4b Mon Sep 17 00:00:00 2001 From: Doug Date: Wed, 8 Sep 2021 16:24:50 +0100 Subject: [PATCH 27/28] Add matrix.to to firstURLDetectionIgnoredHosts. --- Config/CommonConfiguration.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Config/CommonConfiguration.swift b/Config/CommonConfiguration.swift index b6aa77dade..8ae68e42eb 100644 --- a/Config/CommonConfiguration.swift +++ b/Config/CommonConfiguration.swift @@ -49,9 +49,14 @@ class CommonConfiguration: NSObject, Configurable { settings.messageDetailsAllowCopyingMedia = BuildSettings.messageDetailsAllowCopyMedia settings.messageDetailsAllowPastingMedia = BuildSettings.messageDetailsAllowPasteMedia - // Enable link detection if url preview are enabled + // Enable link detection if url previews are enabled settings.enableBubbleComponentLinkDetection = true + // Prevent URL previews from being generated for matrix.to links + if let matrixDotToHost = URL(string: kMXMatrixDotToUrl)?.host { + settings.firstURLDetectionIgnoredHosts = [matrixDotToHost] + } + MXKContactManager.shared().allowLocalContactsAccess = BuildSettings.allowLocalContactsAccess } From dfb63e1436480b102c95d34a2ee3ea23a6d116c0 Mon Sep 17 00:00:00 2001 From: Doug Date: Wed, 8 Sep 2021 16:35:16 +0100 Subject: [PATCH 28/28] Revert "Add matrix.to to firstURLDetectionIgnoredHosts." This reverts commit ad618b463952260cfebf6b2d967f49bc992cba4b. --- Config/CommonConfiguration.swift | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Config/CommonConfiguration.swift b/Config/CommonConfiguration.swift index 8ae68e42eb..b6aa77dade 100644 --- a/Config/CommonConfiguration.swift +++ b/Config/CommonConfiguration.swift @@ -49,14 +49,9 @@ class CommonConfiguration: NSObject, Configurable { settings.messageDetailsAllowCopyingMedia = BuildSettings.messageDetailsAllowCopyMedia settings.messageDetailsAllowPastingMedia = BuildSettings.messageDetailsAllowPasteMedia - // Enable link detection if url previews are enabled + // Enable link detection if url preview are enabled settings.enableBubbleComponentLinkDetection = true - // Prevent URL previews from being generated for matrix.to links - if let matrixDotToHost = URL(string: kMXMatrixDotToUrl)?.host { - settings.firstURLDetectionIgnoredHosts = [matrixDotToHost] - } - MXKContactManager.shared().allowLocalContactsAccess = BuildSettings.allowLocalContactsAccess }