Polls refinements (#1608)

* Refine focus behavior * Begin draft mode * Add alert on poll form * Add poll ended asset * Add fallback text for ended poll event * Cleanup * Fix assets * Remove poll feature flags * Fix UI tests * Fix ui tests * Refine discard poll alert * Remove unused import * Rename hasDraftContent -> hasContent * Restore createPoll-2 ref screenshots
2026-11-03 06:55:59 +00:00 · 2023-09-01 15:55:11 +02:00
parent 41662a5313
commit 9b313b814e
36 changed files with 458 additions and 89 deletions
@@ -39,9 +39,7 @@ final class AppSettings {
        case hasShownWelcomeScreen
        case notificationSettingsEnabled
        case swiftUITimelineEnabled
-        case pollsInTimeline
        case richTextEditorEnabled
-        case pollsCreationEnabled
    }
    
    private static var suiteName: String = InfoPlistReader.main.appGroupIdentifier
@@ -242,12 +240,6 @@ final class AppSettings {
    @UserPreference(key: UserDefaultsKeys.swiftUITimelineEnabled, defaultValue: false, storageType: .volatile)
    var swiftUITimelineEnabled

-    @UserPreference(key: UserDefaultsKeys.pollsInTimeline, defaultValue: false, storageType: .userDefaults(store))
-    var pollsInTimelineEnabled
-
    @UserPreference(key: UserDefaultsKeys.richTextEditorEnabled, defaultValue: false, storageType: .userDefaults(store))
    var richTextEditorEnabled
-
-    @UserPreference(key: UserDefaultsKeys.pollsCreationEnabled, defaultValue: false, storageType: .userDefaults(store))
-    var pollsCreationEnabled
 }
@@ -35,6 +35,7 @@ internal enum Asset {
    internal static let encryptionNormal = ImageAsset(name: "images/encryption-normal")
    internal static let encryptionTrusted = ImageAsset(name: "images/encryption-trusted")
    internal static let encryptionWarning = ImageAsset(name: "images/encryption-warning")
+    internal static let endedPoll = ImageAsset(name: "images/ended-poll")
    internal static let launchBackground = ImageAsset(name: "images/launch-background")
    internal static let launchLogo = ImageAsset(name: "images/launch-logo")
    internal static let locationMarker = ImageAsset(name: "images/location-marker")
@@ -42,6 +43,7 @@ internal enum Asset {
    internal static let locationPointerFull = ImageAsset(name: "images/location-pointer-full")
    internal static let locationPointer = ImageAsset(name: "images/location-pointer")
    internal static let timelineComposerSendMessage = ImageAsset(name: "images/timeline-composer-send-message")
+    internal static let timelineEndedPoll = ImageAsset(name: "images/timeline-ended-poll")
    internal static let timelinePollAttachment = ImageAsset(name: "images/timeline-poll-attachment")
    internal static let timelinePoll = ImageAsset(name: "images/timeline-poll")
    internal static let timelineReactionAddMore = ImageAsset(name: "images/timeline-reaction-add-more")
@@ -274,6 +274,8 @@ public enum L10n {
  public static var commonSuggestions: String { return L10n.tr("Localizable", "common_suggestions") }
  /// Syncing
  public static var commonSyncing: String { return L10n.tr("Localizable", "common_syncing") }
+  /// Text
+  public static var commonText: String { return L10n.tr("Localizable", "common_text") }
  /// Third-party notices
  public static var commonThirdPartyNotices: String { return L10n.tr("Localizable", "common_third_party_notices") }
  /// Topic
@@ -462,10 +464,16 @@ public enum L10n {
  public static var richTextEditorA11yAddAttachment: String { return L10n.tr("Localizable", "rich_text_editor_a11y_add_attachment") }
  /// Toggle bullet list
  public static var richTextEditorBulletList: String { return L10n.tr("Localizable", "rich_text_editor_bullet_list") }
+  /// Close formatting options
+  public static var richTextEditorCloseFormattingOptions: String { return L10n.tr("Localizable", "rich_text_editor_close_formatting_options") }
  /// Toggle code block
  public static var richTextEditorCodeBlock: String { return L10n.tr("Localizable", "rich_text_editor_code_block") }
  /// Message…
  public static var richTextEditorComposerPlaceholder: String { return L10n.tr("Localizable", "rich_text_editor_composer_placeholder") }
+  /// Create a link
+  public static var richTextEditorCreateLink: String { return L10n.tr("Localizable", "rich_text_editor_create_link") }
+  /// Edit link
+  public static var richTextEditorEditLink: String { return L10n.tr("Localizable", "rich_text_editor_edit_link") }
  /// Apply bold format
  public static var richTextEditorFormatBold: String { return L10n.tr("Localizable", "rich_text_editor_format_bold") }
  /// Apply italic format
@@ -484,10 +492,14 @@ public enum L10n {
  public static var richTextEditorLink: String { return L10n.tr("Localizable", "rich_text_editor_link") }
  /// Toggle numbered list
  public static var richTextEditorNumberedList: String { return L10n.tr("Localizable", "rich_text_editor_numbered_list") }
+  /// Open compose options
+  public static var richTextEditorOpenComposeOptions: String { return L10n.tr("Localizable", "rich_text_editor_open_compose_options") }
  /// Toggle quote
  public static var richTextEditorQuote: String { return L10n.tr("Localizable", "rich_text_editor_quote") }
  /// Unindent
  public static var richTextEditorUnindent: String { return L10n.tr("Localizable", "rich_text_editor_unindent") }
+  /// Link
+  public static var richTextEditorUrlPlaceholder: String { return L10n.tr("Localizable", "rich_text_editor_url_placeholder") }
  /// This is the beginning of %1$@.
  public static func roomTimelineBeginningOfRoom(_ p1: Any) -> String {
    return L10n.tr("Localizable", "room_timeline_beginning_of_room", String(describing: p1))
@@ -612,8 +624,10 @@ public enum L10n {
  public static func screenCreatePollAnswerHint(_ p1: Int) -> String {
    return L10n.tr("Localizable", "screen_create_poll_answer_hint", p1)
  }
-  /// Are you sure you would like to go back?
-  public static var screenCreatePollConfirmation: String { return L10n.tr("Localizable", "screen_create_poll_confirmation") }
+  /// Are you sure you want to discard this poll?
+  public static var screenCreatePollDiscardConfirmation: String { return L10n.tr("Localizable", "screen_create_poll_discard_confirmation") }
+  /// Discard Poll
+  public static var screenCreatePollDiscardConfirmationTitle: String { return L10n.tr("Localizable", "screen_create_poll_discard_confirmation_title") }
  /// Question or topic
  public static var screenCreatePollQuestionDesc: String { return L10n.tr("Localizable", "screen_create_poll_question_desc") }
  /// What is the poll about?
@@ -1508,23 +1508,23 @@ class RoomProxyMock: RoomProxyProtocol {
    }
    //MARK: - endPoll

-    var endPollPollStartIDCallsCount = 0
-    var endPollPollStartIDCalled: Bool {
-        return endPollPollStartIDCallsCount > 0
+    var endPollPollStartIDTextCallsCount = 0
+    var endPollPollStartIDTextCalled: Bool {
+        return endPollPollStartIDTextCallsCount > 0
    }
-    var endPollPollStartIDReceivedPollStartID: String?
-    var endPollPollStartIDReceivedInvocations: [String] = []
-    var endPollPollStartIDReturnValue: Result<Void, RoomProxyError>!
-    var endPollPollStartIDClosure: ((String) async -> Result<Void, RoomProxyError>)?
+    var endPollPollStartIDTextReceivedArguments: (pollStartID: String, text: String)?
+    var endPollPollStartIDTextReceivedInvocations: [(pollStartID: String, text: String)] = []
+    var endPollPollStartIDTextReturnValue: Result<Void, RoomProxyError>!
+    var endPollPollStartIDTextClosure: ((String, String) async -> Result<Void, RoomProxyError>)?

-    func endPoll(pollStartID: String) async -> Result<Void, RoomProxyError> {
-        endPollPollStartIDCallsCount += 1
-        endPollPollStartIDReceivedPollStartID = pollStartID
-        endPollPollStartIDReceivedInvocations.append(pollStartID)
-        if let endPollPollStartIDClosure = endPollPollStartIDClosure {
-            return await endPollPollStartIDClosure(pollStartID)
+    func endPoll(pollStartID: String, text: String) async -> Result<Void, RoomProxyError> {
+        endPollPollStartIDTextCallsCount += 1
+        endPollPollStartIDTextReceivedArguments = (pollStartID: pollStartID, text: text)
+        endPollPollStartIDTextReceivedInvocations.append((pollStartID: pollStartID, text: text))
+        if let endPollPollStartIDTextClosure = endPollPollStartIDTextClosure {
+            return await endPollPollStartIDTextClosure(pollStartID, text)
        } else {
-            return endPollPollStartIDReturnValue
+            return endPollPollStartIDTextReturnValue
        }
    }
 }
@@ -66,13 +66,11 @@ struct RoomAttachmentPicker: View {
                }
                .accessibilityIdentifier(A11yIdentifiers.roomScreen.attachmentPickerLocation)

-                if ServiceLocator.shared.settings.pollsCreationEnabled {
-                    Button {
-                        context.showAttachmentPopover = false
-                        context.send(viewAction: .displayPollForm)
-                    } label: {
-                        PickerLabel(title: L10n.screenRoomAttachmentSourcePoll, icon: Image(asset: Asset.Images.timelinePollAttachment))
-                    }
+                Button {
+                    context.showAttachmentPopover = false
+                    context.send(viewAction: .displayPollForm)
+                } label: {
+                    PickerLabel(title: L10n.screenRoomAttachmentSourcePoll, icon: Image(asset: Asset.Images.timelinePollAttachment))
                }
            }
            .padding(.top, isPresented ? 20 : 0)
@@ -39,6 +39,12 @@ struct CreatePollScreenViewStateBindings {
    var isCreateButtonDisabled: Bool {
        question.isEmpty || options.count < 2 || options.contains { $0.text.isEmpty }
    }
+
+    var hasContent: Bool {
+        !question.isEmpty || options.contains(where: { !$0.text.isEmpty }) || isUndisclosed
+    }
+
+    var alertInfo: AlertInfo<UUID>?
 }

 enum CreatePollScreenViewAction {
@@ -39,7 +39,15 @@ class CreatePollScreenViewModel: CreatePollScreenViewModelType, CreatePollScreen
                                        options: state.bindings.options.map(\.text),
                                        pollKind: state.bindings.isUndisclosed ? .undisclosed : .disclosed))
        case .cancel:
-            actionsSubject.send(.cancel)
+            if state.bindings.hasContent {
+                state.bindings.alertInfo = .init(id: .init(),
+                                                 title: L10n.screenCreatePollDiscardConfirmationTitle,
+                                                 message: L10n.screenCreatePollDiscardConfirmation,
+                                                 primaryButton: .init(title: L10n.actionCancel, action: nil),
+                                                 secondaryButton: .init(title: L10n.actionOk, action: { self.actionsSubject.send(.cancel) }))
+            } else {
+                actionsSubject.send(.cancel)
+            }
        case .deleteOption(let index):
            // fixes a crash that caused an index out of range when an option with the keyboard focus was deleted
            Task {
@@ -39,6 +39,8 @@ struct CreatePollScreen: View {
        .navigationBarTitleDisplayMode(.inline)
        .toolbar { toolbar }
        .animation(.elementDefault, value: context.options)
+        .interactiveDismissDisabled(context.viewState.bindings.hasContent)
+        .alert(item: $context.alertInfo)
    }

    // MARK: - Private
@@ -55,6 +57,10 @@ struct CreatePollScreen: View {
            .textFieldStyle(.compoundForm)
            .focused($focus, equals: .question)
            .accessibilityIdentifier(A11yIdentifiers.createPollScreen.question)
+            .onSubmit {
+                focus = context.options.indices.first.map { .option(index: $0) }
+            }
+            .submitLabel(.next)
        }
        .compoundFormSection()
    }
@@ -74,6 +80,11 @@ struct CreatePollScreen: View {
                    }
                    .focused($focus, equals: .option(index: index))
                    .accessibilityIdentifier(A11yIdentifiers.createPollScreen.optionID(index))
+                    .onSubmit {
+                        let nextOptionIndex = index == context.options.endIndex - 1 ? nil : index + 1
+                        focus = nextOptionIndex.map { .option(index: $0) }
+                    }
+                    .submitLabel(index == context.options.endIndex - 1 ? .done : .next)
                }
            }
            .onMove { offsets, toOffset in
@@ -83,6 +94,7 @@ struct CreatePollScreen: View {
            if context.options.count < context.viewState.maxNumberOfOptions {
                Button(L10n.screenCreatePollAddOptionBtn) {
                    context.send(viewAction: .addOption)
+                    focus = context.options.indices.last.map { .option(index: $0) }
                }
                .accessibilityIdentifier(A11yIdentifiers.createPollScreen.addOption)
            }
@@ -814,7 +814,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol

    private func endPoll(pollStartID: String) {
        Task {
-            let endPollResult = await roomProxy.endPoll(pollStartID: pollStartID)
+            let endPollResult = await roomProxy.endPoll(pollStartID: pollStartID,
+                                                        text: "The poll with event id: \(pollStartID) has ended")
            switch endPollResult {
            case .success:
                break
@@ -21,6 +21,7 @@ struct PollRoomTimelineView: View {
    @Environment(\.timelineStyle) var timelineStyle
    @EnvironmentObject private var context: RoomScreenViewModel.Context
    @ScaledMetric private var summaryPadding = 32
+    @ScaledMetric private var iconSize = 22

    var body: some View {
        TimelineStyler(timelineItem: timelineItem) {
@@ -58,7 +59,11 @@ struct PollRoomTimelineView: View {

    private var questionView: some View {
        HStack(alignment: .top, spacing: 12) {
-            Image(Asset.Images.timelinePoll.name)
+            let asset = poll.hasEnded ? Asset.Images.timelineEndedPoll : Asset.Images.timelinePoll
+
+            Image(asset.name)
+                .resizable()
+                .frame(width: iconSize, height: iconSize)

            Text(poll.question)
                .multilineTextAlignment(.leading)
@@ -97,19 +97,31 @@ enum TimelineItemMenuAction: Identifiable, Hashable {
    }
    
    /// The action's label.
+    @ViewBuilder
    var label: some View {
        switch self {
-        case .copy: return Label(L10n.actionCopy, systemImage: "doc.on.doc")
-        case .edit: return Label(L10n.actionEdit, systemImage: "pencil.line")
-        case .copyPermalink: return Label(L10n.actionCopyLinkToMessage, systemImage: "link")
-        case .reply: return Label(L10n.actionReply, systemImage: "arrowshape.turn.up.left")
-        case .forward: return Label(L10n.actionForward, systemImage: "arrowshape.turn.up.right")
-        case .redact: return Label(L10n.actionRemove, systemImage: "trash")
-        case .viewSource: return Label(L10n.actionViewSource, systemImage: "doc.text.below.ecg")
-        case .retryDecryption: return Label(L10n.actionRetryDecryption, systemImage: "arrow.down.message")
-        case .report: return Label(L10n.actionReportContent, systemImage: "exclamationmark.bubble")
-        case .react: return Label(L10n.actionReact, systemImage: "hand.thumbsup")
-        case .endPoll: return Label { Text(L10n.actionEndPoll) } icon: { Image.compound.check.resizable() }
+        case .copy:
+            Label(L10n.actionCopy, systemImage: "doc.on.doc")
+        case .edit:
+            Label(L10n.actionEdit, systemImage: "pencil.line")
+        case .copyPermalink:
+            Label(L10n.actionCopyLinkToMessage, systemImage: "link")
+        case .reply:
+            Label(L10n.actionReply, systemImage: "arrowshape.turn.up.left")
+        case .forward:
+            Label(L10n.actionForward, systemImage: "arrowshape.turn.up.right")
+        case .redact:
+            Label(L10n.actionRemove, systemImage: "trash")
+        case .viewSource:
+            Label(L10n.actionViewSource, systemImage: "doc.text.below.ecg")
+        case .retryDecryption:
+            Label(L10n.actionRetryDecryption, systemImage: "arrow.down.message")
+        case .report:
+            Label(L10n.actionReportContent, systemImage: "exclamationmark.bubble")
+        case .react:
+            Label(L10n.actionReact, systemImage: "hand.thumbsup")
+        case .endPoll:
+            Label { Text(L10n.actionEndPoll) } icon: { CompoundIcon(customImage: Asset.Images.endedPoll.swiftUIImage) }
        }
    }
 }
@@ -50,9 +50,7 @@ protocol DeveloperOptionsProtocol: AnyObject {
    var readReceiptsEnabled: Bool { get set }
    var notificationSettingsEnabled: Bool { get set }
    var swiftUITimelineEnabled: Bool { get set }
-    var pollsInTimelineEnabled: Bool { get set }
    var richTextEditorEnabled: Bool { get set }
-    var pollsCreationEnabled: Bool { get set }
 }

 extension AppSettings: DeveloperOptionsProtocol { }
@@ -60,16 +60,6 @@ struct DeveloperOptionsScreen: View {
                }
            }

-            Section("Polls") {
-                Toggle(isOn: $context.pollsInTimelineEnabled) {
-                    Text("View polls in timeline")
-                }
-
-                Toggle(isOn: $context.pollsCreationEnabled) {
-                    Text("View polls creation flow")
-                }
-            }
-
            Section("Rich Text Editor") {
                Toggle(isOn: $context.richTextEditorEnabled) {
                    Text("Use the Rich Text Editor")
@@ -712,10 +712,10 @@ class RoomProxy: RoomProxyProtocol {
        }
    }

-    func endPoll(pollStartID: String) async -> Result<Void, RoomProxyError> {
+    func endPoll(pollStartID: String, text: String) async -> Result<Void, RoomProxyError> {
        await Task.dispatch(on: .global()) {
            do {
-                return try .success(self.room.endPoll(pollStartId: pollStartID, text: .init(), txnId: genTransactionId()))
+                return try .success(self.room.endPoll(pollStartId: pollStartID, text: text, txnId: genTransactionId()))
            } catch {
                MXLog.error("Failed ending a poll: \(error), pollStartID: \(pollStartID)")
                return .failure(.failedEndingPoll)
@@ -173,7 +173,7 @@ protocol RoomProxyProtocol {

    func sendPollResponse(pollStartID: String, answers: [String]) async -> Result<Void, RoomProxyError>

-    func endPoll(pollStartID: String) async -> Result<Void, RoomProxyError>
+    func endPoll(pollStartID: String, text: String) async -> Result<Void, RoomProxyError>
 }

 extension RoomProxyProtocol {
@@ -69,9 +69,6 @@ struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol {
                                                       previousAvatarURLString: prevAvatarUrl,
                                                       isOutgoing: isOutgoing)
        case .poll(question: let question, kind: let kind, maxSelections: let maxSelections, answers: let answers, votes: let votes, endTime: let endTime):
-            guard ServiceLocator.shared.settings.pollsInTimelineEnabled else {
-                return nil
-            }
            return buildPollTimelineItem(question, kind, maxSelections, answers, votes, endTime, eventItemProxy, isOutgoing)
        }
    }