Seamlessly switch for the RichTextEditor based message composer to the… (#2753)

* Seemlesly switch for the RichTextEditor based message composer to the plain one depending on whether formatting options are enabled or not.

* Address PR comments

* Fixes #2803 - Add extra padding at the bottom of the composer suggestions list

* Update preview test snapshots

* Update UI test snapshots
This commit is contained in:
Stefan Ceriu 2024-05-08 17:57:32 +03:00 committed by GitHub
parent 75ac523fa2
commit 636274df74
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
59 changed files with 132 additions and 160 deletions

View File

@ -36,7 +36,6 @@ final class AppSettings {
case pusherProfileTag case pusherProfileTag
case logLevel case logLevel
case viewSourceEnabled case viewSourceEnabled
case richTextEditorEnabled
case appAppearance case appAppearance
case sharePresence case sharePresence
case hideUnreadMessagesBadge case hideUnreadMessagesBadge
@ -234,9 +233,6 @@ final class AppSettings {
@UserPreference(key: UserDefaultsKeys.viewSourceEnabled, defaultValue: false, storageType: .userDefaults(store)) @UserPreference(key: UserDefaultsKeys.viewSourceEnabled, defaultValue: false, storageType: .userDefaults(store))
var viewSourceEnabled var viewSourceEnabled
@UserPreference(key: UserDefaultsKeys.richTextEditorEnabled, defaultValue: true, storageType: .userDefaults(store))
var richTextEditorEnabled
// MARK: - Element Call // MARK: - Element Call
@UserPreference(key: UserDefaultsKeys.elementCallBaseURL, defaultValue: "https://call.element.io", storageType: .userDefaults(store)) @UserPreference(key: UserDefaultsKeys.elementCallBaseURL, defaultValue: "https://call.element.io", storageType: .userDefaults(store))

View File

@ -60,6 +60,7 @@ enum ComposerToolbarViewAction {
case voiceMessage(ComposerToolbarVoiceMessageAction) case voiceMessage(ComposerToolbarVoiceMessageAction)
case plainComposerTextChanged case plainComposerTextChanged
case didToggleFormattingOptions
} }
enum ComposerAttachmentType { enum ComposerAttachmentType {
@ -95,7 +96,7 @@ struct ComposerToolbarViewState: BindableState {
case .previewVoiceMessage: case .previewVoiceMessage:
return true return true
default: default:
if ServiceLocator.shared.settings.richTextEditorEnabled { if bindings.composerFormattingEnabled {
return !composerEmpty return !composerEmpty
} else { } else {
return !bindings.plainComposerText.string.isEmpty return !bindings.plainComposerText.string.isEmpty
@ -108,7 +109,7 @@ struct ComposerToolbarViewState: BindableState {
return false return false
} }
if ServiceLocator.shared.settings.richTextEditorEnabled { if bindings.composerFormattingEnabled {
return composerEmpty return composerEmpty
} else { } else {
return bindings.plainComposerText.string.isEmpty return bindings.plainComposerText.string.isEmpty
@ -128,7 +129,7 @@ struct ComposerToolbarViewState: BindableState {
struct ComposerToolbarViewStateBindings { struct ComposerToolbarViewStateBindings {
var plainComposerText: NSAttributedString = .init(string: "") var plainComposerText: NSAttributedString = .init(string: "")
var composerFocused = false var composerFocused = false
var composerActionsEnabled = false var composerFormattingEnabled = false
var composerExpanded = false var composerExpanded = false
var formatItems: [FormatItem] = .init() var formatItems: [FormatItem] = .init()
var alertInfo: AlertInfo<UUID>? var alertInfo: AlertInfo<UUID>?

View File

@ -125,10 +125,9 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
case .previewVoiceMessage: case .previewVoiceMessage:
actionsSubject.send(.voiceMessage(.send)) actionsSubject.send(.voiceMessage(.send))
default: default:
if ServiceLocator.shared.settings.richTextEditorEnabled { if context.composerFormattingEnabled {
let sendHTML = appSettings.richTextEditorEnabled
actionsSubject.send(.sendMessage(plain: wysiwygViewModel.content.markdown, actionsSubject.send(.sendMessage(plain: wysiwygViewModel.content.markdown,
html: sendHTML ? wysiwygViewModel.content.html : nil, html: wysiwygViewModel.content.html,
mode: state.composerMode, mode: state.composerMode,
intentionalMentions: wysiwygViewModel.getMentionsState().toIntentionalMentions())) intentionalMentions: wysiwygViewModel.getMentionsState().toIntentionalMentions()))
} else { } else {
@ -148,7 +147,7 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
case .handlePasteOrDrop(let provider): case .handlePasteOrDrop(let provider):
actionsSubject.send(.handlePasteOrDrop(provider: provider)) actionsSubject.send(.handlePasteOrDrop(provider: provider))
case .enableTextFormatting: case .enableTextFormatting:
state.bindings.composerActionsEnabled = true state.bindings.composerFormattingEnabled = true
state.bindings.composerFocused = true state.bindings.composerFocused = true
case .composerAction(let action): case .composerAction(let action):
if action == .link { if action == .link {
@ -162,6 +161,15 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
processVoiceMessageAction(voiceMessageAction) processVoiceMessageAction(voiceMessageAction)
case .plainComposerTextChanged: case .plainComposerTextChanged:
completionSuggestionService.processTextMessage(state.bindings.plainComposerText.string) completionSuggestionService.processTextMessage(state.bindings.plainComposerText.string)
case .didToggleFormattingOptions:
if context.composerFormattingEnabled {
DispatchQueue.main.async {
self.wysiwygViewModel.textView.flushPills()
self.wysiwygViewModel.setMarkdownContent(self.context.plainComposerText.string)
}
} else {
context.plainComposerText = NSAttributedString(string: wysiwygViewModel.attributedContent.text.string)
}
} }
} }
@ -244,7 +252,7 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
private func processVoiceMessageAction(_ action: ComposerToolbarVoiceMessageAction) { private func processVoiceMessageAction(_ action: ComposerToolbarVoiceMessageAction) {
switch action { switch action {
case .startRecording: case .startRecording:
state.bindings.composerActionsEnabled = false state.bindings.composerFormattingEnabled = false
actionsSubject.send(.voiceMessage(.startRecording)) actionsSubject.send(.voiceMessage(.startRecording))
case .stopRecording: case .stopRecording:
actionsSubject.send(.voiceMessage(.stopRecording)) actionsSubject.send(.voiceMessage(.stopRecording))
@ -297,7 +305,7 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
return return
} }
if appSettings.richTextEditorEnabled { if context.composerFormattingEnabled {
wysiwygViewModel.setMention(url: url.absoluteString, name: item.displayName ?? item.id, mentionType: .user) wysiwygViewModel.setMention(url: url.absoluteString, name: item.displayName ?? item.id, mentionType: .user)
} else { } else {
let attributedString = NSMutableAttributedString(attributedString: state.bindings.plainComposerText) let attributedString = NSMutableAttributedString(attributedString: state.bindings.plainComposerText)
@ -305,7 +313,7 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
state.bindings.plainComposerText = attributedString state.bindings.plainComposerText = attributedString
} }
case .allUsers: case .allUsers:
if appSettings.richTextEditorEnabled { if context.composerFormattingEnabled {
wysiwygViewModel.setAtRoomMention() wysiwygViewModel.setAtRoomMention()
} else { } else {
let attributedString = NSMutableAttributedString(attributedString: state.bindings.plainComposerText) let attributedString = NSMutableAttributedString(attributedString: state.bindings.plainComposerText)
@ -333,7 +341,7 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
} }
private func set(text: String) { private func set(text: String) {
if ServiceLocator.shared.settings.richTextEditorEnabled { if context.composerFormattingEnabled {
wysiwygViewModel.textView.flushPills() wysiwygViewModel.textView.flushPills()
wysiwygViewModel.setHtmlContent(text) wysiwygViewModel.setHtmlContent(text)

View File

@ -52,6 +52,7 @@ struct CompletionSuggestionView: View {
list() list()
} }
} }
.padding(.bottom, Constants.listItemPadding)
} }
} }

View File

@ -34,7 +34,7 @@ struct ComposerToolbar: View {
VStack(spacing: 8) { VStack(spacing: 8) {
topBar topBar
if context.composerActionsEnabled { if context.composerFormattingEnabled {
if verticalSizeClass != .compact, if verticalSizeClass != .compact,
context.composerExpanded { context.composerExpanded {
suggestionView suggestionView
@ -68,7 +68,7 @@ struct ComposerToolbar: View {
topBarLayout { topBarLayout {
mainTopBarContent mainTopBarContent
if !context.composerActionsEnabled { if !context.composerFormattingEnabled {
if context.viewState.isUploading { if context.viewState.isUploading {
ProgressView() ProgressView()
.scaledFrame(size: 44, relativeTo: .title) .scaledFrame(size: 44, relativeTo: .title)
@ -106,7 +106,7 @@ struct ComposerToolbar: View {
private var mainTopBarContent: some View { private var mainTopBarContent: some View {
ZStack(alignment: .bottom) { ZStack(alignment: .bottom) {
topBarLayout { topBarLayout {
if !context.composerActionsEnabled { if !context.composerFormattingEnabled {
RoomAttachmentPicker(context: context) RoomAttachmentPicker(context: context)
} }
messageComposer messageComposer
@ -122,7 +122,7 @@ struct ComposerToolbar: View {
private var closeRTEButton: some View { private var closeRTEButton: some View {
Button { Button {
context.composerActionsEnabled = false context.composerFormattingEnabled = false
context.composerExpanded = false context.composerExpanded = false
} label: { } label: {
Image(Asset.Images.closeRte.name) Image(Asset.Images.closeRte.name)
@ -158,7 +158,8 @@ struct ComposerToolbar: View {
MessageComposer(plainComposerText: $context.plainComposerText, MessageComposer(plainComposerText: $context.plainComposerText,
composerView: composerView, composerView: composerView,
mode: context.viewState.composerMode, mode: context.viewState.composerMode,
showResizeGrabber: context.viewState.bindings.composerActionsEnabled, composerFormattingEnabled: context.composerFormattingEnabled,
showResizeGrabber: context.composerFormattingEnabled,
isExpanded: $context.composerExpanded) { isExpanded: $context.composerExpanded) {
context.send(viewAction: .sendMessage) context.send(viewAction: .sendMessage)
} editAction: { } editAction: {
@ -179,8 +180,8 @@ struct ComposerToolbar: View {
} }
.environmentObject(context) .environmentObject(context)
.focused($composerFocused) .focused($composerFocused)
.padding(.leading, context.composerActionsEnabled ? 7 : 0) .padding(.leading, context.composerFormattingEnabled ? 7 : 0)
.padding(.trailing, context.composerActionsEnabled ? 4 : 0) .padding(.trailing, context.composerFormattingEnabled ? 4 : 0)
.onTapGesture { .onTapGesture {
guard !composerFocused else { return } guard !composerFocused else { return }
composerFocused = true composerFocused = true
@ -196,6 +197,9 @@ struct ComposerToolbar: View {
.onChange(of: context.plainComposerText) { _ in .onChange(of: context.plainComposerText) { _ in
context.send(viewAction: .plainComposerTextChanged) context.send(viewAction: .plainComposerTextChanged)
} }
.onChange(of: context.composerFormattingEnabled) { _ in
context.send(viewAction: .didToggleFormattingOptions)
}
.onAppear { .onAppear {
composerFocused = context.composerFocused composerFocused = context.composerFocused
} }

View File

@ -25,6 +25,7 @@ struct MessageComposer: View {
@Binding var plainComposerText: NSAttributedString @Binding var plainComposerText: NSAttributedString
let composerView: WysiwygComposerView let composerView: WysiwygComposerView
let mode: RoomScreenComposerMode let mode: RoomScreenComposerMode
let composerFormattingEnabled: Bool
let showResizeGrabber: Bool let showResizeGrabber: Bool
@Binding var isExpanded: Bool @Binding var isExpanded: Bool
let sendAction: () -> Void let sendAction: () -> Void
@ -68,7 +69,7 @@ struct MessageComposer: View {
VStack(alignment: .leading, spacing: -6) { VStack(alignment: .leading, spacing: -6) {
header header
if ServiceLocator.shared.settings.richTextEditorEnabled { if composerFormattingEnabled {
Color.clear Color.clear
.overlay(alignment: .top) { .overlay(alignment: .top) {
composerView composerView
@ -254,6 +255,7 @@ struct MessageComposer_Previews: PreviewProvider, TestablePreview {
return MessageComposer(plainComposerText: .constant(content), return MessageComposer(plainComposerText: .constant(content),
composerView: composerView, composerView: composerView,
mode: mode, mode: mode,
composerFormattingEnabled: false,
showResizeGrabber: false, showResizeGrabber: false,
isExpanded: .constant(false), isExpanded: .constant(false),
sendAction: { }, sendAction: { },

View File

@ -36,7 +36,8 @@ struct MessageComposerTextField: View {
private var placeholderView: some View { private var placeholderView: some View {
if text.string.isEmpty { if text.string.isEmpty {
Text(placeholder) Text(placeholder)
.foregroundColor(.compound.textPlaceholder) .font(Font(UIFont.preferredFont(forTextStyle: .body)))
.foregroundColor(.compound.textSecondary)
.accessibilityHidden(true) .accessibilityHidden(true)
} }
} }

View File

@ -37,15 +37,13 @@ struct RoomAttachmentPicker: View {
var menuContent: some View { var menuContent: some View {
VStack(alignment: .leading, spacing: 0.0) { VStack(alignment: .leading, spacing: 0.0) {
if ServiceLocator.shared.settings.richTextEditorEnabled { Button {
Button { context.send(viewAction: .enableTextFormatting)
context.send(viewAction: .enableTextFormatting) } label: {
} label: { Label(L10n.screenRoomAttachmentTextFormatting, icon: \.textFormatting)
Label(L10n.screenRoomAttachmentTextFormatting, icon: \.textFormatting) .labelStyle(.menuSheet)
.labelStyle(.menuSheet)
}
.accessibilityIdentifier(A11yIdentifiers.roomScreen.attachmentPickerTextFormatting)
} }
.accessibilityIdentifier(A11yIdentifiers.roomScreen.attachmentPickerTextFormatting)
Button { Button {
context.send(viewAction: .attach(.poll)) context.send(viewAction: .attach(.poll))

View File

@ -272,18 +272,10 @@ class RoomScreenInteractionHandler {
private func processEditMessageEvent(_ messageTimelineItem: EventBasedMessageTimelineItemProtocol) { private func processEditMessageEvent(_ messageTimelineItem: EventBasedMessageTimelineItemProtocol) {
let text: String let text: String
switch messageTimelineItem.contentType { switch messageTimelineItem.contentType {
case .text(let textItem): case .text:
if ServiceLocator.shared.settings.richTextEditorEnabled, let formattedBodyHTMLString = textItem.formattedBodyHTMLString { text = messageTimelineItem.body
text = formattedBodyHTMLString case .emote:
} else { text = "/me " + messageTimelineItem.body
text = messageTimelineItem.body
}
case .emote(let emoteItem):
if ServiceLocator.shared.settings.richTextEditorEnabled, let formattedBodyHTMLString = emoteItem.formattedBodyHTMLString {
text = "/me " + formattedBodyHTMLString
} else {
text = "/me " + messageTimelineItem.body
}
default: default:
text = messageTimelineItem.body text = messageTimelineItem.body
} }

View File

@ -35,9 +35,9 @@ struct RoomScreen: View {
.background(Color.compound.bgCanvasDefault.ignoresSafeArea()) .background(Color.compound.bgCanvasDefault.ignoresSafeArea())
.safeAreaInset(edge: .bottom, spacing: 0) { .safeAreaInset(edge: .bottom, spacing: 0) {
composerToolbar composerToolbar
.padding(.bottom, composerToolbarContext.composerActionsEnabled ? 8 : 12) .padding(.bottom, composerToolbarContext.composerFormattingEnabled ? 8 : 12)
.background { .background {
if composerToolbarContext.composerActionsEnabled { if composerToolbarContext.composerFormattingEnabled {
RoundedRectangle(cornerRadius: 20) RoundedRectangle(cornerRadius: 20)
.stroke(Color.compound.borderInteractiveSecondary, lineWidth: 0.5) .stroke(Color.compound.borderInteractiveSecondary, lineWidth: 0.5)
.ignoresSafeArea() .ignoresSafeArea()
@ -186,7 +186,7 @@ struct RoomScreen: View {
} }
private var isNavigationBarHidden: Bool { private var isNavigationBarHidden: Bool {
composerToolbarContext.composerActionsEnabled && composerToolbarContext.composerExpanded && UIDevice.current.userInterfaceIdiom == .pad composerToolbarContext.composerFormattingEnabled && composerToolbarContext.composerExpanded && UIDevice.current.userInterfaceIdiom == .pad
} }
} }

View File

@ -40,7 +40,6 @@ enum AdvancedSettingsScreenViewAction { }
protocol AdvancedSettingsProtocol: AnyObject { protocol AdvancedSettingsProtocol: AnyObject {
var timelineStyle: TimelineStyle { get set } var timelineStyle: TimelineStyle { get set }
var viewSourceEnabled: Bool { get set } var viewSourceEnabled: Bool { get set }
var richTextEditorEnabled: Bool { get set }
var appAppearance: AppAppearance { get set } var appAppearance: AppAppearance { get set }
var sharePresence: Bool { get set } var sharePresence: Bool { get set }
} }

View File

@ -31,9 +31,6 @@ struct AdvancedSettingsScreen: View {
kind: .picker(selection: $context.timelineStyle, kind: .picker(selection: $context.timelineStyle,
items: TimelineStyle.allCases.map { (title: $0.name, tag: $0) })) items: TimelineStyle.allCases.map { (title: $0.name, tag: $0) }))
ListRow(label: .plain(title: L10n.commonRichTextEditor),
kind: .toggle($context.richTextEditorEnabled))
ListRow(label: .plain(title: L10n.actionViewSource), ListRow(label: .plain(title: L10n.actionViewSource),
kind: .toggle($context.viewSourceEnabled)) kind: .toggle($context.viewSourceEnabled))

View File

@ -487,9 +487,8 @@ class MockScreen: Identifiable {
var sessionVerificationControllerProxy = SessionVerificationControllerProxyMock.configureMock(requestDelay: .seconds(5)) var sessionVerificationControllerProxy = SessionVerificationControllerProxyMock.configureMock(requestDelay: .seconds(5))
let parameters = SessionVerificationScreenCoordinatorParameters(sessionVerificationControllerProxy: sessionVerificationControllerProxy) let parameters = SessionVerificationScreenCoordinatorParameters(sessionVerificationControllerProxy: sessionVerificationControllerProxy)
return SessionVerificationScreenCoordinator(parameters: parameters) return SessionVerificationScreenCoordinator(parameters: parameters)
case .userSessionScreen, .userSessionScreenReply, .userSessionScreenRTE: case .userSessionScreen, .userSessionScreenReply:
let appSettings: AppSettings = ServiceLocator.shared.settings let appSettings: AppSettings = ServiceLocator.shared.settings
appSettings.richTextEditorEnabled = id == .userSessionScreenRTE
appSettings.hasRunIdentityConfirmationOnboarding = true appSettings.hasRunIdentityConfirmationOnboarding = true
appSettings.hasRunNotificationPermissionsOnboarding = true appSettings.hasRunNotificationPermissionsOnboarding = true
appSettings.analyticsConsentState = .optedOut appSettings.analyticsConsentState = .optedOut

View File

@ -53,7 +53,6 @@ enum UITestsScreenIdentifier: String {
case templateScreen case templateScreen
case userSessionScreen case userSessionScreen
case userSessionScreenReply case userSessionScreenReply
case userSessionScreenRTE
} }
extension UITestsScreenIdentifier: CustomStringConvertible { extension UITestsScreenIdentifier: CustomStringConvertible {

View File

@ -45,20 +45,6 @@ class UserSessionScreenTests: XCTestCase {
try await app.assertScreenshot(.userSessionScreenReply) try await app.assertScreenshot(.userSessionScreenReply)
} }
func testUserSessionRTE() async throws {
let app = Application.launch(.userSessionScreenRTE)
app.buttons[A11yIdentifiers.homeScreen.roomName(firstRoomName)].tap()
XCTAssert(app.staticTexts[firstRoomName].waitForExistence(timeout: 5.0))
try await Task.sleep(for: .seconds(1))
app.buttons[A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions].forceTap()
try await app.assertScreenshot(.userSessionScreenRTE, step: 1)
app.buttons[A11yIdentifiers.roomScreen.attachmentPickerTextFormatting].tap()
try await app.assertScreenshot(.userSessionScreenRTE, step: 2)
}
func testElementCall() async throws { func testElementCall() async throws {
let app = Application.launch(.userSessionScreen) let app = Application.launch(.userSessionScreen)

View File

@ -30,7 +30,6 @@ class ComposerToolbarViewModelTests: XCTestCase {
override func setUp() { override func setUp() {
AppSettings.resetAllSettings() AppSettings.resetAllSettings()
appSettings = AppSettings() appSettings = AppSettings()
appSettings.richTextEditorEnabled = true
ServiceLocator.shared.register(appSettings: appSettings) ServiceLocator.shared.register(appSettings: appSettings)
wysiwygViewModel = WysiwygComposerViewModel() wysiwygViewModel = WysiwygComposerViewModel()
completionSuggestionServiceMock = CompletionSuggestionServiceMock(configuration: .init()) completionSuggestionServiceMock = CompletionSuggestionServiceMock(configuration: .init())
@ -39,6 +38,8 @@ class ComposerToolbarViewModelTests: XCTestCase {
mediaProvider: MockMediaProvider(), mediaProvider: MockMediaProvider(),
appSettings: appSettings, appSettings: appSettings,
mentionDisplayHelper: ComposerMentionDisplayHelper.mock) mentionDisplayHelper: ComposerMentionDisplayHelper.mock)
viewModel.context.composerFormattingEnabled = true
} }
override func tearDown() { override func tearDown() {
@ -94,7 +95,7 @@ class ComposerToolbarViewModelTests: XCTestCase {
XCTAssertTrue(viewModel.state.bindings.composerFocused) XCTAssertTrue(viewModel.state.bindings.composerFocused)
viewModel.state.composerEmpty = false viewModel.state.composerEmpty = false
viewModel.process(viewAction: .sendMessage) viewModel.process(viewAction: .sendMessage)
XCTAssertTrue(viewModel.state.bindings.composerActionsEnabled) XCTAssertTrue(viewModel.state.bindings.composerFormattingEnabled)
} }
func testAlertIsShownAfterLinkAction() { func testAlertIsShownAfterLinkAction() {