From bceebff6cbf876160cf2b153c9403d6f7bc9ec20 Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Tue, 3 Oct 2023 10:11:05 +0200 Subject: [PATCH] Handle tap on user mentions (#1850) * user mention routing implemented * more tests * better naming * fixed a test --- .../Sources/Application/AppCoordinator.swift | 2 + .../Application/Navigation/AppRoutes.swift | 16 ++++ .../RoomFlowCoordinator.swift | 11 +++ .../UserSessionFlowCoordinator.swift | 2 +- .../HTMLParsing/AttributedStringBuilder.swift | 64 +++++++------- .../Sources/Other/Pills/MentionBuilder.swift | 12 +-- .../Sources/AppRouteURLParserTests.swift | 12 +++ .../AttributedStringBuilderTests.swift | 87 ++++++++++++++++++- 8 files changed, 169 insertions(+), 37 deletions(-) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index fa9a0a991..6850b5188 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -158,6 +158,8 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate, } else { navigationRootCoordinator.setSheetCoordinator(GenericCallLinkCoordinator(parameters: .init(url: url))) } + case .roomMemberDetails: + userSessionFlowCoordinator?.handleAppRoute(route, animated: true) default: break } diff --git a/ElementX/Sources/Application/Navigation/AppRoutes.swift b/ElementX/Sources/Application/Navigation/AppRoutes.swift index 41ca9e17c..b217c4b3c 100644 --- a/ElementX/Sources/Application/Navigation/AppRoutes.swift +++ b/ElementX/Sources/Application/Navigation/AppRoutes.swift @@ -21,6 +21,7 @@ enum AppRoute: Equatable { case roomList case room(roomID: String) case roomDetails(roomID: String) + case roomMemberDetails(userID: String) case invites case genericCallLink(url: URL) } @@ -30,6 +31,7 @@ struct AppRouteURLParser { init(appSettings: AppSettings) { urlParsers = [ + MatrixPermalinkParser(appSettings: appSettings), OIDCCallbackURLParser(appSettings: appSettings), ElementCallURLParser() ] @@ -105,3 +107,17 @@ struct ElementCallURLParser: URLParser { return .genericCallLink(url: url) } } + +struct MatrixPermalinkParser: URLParser { + let appSettings: AppSettings + + func route(from url: URL) -> AppRoute? { + switch PermalinkBuilder.detectPermalink(in: url, baseURL: appSettings.permalinkBaseURL) { + case .userIdentifier(let userID): + return .roomMemberDetails(userID: userID) + // Other cases will be handled in the future + default: + return nil + } + } +} diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index 1550f3067..1ef7915a8 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift @@ -81,6 +81,17 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { stateMachine.tryEvent(.presentRoomDetails(roomID: roomID), userInfo: EventUserInfo(animated: animated)) case .roomList: stateMachine.tryEvent(.dismissRoom, userInfo: EventUserInfo(animated: animated)) + case .roomMemberDetails(let userID): + Task { + switch await roomProxy?.getMember(userID: userID) { + case .success(let member): + stateMachine.tryEvent(.presentRoomMemberDetails(member: .init(value: member))) + case .failure(let error): + MXLog.error("[RoomFlowCoordinator] Failed to get member: \(error)") + case .none: + MXLog.error("[RoomFlowCoordinator] Failed to get member: RoomProxy is nil") + } + } case .invites: break case .genericCallLink, .oidcCallback: diff --git a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift index 7772aaaf1..b434c0c99 100644 --- a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift @@ -115,7 +115,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol { guard let self else { return } switch appRoute { - case .room, .roomDetails, .roomList: + case .room, .roomDetails, .roomList, .roomMemberDetails: self.roomFlowCoordinator.handleAppRoute(appRoute, animated: animated) case .invites: if UIDevice.current.isPhone { diff --git a/ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift b/ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift index 7e72069ac..51956d3df 100644 --- a/ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift +++ b/ElementX/Sources/Other/HTMLParsing/AttributedStringBuilder.swift @@ -57,6 +57,7 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol { let mutableAttributedString = NSMutableAttributedString(string: string) addLinks(mutableAttributedString) + addAllUsersMention(mutableAttributedString) detectPermalinks(mutableAttributedString) removeLinkColors(mutableAttributedString) @@ -110,6 +111,7 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol { let mutableAttributedString = NSMutableAttributedString(attributedString: attributedString) removeDefaultForegroundColor(mutableAttributedString) addLinks(mutableAttributedString) + addAllUsersMention(mutableAttributedString) replaceMarkedBlockquotes(mutableAttributedString) replaceMarkedCodeBlocks(mutableAttributedString) detectPermalinks(mutableAttributedString) @@ -184,38 +186,42 @@ struct AttributedStringBuilder: AttributedStringBuilderProtocol { let linkMatches = MatrixEntityRegex.linkRegex.matches(in: string, options: []) matches.append(contentsOf: linkMatches) - if matches.count > 0 { - // Sort the links by length so the longest one always takes priority - matches.sorted { $0.range.length > $1.range.length }.forEach { match in - guard let matchRange = Range(match.range, in: string) else { - return - } - - var hasLink = false - attributedString.enumerateAttribute(.link, in: match.range, options: []) { value, _, stop in - if value != nil { - hasLink = true - stop.pointee = true - } - } - - if hasLink { - return - } - - var link = String(string[matchRange]) - - if linkMatches.contains(match), !link.contains("://") { - link.insert(contentsOf: "https://", at: link.startIndex) - } - - if let url = URL(string: link) { - attributedString.addAttribute(.link, value: url, range: match.range) + + guard matches.count > 0 else { + return + } + // Sort the links by length so the longest one always takes priority + matches.sorted { $0.range.length > $1.range.length }.forEach { match in + guard let matchRange = Range(match.range, in: string) else { + return + } + + var hasLink = false + attributedString.enumerateAttribute(.link, in: match.range, options: []) { value, _, stop in + if value != nil { + hasLink = true + stop.pointee = true } } + + if hasLink { + return + } + + var link = String(string[matchRange]) + + if linkMatches.contains(match), !link.contains("://") { + link.insert(contentsOf: "https://", at: link.startIndex) + } + + if let url = URL(string: link) { + attributedString.addAttribute(.link, value: url, range: match.range) + } } - - MatrixEntityRegex.allUsersRegex.matches(in: string, options: []).forEach { match in + } + + private func addAllUsersMention(_ attributedString: NSMutableAttributedString) { + MatrixEntityRegex.allUsersRegex.matches(in: attributedString.string, options: []).forEach { match in if attributedString.attribute(.link, at: 0, longestEffectiveRange: nil, in: match.range) == nil { attributedString.addAttribute(.MatrixAllUsersMention, value: true, range: match.range) } diff --git a/ElementX/Sources/Other/Pills/MentionBuilder.swift b/ElementX/Sources/Other/Pills/MentionBuilder.swift index 22674abec..9074d9032 100644 --- a/ElementX/Sources/Other/Pills/MentionBuilder.swift +++ b/ElementX/Sources/Other/Pills/MentionBuilder.swift @@ -37,13 +37,13 @@ struct MentionBuilder: MentionBuilderProtocol { return } - var attributesToAdd: [NSAttributedString.Key: Any] = [.link: url, .MatrixUserID: userID] + var attachmentAttributes: [NSAttributedString.Key: Any] = [.link: url, .MatrixUserID: userID] if let blockquote { // mentions can be in blockquotes, so if the replaced string was in one, we keep the attribute - attributesToAdd[.MatrixBlockquote] = blockquote + attachmentAttributes[.MatrixBlockquote] = blockquote } let attachmentString = NSMutableAttributedString(attachment: attachment) - attachmentString.addAttributes(attributes, range: NSRange(location: 0, length: attachmentString.length)) + attachmentString.addAttributes(attachmentAttributes, range: NSRange(location: 0, length: attachmentString.length)) attributedString.replaceCharacters(in: range, with: attachmentString) } @@ -61,13 +61,13 @@ struct MentionBuilder: MentionBuilderProtocol { return } - var attributesToAdd: [NSAttributedString.Key: Any] = [:] + var attachmentAttributes: [NSAttributedString.Key: Any] = [:] if let blockquote { // mentions can be in blockquotes, so if the replaced string was in one, we keep the attribute - attributesToAdd[.MatrixBlockquote] = blockquote + attachmentAttributes[.MatrixBlockquote] = blockquote } let attachmentString = NSMutableAttributedString(attachment: attachment) - attachmentString.addAttributes(attributes, range: NSRange(location: 0, length: attachmentString.length)) + attachmentString.addAttributes(attachmentAttributes, range: NSRange(location: 0, length: attachmentString.length)) attributedString.replaceCharacters(in: range, with: attachmentString) } } diff --git a/UnitTests/Sources/AppRouteURLParserTests.swift b/UnitTests/Sources/AppRouteURLParserTests.swift index 7a01191a8..7a0326875 100644 --- a/UnitTests/Sources/AppRouteURLParserTests.swift +++ b/UnitTests/Sources/AppRouteURLParserTests.swift @@ -108,4 +108,16 @@ class AppRouteURLParserTests: XCTestCase { // Then the route shouldn't be considered valid and should be ignored. XCTAssertEqual(route, nil) } + + func testMatrixUserURL() { + let userID = "@test:matrix.org" + guard let url = URL(string: "\(appSettings.permalinkBaseURL)/#/\(userID)") else { + XCTFail("Invalid url") + return + } + + let route = appRouteURLParser.route(from: url) + + XCTAssertEqual(route, .roomMemberDetails(userID: userID)) + } } diff --git a/UnitTests/Sources/AttributedStringBuilderTests.swift b/UnitTests/Sources/AttributedStringBuilderTests.swift index 5e172f448..f1b29a7d3 100644 --- a/UnitTests/Sources/AttributedStringBuilderTests.swift +++ b/UnitTests/Sources/AttributedStringBuilderTests.swift @@ -417,8 +417,40 @@ class AttributedStringBuilderTests: XCTestCase { let string = "https://matrix.to/#/@test:matrix.org" let attributedStringFromHTML = attributedStringBuilder.fromHTML(string) XCTAssertNotNil(attributedStringFromHTML?.attachment) + XCTAssertNotNil(attributedStringFromHTML?.link) let attributedStringFromPlain = attributedStringBuilder.fromPlain(string) XCTAssertNotNil(attributedStringFromPlain?.attachment) + XCTAssertNotNil(attributedStringFromHTML?.link) + } + + func testUserMentionAtachmentInBlockQuotes() { + let link = "https://matrix.to/#/@test:matrix.org" + let string = "
hello \(link) how are you?
" + guard let attributedStringFromHTML = attributedStringBuilder.fromHTML(string) else { + XCTFail("Attributed string is nil") + return + } + + for run in attributedStringFromHTML.runs { + XCTAssertNotNil(run.blockquote) + } + + checkAttachment(attributedString: attributedStringFromHTML, expectedRuns: 3) + checkLinkIn(attributedString: attributedStringFromHTML, expectedLink: link, expectedRuns: 3) + } + + func testAllUsersMentionAtachmentInBlockQuotes() { + let string = "
hello @room how are you?
" + guard let attributedStringFromHTML = attributedStringBuilder.fromHTML(string) else { + XCTFail("Attributed string is nil") + return + } + + for run in attributedStringFromHTML.runs { + XCTAssertNotNil(run.blockquote) + } + + checkAttachment(attributedString: attributedStringFromHTML, expectedRuns: 3) } func testAllUsersMentionAttachment() { @@ -464,12 +496,65 @@ class AttributedStringBuilderTests: XCTestCase { XCTAssertNil(attributedStringFromHTML?.link) } + func testUserMentionIsIgnoredInCode() { + let htmlString = "
test https://matrix.org/#/@test:matrix.org test
" + let attributedStringFromHTML = attributedStringBuilder.fromHTML(htmlString) + XCTAssert(attributedStringFromHTML?.runs.count == 1) + XCTAssertNil(attributedStringFromHTML?.attachment) + } + func testAllUsersIsIgnoredInCode() { let htmlString = "
test @room test
" let attributedStringFromHTML = attributedStringBuilder.fromHTML(htmlString) XCTAssert(attributedStringFromHTML?.runs.count == 1) XCTAssertNil(attributedStringFromHTML?.attachment) } + + func testMultipleMentions() { + guard let url = URL(string: "https://matrix.to/#/@test:matrix.org") else { + XCTFail("Invalid url") + return + } + + let string = "Hello @room, but especially hello to you \(url)" + guard let attributedStringFromHTML = attributedStringBuilder.fromHTML(string) else { + XCTFail("Attributed string is nil") + return + } + + var foundAttachments = 0 + var foundLink: URL? + for run in attributedStringFromHTML.runs { + if run.attachment != nil { + foundAttachments += 1 + } + + if let link = run.link { + foundLink = link + } + } + XCTAssertEqual(foundLink, url) + XCTAssertEqual(foundAttachments, 2) + + guard let attributedStringFromPlain = attributedStringBuilder.fromPlain(string) else { + XCTFail("Attributed string is nil") + return + } + + foundAttachments = 0 + foundLink = nil + for run in attributedStringFromPlain.runs { + if run.attachment != nil { + foundAttachments += 1 + } + + if let link = run.link { + foundLink = link + } + } + XCTAssertEqual(foundLink, url) + XCTAssertEqual(foundAttachments, 2) + } // MARK: - Private @@ -489,7 +574,7 @@ class AttributedStringBuilderTests: XCTestCase { XCTFail("Couldn't find expected value.") } - private func checkAttachment(attributedString: AttributedString?, expectedRuns: Int) { + private func checkAttachment(attributedString: AttributedString?, expectedRuns: Int, expectedAttachments: Int = 1) { guard let attributedString else { XCTFail("Could not build the attributed string") return