Address most PR comments.

Update Podfile.lock

.swiftlint.yml

@@ -56,9 +56,6 @@ function_body_length:
   warning: 100
   error: 150
 
-nesting:
-  type_level: 2
-
 # naming rules can set warnings/errors for min_length and max_length
 # additionally they can set excluded names
 type_name:

Podfile.lock

@@ -180,7 +180,7 @@ EXTERNAL SOURCES:
 
 CHECKOUT OPTIONS:
   AnalyticsEvents:
-    :commit: aac06956d45cb86ea2bbd7a21b20b14ba8899fcf
+    :commit: f1805ad7c3fafa7fd9c6e2eaa9e0165f8142ecd2
     :git: https://github.com/matrix-org/matrix-analytics-events.git
 
 SPEC CHECKSUMS:

Riot/Managers/Analytics/Analytics.swift

@@ -164,15 +164,28 @@ extension Analytics {
         client.screen(event)
     }
     
+    /// The the presentation of a screen without including a duration
+    /// - Parameter screen: The screen that was shown
+    func trackScreen(_ screen: AnalyticsScreen) {
+        trackScreen(screen, duration: nil)
+    }
+    
     /// Track an element that has been tapped
     /// - Parameters:
     ///   - tap: The element that was tapped
     ///   - index: The index of the element, if it's in a list of elements
