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 @@ -8,8 +8,8 @@
import Combine
import Foundation

private enum SuggestionTriggerPattern: Character {
case at = "@"
private enum SuggestionTriggerRegex: String {
case at = "@\\w+"
}

final class CompletionSuggestionService: CompletionSuggestionServiceProtocol {
Expand All @@ -18,6 +18,7 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol {
private(set) var suggestionsPublisher: AnyPublisher<[SuggestionItem], Never> = Empty().eraseToAnyPublisher()

private let suggestionTriggerSubject = CurrentValueSubject<SuggestionTrigger?, Never>(nil)
var rawSuggestionText = ""

init(roomProxy: JoinedRoomProxyProtocol) {
self.roomProxy = roomProxy
Expand Down Expand Up @@ -73,8 +74,8 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol {
}
}

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

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

// MARK: - Private

private func detectTriggerInText(_ text: String?) -> SuggestionTrigger? {
guard let text else {
private func detectTriggerInText(_ text: String, selectedRange: NSRange) -> SuggestionTrigger? {
guard let regex = try? NSRegularExpression(pattern: SuggestionTriggerRegex.at.rawValue, options: []) else {
return nil
}

let components = text.components(separatedBy: .whitespaces)
let matches = regex.matches(in: text)
let match = matches
.filter { matchResult in
selectedRange.location >= matchResult.range.lowerBound
&& selectedRange.location <= matchResult.range.upperBound
&& selectedRange.length <= matchResult.range.upperBound - matchResult.range.lowerBound
}
.first

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 {
guard let match,
let range = Range(match.range, in: text) else {
return nil
}

return .init(type: .user, text: lastComponent, range: NSRange(range, in: text))
var suggestionText = String(text[range])
suggestionText.removeFirst()
rawSuggestionText = suggestionText

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 @@ -50,8 +50,9 @@ struct MentionSuggestionItem: Identifiable, Equatable {
// sourcery: AutoMockable
protocol CompletionSuggestionServiceProtocol {
var suggestionsPublisher: AnyPublisher<[SuggestionItem], Never> { get }
var rawSuggestionText: String { 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 Expand Up @@ -483,6 +485,9 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
let attributedString = NSMutableAttributedString(attributedString: state.bindings.plainComposerText)
mentionBuilder.handleUserMention(for: attributedString, in: suggestion.range, url: url, userID: item.id, userDisplayName: item.displayName)
state.bindings.plainComposerText = attributedString

let newSelectedRange = NSRange(location: state.bindings.selectedRange.location - completionSuggestionService.rawSuggestionText.count, length: 0)
state.bindings.selectedRange = newSelectedRange
Comment on lines +489 to +490
Copy link
Author

@vickcoo vickcoo Feb 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pixlwave 👋🏻, I want to discuss this part which calculates the correct cursor location, but I don't know the data flow is right or not(the place of the rawSuggestionText property).

For example, the cursor at location 2, then the text converts to pill after selected suggestion, it seems to occupy one character that causes me must to do this calculation

  ↓
@vic test

// This is a pill.
        ↓
(@vick) test

}
case .allUsers:
if context.composerFormattingEnabled {
Expand All @@ -491,6 +496,9 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
let attributedString = NSMutableAttributedString(attributedString: state.bindings.plainComposerText)
mentionBuilder.handleAllUsersMention(for: attributedString, in: suggestion.range)
state.bindings.plainComposerText = attributedString

let newSelectedRange = NSRange(location: state.bindings.selectedRange.location - completionSuggestionService.rawSuggestionText.count, length: 0)
state.bindings.selectedRange = newSelectedRange
}
}
}
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 @@ -135,30 +138,36 @@ private struct UITextViewWrapper: UIViewRepresentable {
// moves the caret back to the bottom of the composer.
// https://github.com/element-hq/element-x-ios/issues/3104
textView.selectedTextRange = selection
} else {
textView.selectedRange = selectedRange
}
}
}

func makeCoordinator() -> Coordinator {
Coordinator(text: $text,
selectedRange: $selectedRange,
maxHeight: maxHeight,
keyHandler: keyHandler,
pasteHandler: pasteHandler)
}

final class Coordinator: NSObject, UITextViewDelegate, ElementTextViewDelegate {
private var text: Binding<NSAttributedString>
private var selectedRange: Binding<NSRange>

private let maxHeight: CGFloat

private let keyHandler: GenericKeyHandler
private let pasteHandler: PasteHandler

init(text: Binding<NSAttributedString>,
selectedRange: Binding<NSRange>,
maxHeight: CGFloat,
keyHandler: @escaping GenericKeyHandler,
pasteHandler: @escaping PasteHandler) {
self.text = text
self.selectedRange = selectedRange
self.maxHeight = maxHeight
self.keyHandler = keyHandler
self.pasteHandler = pasteHandler
Expand All @@ -179,6 +188,14 @@ private struct UITextViewWrapper: UIViewRepresentable {
func textView(_ textView: UITextView, didReceivePasteWith provider: NSItemProvider) {
pasteHandler(provider)
}

func textViewDidChangeSelection(_ textView: UITextView) {
if selectedRange.wrappedValue != textView.selectedRange {
DispatchQueue.main.async {
self.selectedRange.wrappedValue = textView.selectedRange
}
}
}
}
}

Expand Down Expand Up @@ -331,6 +348,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