Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix message completion trigger to work anywhere in the message #3696

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ struct MediaUploadPreviewScreenViewState: BindableState {
struct MediaUploadPreviewScreenBindings: BindableState {
var caption = NSAttributedString()
var presendCallback: (() -> Void)?
var selectedRange = NSRange(location: 0, length: 0)

var isPresentingMediaCaptionWarning = false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ struct MediaUploadPreviewScreen: View {
MessageComposerTextField(placeholder: L10n.richTextEditorComposerCaptionPlaceholder,
text: $context.caption,
presendCallback: $context.presendCallback,
selectedRange: $context.selectedRange,
maxHeight: ComposerConstant.maxHeight,
keyHandler: handleKeyPress) { _ in }
.focused($isComposerFocussed)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol {
}
}

func processTextMessage(_ textMessage: String?) {
setSuggestionTrigger(detectTriggerInText(textMessage))
func processTextMessage(_ textMessage: String?, selectedRange: NSRange?) {
setSuggestionTrigger(detectTriggerInText(text: textMessage, selectedRange: selectedRange))
}

func setSuggestionTrigger(_ suggestionTrigger: SuggestionTrigger?) {
Expand All @@ -83,23 +83,46 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol {

// MARK: - Private

private func detectTriggerInText(_ text: String?) -> SuggestionTrigger? {
guard let text else {
private func detectTriggerInText(text: String?, selectedRange: NSRange?) -> SuggestionTrigger? {
vickcoo marked this conversation as resolved.
Show resolved Hide resolved
guard let text, let selectedRange else {
vickcoo marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

let components = text.components(separatedBy: .whitespaces)
var suggestionText: String?
var range: Range<String.Index>?

guard var lastComponent = components.last,
let range = text.range(of: lastComponent, options: .backwards),
lastComponent.count > 0,
let suggestionKey = SuggestionTriggerPattern(rawValue: lastComponent.removeFirst()),
// If a second character exists and is the same as the key it shouldn't trigger.
lastComponent.first != suggestionKey.rawValue else {
return nil
var startIndex: String.Index = text.startIndex
let components = text
.components(separatedBy: .whitespaces)
.filter { $0.first == SuggestionTriggerPattern.at.rawValue }

for component in components {
vickcoo marked this conversation as resolved.
Show resolved Hide resolved
guard let textRange = text.range(of: component, range: startIndex..<text.endIndex) else {
continue
}
startIndex = textRange.upperBound

let startIndex = textRange.lowerBound.utf16Offset(in: text)
let endIndex = textRange.upperBound.utf16Offset(in: text)

guard selectedRange.location >= startIndex,
selectedRange.location <= endIndex,
selectedRange.length <= endIndex - startIndex
else {
continue
}

suggestionText = String(text[textRange.lowerBound..<textRange.upperBound])
suggestionText?.removeFirst()
range = textRange
}

return .init(type: .user, text: lastComponent, range: NSRange(range, in: text))
guard let suggestionText,
let range else {
return nil
}

return .init(type: .user, text: suggestionText, range: NSRange(range, in: text))
}

private static func shouldIncludeMember(userID: String, displayName: String?, searchText: String) -> Bool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ struct MentionSuggestionItem: Identifiable, Equatable {
protocol CompletionSuggestionServiceProtocol {
var suggestionsPublisher: AnyPublisher<[SuggestionItem], Never> { get }

func processTextMessage(_ textMessage: String?)
func processTextMessage(_ textMessage: String?, selectedRange: NSRange?)

func setSuggestionTrigger(_ suggestionTrigger: SuggestionTrigger?)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ enum ComposerToolbarViewAction {

case plainComposerTextChanged
case didToggleFormattingOptions
case selectedTextChanged
}

enum ComposerAttachmentType {
Expand Down Expand Up @@ -131,6 +132,7 @@ struct ComposerToolbarViewStateBindings {
var composerExpanded = false
var formatItems: [FormatItem] = .init()
var alertInfo: AlertInfo<UUID>?
var selectedRange = NSRange(location: 0, length: 0)

var presendCallback: (() -> Void)?
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,9 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
case .voiceMessage(let voiceMessageAction):
processVoiceMessageAction(voiceMessageAction)
case .plainComposerTextChanged:
completionSuggestionService.processTextMessage(state.bindings.plainComposerText.string)
completionSuggestionService.processTextMessage(state.bindings.plainComposerText.string, selectedRange: context.viewState.bindings.selectedRange)
case .selectedTextChanged:
completionSuggestionService.processTextMessage(state.bindings.plainComposerText.string, selectedRange: context.viewState.bindings.selectedRange)
case .didToggleFormattingOptions:
if context.composerFormattingEnabled {
guard !context.plainComposerText.string.isEmpty else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ struct ComposerToolbar: View {
private var messageComposer: some View {
MessageComposer(plainComposerText: $context.plainComposerText,
presendCallback: $context.presendCallback,
selectedRange: $context.selectedRange,
composerView: composerView,
mode: context.viewState.composerMode,
composerFormattingEnabled: context.composerFormattingEnabled,
Expand Down Expand Up @@ -199,6 +200,9 @@ struct ComposerToolbar: View {
.onChange(of: context.composerFormattingEnabled) {
context.send(viewAction: .didToggleFormattingOptions)
}
.onChange(of: context.selectedRange) {
context.send(viewAction: .selectedTextChanged)
}
.onAppear {
composerFocused = context.composerFocused
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ typealias PasteHandler = (NSItemProvider) -> Void
struct MessageComposer: View {
@Binding var plainComposerText: NSAttributedString
@Binding var presendCallback: (() -> Void)?
@Binding var selectedRange: NSRange
let composerView: WysiwygComposerView
let mode: ComposerMode
let composerFormattingEnabled: Bool
Expand Down Expand Up @@ -66,6 +67,7 @@ struct MessageComposer: View {
MessageComposerTextField(placeholder: L10n.richTextEditorComposerPlaceholder,
text: $plainComposerText,
presendCallback: $presendCallback,
selectedRange: $selectedRange,
maxHeight: ComposerConstant.maxHeight,
keyHandler: { handleKeyPress($0) },
pasteHandler: pasteAction)
Expand Down Expand Up @@ -285,6 +287,7 @@ struct MessageComposer_Previews: PreviewProvider, TestablePreview {

return MessageComposer(plainComposerText: .constant(content),
presendCallback: .constant(nil),
selectedRange: .constant(NSRange(location: 0, length: 0)),
composerView: composerView,
mode: mode,
composerFormattingEnabled: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ struct MessageComposerTextField: View {
let placeholder: String
@Binding var text: NSAttributedString
@Binding var presendCallback: (() -> Void)?
@Binding var selectedRange: NSRange

let maxHeight: CGFloat
let keyHandler: GenericKeyHandler
Expand All @@ -20,6 +21,7 @@ struct MessageComposerTextField: View {
var body: some View {
UITextViewWrapper(text: $text,
presendCallback: $presendCallback,
selectedRange: $selectedRange,
maxHeight: maxHeight,
keyHandler: keyHandler,
pasteHandler: pasteHandler)
Expand Down Expand Up @@ -54,6 +56,7 @@ private struct UITextViewWrapper: UIViewRepresentable {

@Binding var text: NSAttributedString
@Binding var presendCallback: (() -> Void)?
@Binding var selectedRange: NSRange

let maxHeight: CGFloat

Expand Down Expand Up @@ -105,7 +108,9 @@ private struct UITextViewWrapper: UIViewRepresentable {
// Prevent the textView from inheriting attributes from mention pills
textView.typingAttributes = [.font: font,
.foregroundColor: UIColor.compound.textPrimary]

DispatchQueue.main.async {
selectedRange = textView.selectedRange
}
if textView.attributedText != text {
// Remember the selection if only the attributes have changed.
let selection = textView.attributedText.string == text.string ? textView.selectedTextRange : nil
Expand Down Expand Up @@ -331,6 +336,7 @@ struct MessageComposerTextField_Previews: PreviewProvider, TestablePreview {
MessageComposerTextField(placeholder: "Placeholder",
text: $text,
presendCallback: .constant(nil),
selectedRange: .constant(NSRange(location: 0, length: 0)),
maxHeight: 300,
keyHandler: { _ in },
pasteHandler: { _ in })
Expand Down
51 changes: 51 additions & 0 deletions UnitTests/Sources/CompletionSuggestionServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,57 @@ final class CompletionSuggestionServiceTests: XCTestCase {
service.setSuggestionTrigger(.init(type: .user, text: "", range: .init()))
try await deferred.fulfill()
}

func testUserSuggestionInDifferentMessagePositions() async throws {
let alice: RoomMemberProxyMock = .mockAlice
let members: [RoomMemberProxyMock] = [alice, .mockBob, .mockCharlie, .mockMe]
let roomProxyMock = JoinedRoomProxyMock(.init(name: "test", members: members))
let service = CompletionSuggestionService(roomProxy: roomProxyMock)

var deferred = deferFulfillment(service.suggestionsPublisher) { suggestion in
suggestion == [.user(item: .init(id: alice.userID, displayName: alice.displayName, avatarURL: alice.avatarURL, range: .init(location: 0, length: 3)))]
}
service.processTextMessage("@al hello", selectedRange: .init(location: 0, length: 1))
try await deferred.fulfill()

deferred = deferFulfillment(service.suggestionsPublisher) { suggestion in
suggestion == [.user(item: .init(id: alice.userID, displayName: alice.displayName, avatarURL: alice.avatarURL, range: .init(location: 5, length: 3)))]
}
service.processTextMessage("test @al", selectedRange: .init(location: 5, length: 1))
try await deferred.fulfill()

deferred = deferFulfillment(service.suggestionsPublisher) { suggestion in
suggestion == [.user(item: .init(id: alice.userID, displayName: alice.displayName, avatarURL: alice.avatarURL, range: .init(location: 5, length: 3)))]
}
service.processTextMessage("test @al test", selectedRange: .init(location: 5, length: 1))
try await deferred.fulfill()
}

func testUserSuggestionWithMultipleMentionSymbol() async throws {
let alice: RoomMemberProxyMock = .mockAlice
let bob: RoomMemberProxyMock = .mockBob
let members: [RoomMemberProxyMock] = [alice, bob, .mockCharlie, .mockMe]
let roomProxyMock = JoinedRoomProxyMock(.init(name: "test", members: members))
let service = CompletionSuggestionService(roomProxy: roomProxyMock)

var deffered = deferFulfillment(service.suggestionsPublisher) { suggestion in
suggestion == [.user(item: .init(id: alice.userID, displayName: alice.displayName, avatarURL: alice.avatarURL, range: .init(location: 0, length: 3)))]
}
service.processTextMessage("@al test @bo", selectedRange: .init(location: 0, length: 1))
try await deffered.fulfill()

deffered = deferFulfillment(service.suggestionsPublisher) { suggestion in
suggestion == [.user(item: .init(id: bob.userID, displayName: bob.displayName, avatarURL: bob.avatarURL, range: .init(location: 9, length: 3)))]
}
service.processTextMessage("@al test @bo", selectedRange: .init(location: 9, length: 1))
try await deffered.fulfill()

deffered = deferFulfillment(service.suggestionsPublisher) { suggestion in
suggestion == []
}
service.processTextMessage("@al test @bo", selectedRange: .init(location: 4, length: 1))
try await deffered.fulfill()
}
}

private extension MentionSuggestionItem {
Expand Down