-    func trackTap(_ tap: AnalyticsElement, index: Int?) {
+    func trackTap(_ tap: AnalyticsUIElement, index: Int?) {
         let event = AnalyticsEvent.Click(index: index, name: tap.elementName)
         client.capture(event)
     }
     
+    /// Track an element that has been tapped without including an index
+    /// - Parameters:
+    ///   - tap: The element that was tapped
+    func trackTap(_ tap: AnalyticsUIElement) {
+        trackTap(tap, index: nil)
+    }
+    
     /// Track an E2EE error that occurred
     /// - Parameters:
     ///   - reason: The error that occurred.

Riot/Managers/Analytics/AnalyticsEventHelpers.swift

@@ -1,100 +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
-import AnalyticsEvents
-
-// MARK: - Helpers
-
-extension MXTaskProfileName {
-    var analyticsName: AnalyticsEvent.PerformanceTimer.Name? {
-        switch self {
-        case .startupIncrementalSync:
-            return .StartupIncrementalSync
-        case .startupInitialSync:
-            return .StartupInitialSync
-        case .startupLaunchScreen:
-            return .StartupLaunchScreen
-        case .startupStorePreload:
-            return .StartupStorePreload
-        case .startupMountData:
-            return .StartupStoreReady
-        case .initialSyncRequest:
-            return .InitialSyncRequest
-        case .initialSyncParsing:
-            return .InitialSyncParsing
-        case .notificationsOpenEvent:
-            return .NotificationsOpenEvent
-        default:
-            return nil
-        }
-    }
-}
-
-extension __MXCallHangupReason {
-    var errorName: AnalyticsEvent.Error.Name {
-        switch self {
-        case .userHangup:
-            return .VoipUserHangup
-        case .inviteTimeout:
-            return .VoipInviteTimeout
-        case .iceFailed:
-            return .VoipIceFailed
-        case .iceTimeout:
-            return .VoipIceTimeout
-        case .userMediaFailed:
-            return .VoipUserMediaFailed
-        case .unknownError:
-            return .UnknownError
-        default:
-            return .UnknownError
-        }
-    }
-}
-
-extension DecryptionFailureReason {
-    var errorName: AnalyticsEvent.Error.Name {
-        switch self {
-        case .unspecified:
-            return .OlmUnspecifiedError
-        case .olmKeysNotSent:
-            return .OlmKeysNotSentError
-        case .olmIndexError:
-            return .OlmIndexError
-        case .unexpected:
-            return .UnknownError
-        }
-    }
-}
-
-extension AnalyticsEvent.JoinedRoom.RoomSize {
-    init?(memberCount: UInt) {
-        switch memberCount {
-        case 2:
-            self = .Two
-        case 3...10:
-            self = .ThreeToTen
-        case 11...100:
-            self = .ElevenToOneHundred
-        case 101...1000:
-            self = .OneHundredAndOneToAThousand
-        case 1001...:
-            self = .MoreThanAThousand
-        default:
-            return nil
-        }
-    }
-}

Riot/Managers/Analytics/AnalyticsSettings.swift

@@ -28,8 +28,11 @@ struct AnalyticsSettings {
     /// This is suggested to be a 128-bit hex encoded string.
     var id: String?
     
-    /// Unused on iOS but necessary to load the value in case opt in was declined on web,
-    /// but accepted on iOS. Otherwise generating an ID would wipe out the existing value.
+    /// Whether the user has opted in on web or not. This is unused on iOS but necessary
+    /// to store here so that it's value is preserved when updating the account data if we
+    /// generated an ID on iOS.
+    ///
+    /// `true` if opted in on web, `false` if opted out on web and `nil` if the web prompt is not yet seen.
     private var webOptIn: Bool?
     
     /// Generate a new random analytics ID. This method has no effect if an ID already exists.
@@ -40,7 +43,8 @@ struct AnalyticsSettings {
 }
 
 extension AnalyticsSettings {
-    init(dictionary: Dictionary<AnyHashable, Any>?) {
+    // Private as AnalyticsSettings should only be created from an MXSession
+    private init(dictionary: Dictionary<AnyHashable, Any>?) {
         self.id = dictionary?[Constants.idKey] as? String
         self.webOptIn = dictionary?[Constants.webOptInKey] as? Bool
     }
@@ -54,6 +58,8 @@ extension AnalyticsSettings {
     }
 }
 
+// MARK: - Public initializer
+
 extension AnalyticsSettings {
     init(session: MXSession) {
         self.init(dictionary: session.accountData.accountData(forEventType: AnalyticsSettings.eventType))

Riot/Managers/Analytics/AnalyticsUIElement.swift

@@ -17,12 +17,14 @@
 import AnalyticsEvents
 
 /// A tappable UI element that can be track in Analytics.
-@objc enum AnalyticsElement: Int {
+@objc enum AnalyticsUIElement: Int {
     case sendMessageButton
     
     /// The element name reported to the AnalyticsEvent.
     var elementName: AnalyticsEvent.Click.Name {
         switch self {
+        // Note: This is a test element that doesn't need to be captured.
+        // It will likely be removed when the AnalyticsEvent.Click is updated.
         case .sendMessageButton:
             return .SendMessageButton
         }

Riot/Managers/Analytics/DecryptionFailure.swift

@@ -14,7 +14,7 @@
 // limitations under the License.
 //
 
-import Foundation
+import AnalyticsEvents
 
 /// Failure reasons as defined in https://docs.google.com/document/d/1es7cTCeJEXXfRCTRgZerAM2Wg5ZerHjvlpfTW-gsOfI.
 @objc enum DecryptionFailureReason: Int {
@@ -22,6 +22,19 @@ import Foundation
     case olmKeysNotSent
     case olmIndexError
     case unexpected
+    
+    var errorName: AnalyticsEvent.Error.Name {
+        switch self {
+        case .unspecified:
+            return .OlmUnspecifiedError
+        case .olmKeysNotSent:
+            return .OlmKeysNotSentError
+        case .olmIndexError:
+            return .OlmIndexError
+        case .unexpected:
+            return .UnknownError
+        }
+    }
 }
 
 /// `DecryptionFailure` represents a decryption failure.

Riot/Managers/Analytics/Helpers/JoinedRoomSize+MemberCount.swift

@@ -0,0 +1,36 @@
+// 
+// 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 AnalyticsEvents
+
+extension AnalyticsEvent.JoinedRoom.RoomSize {
+    init?(memberCount: UInt) {
+        switch memberCount {
+        case 2:
+            self = .Two
+        case 3...10:
+            self = .ThreeToTen
+        case 11...100:
+            self = .ElevenToOneHundred
+        case 101...1000:
+            self = .OneHundredAndOneToAThousand
+        case 1001...:
+            self = .MoreThanAThousand
+        default:
+            return nil
+        }
+    }
+}

Riot/Managers/Analytics/Helpers/MXCallHangupReason+Analytics.swift

@@ -0,0 +1,38 @@
+// 
+// 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 AnalyticsEvents
+
+extension __MXCallHangupReason {
+    var errorName: AnalyticsEvent.Error.Name {
+        switch self {
+        case .userHangup:
+            return .VoipUserHangup
+        case .inviteTimeout:
+            return .VoipInviteTimeout
+        case .iceFailed:
+            return .VoipIceFailed
+        case .iceTimeout:
+            return .VoipIceTimeout
+        case .userMediaFailed:
+            return .VoipUserMediaFailed
+        case .unknownError:
+            return .UnknownError
+        default:
+            return .UnknownError
+        }
+    }
+}

Riot/Managers/Analytics/Helpers/MXTaskProfileName+Analytics.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 AnalyticsEvents
+
+extension MXTaskProfileName {
+    var analyticsName: AnalyticsEvent.PerformanceTimer.Name? {
+        switch self {
+        case .startupIncrementalSync:
+            return .StartupIncrementalSync
+        case .startupInitialSync:
+            return .StartupInitialSync
+        case .startupLaunchScreen:
+            return .StartupLaunchScreen
+        case .startupStorePreload:
+            return .StartupStorePreload
+        case .startupMountData:
+            return .StartupStoreReady
+        case .initialSyncRequest:
+            return .InitialSyncRequest
+        case .initialSyncParsing:
+            return .InitialSyncParsing
+        case .notificationsOpenEvent:
+            return .NotificationsOpenEvent
+        default:
+            return nil
+        }
+    }
+}

Riot/Modules/TabBar/TabBarCoordinator.swift

@@ -491,6 +491,10 @@ final class TabBarCoordinator: NSObject, TabBarCoordinatorType {
     private func presentAnalyticsPrompt(with session: MXSession) {
         let parameters = AnalyticsPromptCoordinatorParameters(session: session, navigationRouter: navigationRouter)
         let coordinator = AnalyticsPromptCoordinator(parameters: parameters)
+        coordinator.completion = { [weak self] in
+            self?.remove(childCoordinator: coordinator)
+        }
+        
         coordinator.start()
         add(childCoordinator: coordinator)
     }

Riot/SupportingFiles/Riot-Bridging-Header.h

@@ -45,6 +45,7 @@
 #import "RoomInputToolbarView.h"
 #import "NSArray+Element.h"
 #import "ShareItemSender.h"
+#import "HTMLFormatter.h"
 
 // MatrixKit common imports, shared with all targets
 #import "MatrixKit-Bridging-Header.h"

Riot/Utils/HTMLFormatter.h

@@ -0,0 +1,38 @@
+// 
+// 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/Foundation.h>
+#import <CoreGraphics/CoreGraphics.h>
+
+NS_ASSUME_NONNULL_BEGIN
+
+@interface HTMLFormatter : NSObject
+
+/** Builds an attributed string from a string containing html.
+ @param htmlString The html string to use.
+ @param allowedTags The html tags that should be allowed.
+ @param fontSize The default font size to use.
+ 
+ Note: It is recommended to include "p" and "body" tags in
+ `allowedTags` as these are often added when parsing.
+ */
+- (NSAttributedString * _Nonnull)formatHTML:(NSString * _Nonnull)htmlString
+                            withAllowedTags:(NSArray<NSString *> * _Nonnull)allowedTags
+                                   fontSize:(CGFloat)fontSize;
+
+@end
+
+NS_ASSUME_NONNULL_END

Riot/Utils/HTMLFormatter.m

@@ -0,0 +1,61 @@
+// 
+// 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 "HTMLFormatter.h"
+#import "GeneratedInterface-Swift.h"
+
+@implementation HTMLFormatter
+
+- (NSAttributedString *)formatHTML:(NSString *)htmlString withAllowedTags:(NSArray<NSString *> *)allowedTags fontSize:(CGFloat)fontSize
+{
+    // TODO: This method should be more general purpose and usable from MXKEventFormatter and GroupHomeViewController
+    // FIXME: The implementation is currently in Objective-C as there is a crash in the callback when implemented in Swift
+    UIFont *font = [UIFont systemFontOfSize:fontSize];
+    
+    // Do some sanitisation before finalizing the string
+    DTHTMLAttributedStringBuilderWillFlushCallback sanitizeCallback = ^(DTHTMLElement *element) {
+        [element sanitizeWith:allowedTags bodyFont:font imageHandler:nil];
+    };
+
+    NSDictionary *options = @{
+                              DTUseiOS6Attributes: @(YES),              // Enable it to be able to display the attributed string in a UITextView
+                              DTDefaultFontFamily: font.familyName,
+                              DTDefaultFontName: font.fontName,
+                              DTDefaultFontSize: @(font.pointSize),
+                              DTDefaultLinkDecoration: @(NO),
+                              DTWillFlushBlockCallBack: sanitizeCallback
+                              };
+
+    // Do not use the default HTML renderer of NSAttributedString because this method
+    // runs on the UI thread which we want to avoid because renderHTMLString is called
+    // most of the time from a background thread.
+    // Use DTCoreText HTML renderer instead.
+    // Using DTCoreText, which renders static string, helps to avoid code injection attacks
+    // that could happen with the default HTML renderer of NSAttributedString which is a
+    // webview.
+    NSAttributedString *string = [[NSAttributedString alloc] initWithHTMLData:[htmlString dataUsingEncoding:NSUTF8StringEncoding] options:options documentAttributes:NULL];
+        
+    // Apply additional treatments
+    string = [MXKTools removeDTCoreTextArtifacts:string];
+    
+    if (!string) {
+        return [[NSAttributedString alloc] initWithString:htmlString];
+    }
+    
+    return string;
+}
+
+@end

Riot/Utils/HTMLFormatter.swift

@@ -16,14 +16,14 @@
 
 import Foundation
 
-extension Tools {
+extension HTMLFormatter {
     /// Builds an attributed string by replacing a `%@` placeholder with the supplied link text and URL.
     /// - Parameters:
     ///   - string: The string to be formatted.
     ///   - link: The link text to be inserted.
     ///   - url: The URL to be linked to.
     /// - Returns: An attributed string.
-    static func format(_ string: String, with link: String, using url: URL) -> NSAttributedString {
+    func format(_ string: String, with link: String, using url: URL) -> NSAttributedString {
         let baseString = NSMutableAttributedString(string: string)
         let attributedLink = NSAttributedString(string: link, attributes: [.link: url])
         

Riot/Utils/Tools.h

@@ -59,14 +59,4 @@
  */
 + (NSAttributedString *)setTextColorAlpha:(CGFloat)alpha inAttributedString:(NSAttributedString*)attributedString;
 
-/** Builds an attributed string from a string containing html.
- @param htmlString The html string to use.
- @param allowedTags The html tags that should be allowed.
- 
- Note: It is recommended to include "p" and "body" tags in
- `allowedTags` as these are often added when parsing.
- */
-+ (NSAttributedString * _Nonnull)attributedStringFromHTML:(NSString * _Nonnull)htmlString
-                                          withAllowedTags:(NSArray<NSString *> * _Nonnull)allowedTags;
-
 @end

Riot/Utils/Tools.m

@@ -138,41 +138,4 @@
     return string;
 }
 
-+ (NSAttributedString *)attributedStringFromHTML:(NSString *)htmlString withAllowedTags:(NSArray<NSString *> *)allowedTags
-{
-    UIFont *font = [UIFont systemFontOfSize:[UIFont systemFontSize]];
-    
-    // Do some sanitisation before finalizing the string
-    DTHTMLAttributedStringBuilderWillFlushCallback sanitizeCallback = ^(DTHTMLElement *element) {
-        [element sanitizeWith:allowedTags bodyFont:font imageHandler:nil];
-    };
-
-    NSDictionary *options = @{
-                              DTUseiOS6Attributes: @(YES),              // Enable it to be able to display the attributed string in a UITextView
-                              DTDefaultFontFamily: font.familyName,
-                              DTDefaultFontName: font.fontName,
-                              DTDefaultFontSize: @(font.pointSize),
-                              DTDefaultLinkDecoration: @(NO),
-                              DTWillFlushBlockCallBack: sanitizeCallback
-                              };
-
-    // Do not use the default HTML renderer of NSAttributedString because this method
-    // runs on the UI thread which we want to avoid because renderHTMLString is called
-    // most of the time from a background thread.
-    // Use DTCoreText HTML renderer instead.
-    // Using DTCoreText, which renders static string, helps to avoid code injection attacks
-    // that could happen with the default HTML renderer of NSAttributedString which is a
-    // webview.
-    NSAttributedString *string = [[NSAttributedString alloc] initWithHTMLData:[htmlString dataUsingEncoding:NSUTF8StringEncoding] options:options documentAttributes:NULL];
-        
-    // Apply additional treatments
-    string = [MXKTools removeDTCoreTextArtifacts:string];
-    
-    if (!string) {
-        return [[NSAttributedString alloc] initWithString:htmlString];
-    }
-    
-    return string;
-}
-
 @end

RiotSwiftUI/Modules/AnalyticsPrompt/Coordinator/AnalyticsPromptCoordinator.swift

@@ -89,9 +89,11 @@ final class AnalyticsPromptCoordinator: Coordinator {
             case .enable:
                 Analytics.shared.optIn(with: self.parameters.session)
                 self.parameters.navigationRouter.dismissModule(animated: true, completion: nil)
+                self.completion?()
             case .disable:
                 Analytics.shared.optOut()
                 self.parameters.navigationRouter.dismissModule(animated: true, completion: nil)
+                self.completion?()
             }
         }
     }

RiotSwiftUI/Modules/AnalyticsPrompt/Coordinator/AnalyticsPromptStrings.swift

@@ -14,20 +14,20 @@
 // limitations under the License.
 //
 
-import DTCoreText
+import Foundation
 
 @available(iOS 14.0, *)
 struct AnalyticsPromptStrings: AnalyticsPromptStringsProtocol {
     let appDisplayName = AppInfo.current.displayName
     
-    let point1 = Tools.attributedString(fromHTML: VectorL10n.analyticsPromptPoint1, withAllowedTags: ["b", "p"])
-    let point2 = Tools.attributedString(fromHTML: VectorL10n.analyticsPromptPoint2, withAllowedTags: ["b", "p"])
+    let point1 = HTMLFormatter().formatHTML(VectorL10n.analyticsPromptPoint1, withAllowedTags: ["b", "p"], fontSize: UIFont.systemFontSize)
+    let point2 = HTMLFormatter().formatHTML(VectorL10n.analyticsPromptPoint2, withAllowedTags: ["b", "p"], fontSize: UIFont.systemFontSize)
     
-    let termsNewUser = Tools.format(VectorL10n.analyticsPromptTermsNewUser("%@"),
-                                   with: VectorL10n.analyticsPromptTermsLinkNewUser,
-                                   using: BuildSettings.analyticsTermsURL)
-    let termsUpgrade = Tools.format(VectorL10n.analyticsPromptTermsUpgrade("%@"),
-                                   with: VectorL10n.analyticsPromptTermsLinkUpgrade,
-                                   using: BuildSettings.analyticsTermsURL)
+    let termsNewUser = HTMLFormatter().format(VectorL10n.analyticsPromptTermsNewUser("%@"),
+                                              with: VectorL10n.analyticsPromptTermsLinkNewUser,
+                                              using: BuildSettings.analyticsTermsURL)
+    let termsUpgrade = HTMLFormatter().format(VectorL10n.analyticsPromptTermsUpgrade("%@"),
+                                              with: VectorL10n.analyticsPromptTermsLinkUpgrade,
+                                              using: BuildSettings.analyticsTermsURL)
 }