From 412f140e938efdf24c332b868fd5376b073000b6 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 8 Jul 2024 14:38:12 +0200 Subject: [PATCH 1/3] Version 2.37.5 --- CHANGELOG.md | 4 ++++ Cargo.lock | 6 +++--- bindings/wysiwyg-ffi/Cargo.toml | 2 +- bindings/wysiwyg-wasm/Cargo.toml | 2 +- bindings/wysiwyg-wasm/package-lock.json | 4 ++-- bindings/wysiwyg-wasm/package.json | 2 +- crates/wysiwyg/Cargo.toml | 2 +- platforms/android/gradle.properties | 2 +- platforms/web/package.json | 2 +- 9 files changed, 15 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c92b0342f..b10cbf0ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +# [2.37.5] - 2024-06-20 +- [Rust] Fixed a crash that happened when entering a new line after setting through html a blockquote containing a single line. +- [Common] Updated dependencies. + # [2.37.4] - 2024-06-20 - [Android] Make `CustomMentionSpan` public. - [Rust] Escape text passed to `ComposerModel::set_link_with_text` and `insert_mention*` methods. diff --git a/Cargo.lock b/Cargo.lock index dc00ccc55..b8b36fa41 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1391,7 +1391,7 @@ dependencies = [ [[package]] name = "uniffi-wysiwyg-composer" -version = "2.37.4" +version = "2.37.5" dependencies = [ "html-escape", "matrix_mentions", @@ -1852,7 +1852,7 @@ dependencies = [ [[package]] name = "wysiwyg" -version = "2.37.4" +version = "2.37.5" dependencies = [ "cfg-if", "email_address", @@ -1876,7 +1876,7 @@ dependencies = [ [[package]] name = "wysiwyg-wasm" -version = "2.37.4" +version = "2.37.5" dependencies = [ "console_error_panic_hook", "js-sys", diff --git a/bindings/wysiwyg-ffi/Cargo.toml b/bindings/wysiwyg-ffi/Cargo.toml index 158d68487..02c43aeee 100644 --- a/bindings/wysiwyg-ffi/Cargo.toml +++ b/bindings/wysiwyg-ffi/Cargo.toml @@ -7,7 +7,7 @@ description = "Swift and Kotlin bindings for wysiwyg-rust" keywords = ["matrix", "chat", "messaging", "composer", "wysiwyg"] license = "Apache-2.0" name = "uniffi-wysiwyg-composer" -version = "2.37.4" +version = "2.37.5" rust-version = { workspace = true } [features] diff --git a/bindings/wysiwyg-wasm/Cargo.toml b/bindings/wysiwyg-wasm/Cargo.toml index 2d1fcbe7a..2f1970a3c 100644 --- a/bindings/wysiwyg-wasm/Cargo.toml +++ b/bindings/wysiwyg-wasm/Cargo.toml @@ -7,7 +7,7 @@ description = "WASM bindings for wysiwyg-rust" keywords = ["matrix", "chat", "messaging", "composer", "wysiwyg"] license = "Apache-2.0" name = "wysiwyg-wasm" -version = "2.37.4" +version = "2.37.5" rust-version = { workspace = true } [package.metadata.wasm-pack.profile.profiling] diff --git a/bindings/wysiwyg-wasm/package-lock.json b/bindings/wysiwyg-wasm/package-lock.json index fb0c0b14e..d82cad6de 100644 --- a/bindings/wysiwyg-wasm/package-lock.json +++ b/bindings/wysiwyg-wasm/package-lock.json @@ -1,12 +1,12 @@ { "name": "wysiwyg-wasm", - "version": "2.37.4", + "version": "2.37.5", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "wysiwyg-wasm", - "version": "2.37.4", + "version": "2.37.5", "license": "Apache-2.0", "devDependencies": { "jest": "^28.1.0", diff --git a/bindings/wysiwyg-wasm/package.json b/bindings/wysiwyg-wasm/package.json index 8792b443d..0ed9dc597 100644 --- a/bindings/wysiwyg-wasm/package.json +++ b/bindings/wysiwyg-wasm/package.json @@ -1,6 +1,6 @@ { "name": "wysiwyg-wasm", - "version": "2.37.4", + "version": "2.37.5", "homepage": "https://gitlab.com/andybalaam/wysiwyg-rust", "description": "WASM bindings for wysiwyg-rust", "license": "Apache-2.0", diff --git a/crates/wysiwyg/Cargo.toml b/crates/wysiwyg/Cargo.toml index c9a69f6fb..91bb06996 100644 --- a/crates/wysiwyg/Cargo.toml +++ b/crates/wysiwyg/Cargo.toml @@ -7,7 +7,7 @@ description = "Model code to power a rich text editor for Matrix" keywords = ["matrix", "chat", "messaging", "composer", "wysiwyg"] license = "Apache-2.0" name = "wysiwyg" -version = "2.37.4" +version = "2.37.5" rust-version = { workspace = true } [features] diff --git a/platforms/android/gradle.properties b/platforms/android/gradle.properties index e370899aa..115ee6707 100644 --- a/platforms/android/gradle.properties +++ b/platforms/android/gradle.properties @@ -27,7 +27,7 @@ RELEASE_SIGNING_ENABLED=true GROUP=io.element.android # POM_ARTIFACT_ID is configured in each module's gradle.properties -VERSION_NAME=2.37.4 +VERSION_NAME=2.37.5 POM_NAME=Matrix WYSIWYG POM_DESCRIPTION=Cross-platform rich text editor that generates HTML output. diff --git a/platforms/web/package.json b/platforms/web/package.json index 03e70d0e8..6a3ab4523 100644 --- a/platforms/web/package.json +++ b/platforms/web/package.json @@ -1,6 +1,6 @@ { "name": "@matrix-org/matrix-wysiwyg", - "version": "2.37.4", + "version": "2.37.5", "type": "module", "description": "Wysiwyg composer for matrix.org using React", "author": "matrix.org", From cdb273c7b65ccaa04059f39a3c22e5513d90a8ff Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 10 Jul 2024 08:56:11 +0100 Subject: [PATCH 2/3] Update dependency pointfreeco/swift-snapshot-testing to from: "1.17.1" (#1011) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- platforms/ios/lib/WysiwygComposer/Package.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platforms/ios/lib/WysiwygComposer/Package.swift b/platforms/ios/lib/WysiwygComposer/Package.swift index dbae82fea..04588bd69 100644 --- a/platforms/ios/lib/WysiwygComposer/Package.swift +++ b/platforms/ios/lib/WysiwygComposer/Package.swift @@ -19,7 +19,7 @@ let package = Package( dependencies: [ .package( url: "https://github.com/pointfreeco/swift-snapshot-testing", - from: "1.16.2" + from: "1.17.1" ), .package( url: "https://github.com/Cocoanetics/DTCoreText", From 86ff78b28eabfbf91e42e48b3816b8cf5028a3d7 Mon Sep 17 00:00:00 2001 From: David Langley Date: Mon, 15 Jul 2024 14:26:58 +0100 Subject: [PATCH 3/3] Fix inline predictive text and keyboard suggestion toolbar use cases. (#890) Co-authored-by: Mauro Romito Co-authored-by: Mauro <34335419+Velin92@users.noreply.github.com> --- .github/workflows/ios.yml | 4 +- .../example/Wysiwyg.xcodeproj/project.pbxproj | 23 ++ .../WysiwygUITests+Keyboard.swift | 254 ++++++++++++++++++ .../WysiwygUITests/WysiwygUITests.swift | 5 + .../Extensions/NSAttributedString+Range.swift | 4 +- .../WysiwygComposerView.swift | 9 +- .../WysiwygComposerViewModel.swift | 195 ++++++++++++-- .../WysiwygComposerViewModelProtocol.swift | 1 + .../WysiwygComposerView/WysiwygTextView.swift | 8 +- .../WysiwygComposer/Extensions/String.swift | 9 + .../WysiwygComposer/Tools/StringDiffer.swift | 5 - .../WysiwygComposerViewModelTests.swift | 8 +- .../String+LatinLangugesTests.swift | 37 +++ 13 files changed, 519 insertions(+), 43 deletions(-) create mode 100644 platforms/ios/example/WysiwygUITests/WysiwygUITests+Keyboard.swift create mode 100644 platforms/ios/lib/WysiwygComposer/Tests/WysiwygComposerTests/Extensions/String+LatinLangugesTests.swift diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index f3904b65f..6daa30296 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -23,7 +23,7 @@ jobs: steps: - uses: actions/checkout@v4 - + - name: Install xcresultparser run: brew install a7ex/homebrew-formulae/xcresultparser @@ -69,7 +69,7 @@ jobs: flags: unittests-ios, unittests # https://github.com/codecov/codecov-action/issues/557#issuecomment-1216749652 token: ${{ secrets.CODECOV_TOKEN }} - + - name: UI test coverage working-directory: platforms/ios/example run: exec ./ios-ui-test-coverage.sh diff --git a/platforms/ios/example/Wysiwyg.xcodeproj/project.pbxproj b/platforms/ios/example/Wysiwyg.xcodeproj/project.pbxproj index 78da00584..f546764fa 100644 --- a/platforms/ios/example/Wysiwyg.xcodeproj/project.pbxproj +++ b/platforms/ios/example/Wysiwyg.xcodeproj/project.pbxproj @@ -45,6 +45,7 @@ A6F4D0CF29AE0C1500087A3E /* Users.swift in Sources */ = {isa = PBXBuildFile; fileRef = A6F4D0CE29AE0C1500087A3E /* Users.swift */; }; A6F4D0D129AE0C3200087A3E /* Rooms.swift in Sources */ = {isa = PBXBuildFile; fileRef = A6F4D0D029AE0C3200087A3E /* Rooms.swift */; }; A6F4D0D329AE0C5100087A3E /* Commands.swift in Sources */ = {isa = PBXBuildFile; fileRef = A6F4D0D229AE0C5100087A3E /* Commands.swift */; }; + A75C6AD22C3E989D0096D3A4 /* WysiwygUITests+Keyboard.swift in Sources */ = {isa = PBXBuildFile; fileRef = A75C6AD12C3E989D0096D3A4 /* WysiwygUITests+Keyboard.swift */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -97,6 +98,7 @@ A6F4D0CE29AE0C1500087A3E /* Users.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Users.swift; sourceTree = ""; }; A6F4D0D029AE0C3200087A3E /* Rooms.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Rooms.swift; sourceTree = ""; }; A6F4D0D229AE0C5100087A3E /* Commands.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Commands.swift; sourceTree = ""; }; + A75C6AD12C3E989D0096D3A4 /* WysiwygUITests+Keyboard.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WysiwygUITests+Keyboard.swift"; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -178,6 +180,7 @@ A64AB145296C759A00F08494 /* WysiwygUITests+Quotes.swift */, A661FDA629B0ACB400E799A6 /* WysiwygUITests+Suggestions.swift */, A64AB13D296C732500F08494 /* WysiwygUITests+Typing.swift */, + A75C6AD12C3E989D0096D3A4 /* WysiwygUITests+Keyboard.swift */, ); path = WysiwygUITests; sourceTree = ""; @@ -280,6 +283,7 @@ isa = PBXNativeTarget; buildConfigurationList = A6472CD12886CF840021A0E8 /* Build configuration list for PBXNativeTarget "WysiwygUITests" */; buildPhases = ( + A75C6AD32C3ECA450096D3A4 /* Force software keyboard on simulator */, A6472CBD2886CF840021A0E8 /* Sources */, A6472CBE2886CF840021A0E8 /* Frameworks */, A6472CBF2886CF840021A0E8 /* Resources */, @@ -392,6 +396,24 @@ shellPath = /bin/sh; shellScript = "export PATH=\"$PATH:/opt/homebrew/bin\"\nif which swiftformat >/dev/null; then\n swiftformat $PROJECT_DIR\n swiftformat ../lib/WysiwygComposer\nelse\n echo \"warning: SwiftFormat not installed, download from https://github.com/nicklockwood/SwiftFormat\"\nfi\n"; }; + A75C6AD32C3ECA450096D3A4 /* Force software keyboard on simulator */ = { + isa = PBXShellScriptBuildPhase; + buildActionMask = 2147483647; + files = ( + ); + inputFileListPaths = ( + ); + inputPaths = ( + ); + name = "Force software keyboard on simulator"; + outputFileListPaths = ( + ); + outputPaths = ( + ); + runOnlyForDeploymentPostprocessing = 0; + shellPath = /bin/sh; + shellScript = "external_kb_connected=false\n\nosascript -e 'quit app \"Simulator\"'\n\nSIMUS_KEYBOARD=$(/usr/libexec/PlistBuddy -c \"Print :DevicePreferences\" ~/Library/Preferences/com.apple.iphonesimulator.plist | perl -lne 'print $1 if /^ (\\S*) =/')\n\necho \"$SIMUS_KEYBOARD\" | while read -r a; do /usr/libexec/PlistBuddy -c \"Set :DevicePreferences:$a:ConnectHardwareKeyboard $external_kb_connected\" ~/Library/Preferences/com.apple.iphonesimulator.plist || /usr/libexec/PlistBuddy -c \"Add :DevicePreferences:$a:ConnectHardwareKeyboard bool $external_kb_connected\" ~/Library/Preferences/com.apple.iphonesimulator.plist; done\n"; + }; /* End PBXShellScriptBuildPhase section */ /* Begin PBXSourcesBuildPhase section */ @@ -432,6 +454,7 @@ A6BB18D729F9191C00EB6366 /* WysiwygUITests+Autocorrection.swift in Sources */, A6472CC62886CF840021A0E8 /* WysiwygUITests.swift in Sources */, A64AB140296C73CE00F08494 /* WysiwygUITests+Links.swift in Sources */, + A75C6AD22C3E989D0096D3A4 /* WysiwygUITests+Keyboard.swift in Sources */, A661FDA729B0ACB400E799A6 /* WysiwygUITests+Suggestions.swift in Sources */, A68E7141291D40710023CC04 /* WysiwygSharedConstants.swift in Sources */, A64AB144296C747C00F08494 /* WysiwygUITests+Format.swift in Sources */, diff --git a/platforms/ios/example/WysiwygUITests/WysiwygUITests+Keyboard.swift b/platforms/ios/example/WysiwygUITests/WysiwygUITests+Keyboard.swift new file mode 100644 index 000000000..e0089355d --- /dev/null +++ b/platforms/ios/example/WysiwygUITests/WysiwygUITests+Keyboard.swift @@ -0,0 +1,254 @@ +// +// Copyright 2024 The Matrix.org Foundation C.I.C +// +// 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 XCTest + +// These tests work on the assunmption that we always have the software keyboard enabled which is handled through a build phase run script. +// The following tests may also require specific keyboard languages that will be automatically added if needed. +extension WysiwygUITests { + func testInlinePredictiveText() { + sleep(1) + setupKeyboard(.englishQWERTY) + + // Sometimes autocorrection can break capitalisation, so we need to make sure the first letter is lowercase + app.keyboards.buttons["shift"].tap() + app.typeTextCharByCharUsingKeyboard("hello how a") + // We assert both the tree and textview content because the text view is containing the predictive text at that moment + // Which in the ui test is seen as part of the static text + assertTextViewContent("hello how are you") + assertTreeEquals( + """ + └>"hello how a" + """ + ) + app.keys["space"].tap() + assertTextViewContent("hello how are you ") + assertTreeEquals( + """ + └>"hello how are you " + """ + ) + } + + func testInlinePredictiveTextIsIgnoredWhenSending() { + sleep(1) + setupKeyboard(.englishQWERTY) + + // Sometimes autocorrection can break capitalisation, so we need to make sure the first letter is lowercase + app.keyboards.buttons["shift"].tap() + app.typeTextCharByCharUsingKeyboard("hello how") + // We assert both the tree and textview content because the text view is containing the predictive text at that moment + // Which in the ui test is seen as part of the static text + assertTextViewContent("hello how are you") + assertTreeEquals( + """ + └>"hello how" + """ + ) + button(.sendButton).tap() + sleep(1) + assertContentText(plainText: "hello how", htmlText: "hello how") + } + + func testInlinePredictiveTextIsIgnoredWhenDeleting() { + sleep(1) + setupKeyboard(.englishQWERTY) + + // Sometimes autocorrection can break capitalisation, so we need to make sure the first letter is lowercase + app.keyboards.buttons["shift"].tap() + app.typeTextCharByCharUsingKeyboard("hello how") + app.keys["delete"].tap() + // We assert both the tree and textview content because the text view is containing the predictive text at that moment + // Which in the ui test is seen as part of the static text + assertTextViewContent("hello how are you") + assertTreeEquals( + """ + └>"hello ho" + """ + ) + button(.sendButton).tap() + sleep(1) + assertContentText(plainText: "hello ho", htmlText: "hello ho") + } + + func testDoubleSpaceIntoDot() { + sleep(1) + setupKeyboard(.englishQWERTY) + + // Sometimes autocorrection can break capitalisation, so we need to make sure the first letter is lowercase + app.keyboards.buttons["shift"].tap() + app.typeTextCharByCharUsingKeyboard("hello") + app.keys["space"].tap() + app.keys["space"].tap() + assertTextViewContent("hello. ") + assertTreeEquals( + """ + └>"hello. " + """ + ) + } + + func testDotAfterInlinePredictiveText() { + sleep(1) + setupKeyboard(.englishQWERTY) + + // Sometimes autocorrection can break capitalisation, so we need to make sure the first letter is lowercase + app.keyboards.buttons["shift"].tap() + app.typeTextCharByCharUsingKeyboard("hello how a") + // We assert both the tree and textview content because the text view is containing the predictive text at that moment + // Which in the ui test is seen as part of the static text + assertTextViewContent("hello how are you") + app.keys["space"].tap() + app.keys["more"].tap() + app.keys["."].tap() + + // This optimisation to predictive inline text was introduced in 17.5 + let correctText: String + if #available(iOS 17.5, *) { + correctText = "hello how are you." + } else { + correctText = "hello how are you ." + } + assertTextViewContent(correctText) + // In the failure case a second dot is added in the tree. + assertTreeEquals( + """ + └>"\(correctText)" + """ + ) + } + + func testJapaneseKanaDeletion() { + sleep(1) + setupKeyboard(.japaneseKana) + + app.typeTextCharByCharUsingKeyboard("は") + assertTextViewContent("は") + assertTreeEquals( + """ + └>"は" + """ + ) + app.keys["delete"].tap() + assertTextViewContent("") + XCTAssertEqual(staticText(.treeText).label, "\n") + } + + private func setupKeyboard(_ keyboard: TestKeyboard) { + var changeKeyboardButton: XCUIElement! + // If only 1 language + emoji keyboards are present the emoji button is used to change language + // otherwise the button next keyboard button will be present instead + let nextKeyboard = app.buttons["Next keyboard"] + let emoji = app.buttons["Emoji"] + if nextKeyboard.exists { + changeKeyboardButton = nextKeyboard + } else if emoji.exists { + changeKeyboardButton = emoji + } + + if changeKeyboardButton == nil { + addKeyboardToSettings(keyboard: keyboard) + return + } + + changeKeyboardButton.press(forDuration: 1) + let keyboardSelection = app.tables.staticTexts[keyboard.label] + if !keyboardSelection.exists { + addKeyboardToSettings(keyboard: keyboard) + return + } + keyboardSelection.tap() + } + + private func addKeyboardToSettings(keyboard: TestKeyboard) { + let settingsApp = XCUIApplication(bundleIdentifier: "com.apple.Preferences") + settingsApp.launch() + + settingsApp.tables.cells.staticTexts["General"].tap() + settingsApp.tables.cells.staticTexts["Keyboard"].tap() + settingsApp.tables.cells.staticTexts["Keyboards"].tap() + if settingsApp.tables.cells.staticTexts[keyboard.keyboardIdentifier].exists { + return + } + settingsApp.tables.cells.staticTexts["AddNewKeyboard"].tap() + settingsApp.tables.cells.staticTexts[keyboard.localeIdentifier].tap() + if keyboard.hasSubSelection { + settingsApp.tables.cells.staticTexts[keyboard.keyboardIdentifier].tap() + } + settingsApp.buttons["Done"].tap() + sleep(1) + settingsApp.terminate() + + app.launch() + textView.tap() + sleep(1) + + setupKeyboard(keyboard) + } +} + +private extension XCUIApplication { + func typeTextCharByCharUsingKeyboard(_ text: String) { + for char in text { + if char == " " { + keys["space"].tap() + continue + } + keys[String(char)].tap() + } + } +} + +private enum TestKeyboard { + case englishQWERTY + case japaneseKana + + var keyboardIdentifier: String { + switch self { + case .englishQWERTY: + return "en_US@sw=QWERTY;hw=Automatic" + case .japaneseKana: + return "ja_JP-Kana@sw=Kana;hw=Automatic" + } + } + + var localeIdentifier: String { + switch self { + case .englishQWERTY: + return "en_US" + case .japaneseKana: + return "ja_JP" + } + } + + var label: String { + switch self { + case .englishQWERTY: + return "English (US)" + case .japaneseKana: + return "日本語かな" + } + } + + var hasSubSelection: Bool { + switch self { + case .englishQWERTY: + return false + case .japaneseKana: + return true + } + } +} diff --git a/platforms/ios/example/WysiwygUITests/WysiwygUITests.swift b/platforms/ios/example/WysiwygUITests/WysiwygUITests.swift index be59bb118..a404d51ef 100644 --- a/platforms/ios/example/WysiwygUITests/WysiwygUITests.swift +++ b/platforms/ios/example/WysiwygUITests/WysiwygUITests.swift @@ -179,6 +179,11 @@ extension WysiwygUITests { XCTAssertTrue(pill.exists) XCTAssertEqual(pill.label, displayName) } + + func assertContentText(plainText: String, htmlText: String) { + XCTAssert(staticText(.contentText).label == plainText) + XCTAssert(staticText(.htmlContentText).label == htmlText) + } } extension XCUIElement { diff --git a/platforms/ios/lib/WysiwygComposer/Sources/HTMLParser/Extensions/NSAttributedString+Range.swift b/platforms/ios/lib/WysiwygComposer/Sources/HTMLParser/Extensions/NSAttributedString+Range.swift index 0d8451123..d8b2d817c 100644 --- a/platforms/ios/lib/WysiwygComposer/Sources/HTMLParser/Extensions/NSAttributedString+Range.swift +++ b/platforms/ios/lib/WysiwygComposer/Sources/HTMLParser/Extensions/NSAttributedString+Range.swift @@ -194,8 +194,8 @@ extension NSMutableAttributedString { /// - Returns: self (discardable) @discardableResult func removeDiscardableContent() -> Self { - discardableTextRanges().reversed().forEach { - replaceCharacters(in: $0, with: "") + for discardableTextRange in discardableTextRanges().reversed() { + replaceCharacters(in: discardableTextRange, with: "") } return self diff --git a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerView.swift b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerView.swift index 03735dfd2..36ed7e677 100644 --- a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerView.swift +++ b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerView.swift @@ -85,7 +85,9 @@ public struct WysiwygComposerView: View { @ViewBuilder private var placeholderView: some View { - if viewModel.isContentEmpty, !viewModel.textView.isDictationRunning { + // The content can be empty but the textview not, e.g. if you start dictation + // but have not committed the text yet. + if viewModel.textView.attributedText.length == 0 { Text(placeholder) .font(Font(UIFont.preferredFont(forTextStyle: .body))) .foregroundColor(placeholderColor) @@ -191,7 +193,10 @@ struct UITextViewWrapper: UIViewRepresentable { textView.logText, "Replacement: \"\(text)\""], functionName: #function) - return replaceText(range, text) + let change = replaceText(range, text) + Logger.textView.logDebug(["change: \(change)"], + functionName: #function) + return change } func textViewDidChange(_ textView: UITextView) { diff --git a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerViewModel.swift b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerViewModel.swift index fd758c983..16398136f 100644 --- a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerViewModel.swift +++ b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerViewModel.swift @@ -31,13 +31,13 @@ public class WysiwygComposerViewModel: WysiwygComposerViewModelProtocol, Observa let textView = WysiwygTextView() textView.linkTextAttributes[.foregroundColor] = parserStyle.linkColor textView.mentionDisplayHelper = mentionDisplayHelper - textView.apply(attributedContent) + textView.apply(attributedContent, committed: &committedAttributedText) return textView }() { didSet { textView.linkTextAttributes[.foregroundColor] = parserStyle.linkColor textView.mentionDisplayHelper = mentionDisplayHelper - textView.apply(attributedContent) + textView.apply(attributedContent, committed: &committedAttributedText) } } @@ -135,7 +135,18 @@ public class WysiwygComposerViewModel: WysiwygComposerViewModelProtocol, Observa } private(set) var hasPendingFormats = false - + + /// This is used to track the text commited to the editor by the user, as opposed to text + /// that could be in the editor that is not yet committed (e.g. from inline predictive text or dictation ). + private lazy var committedAttributedText = NSAttributedString(string: "", attributes: defaultTextAttributes) + + private var lastReplaceTextUpdate: ReplaceTextUpdate? + + /// Wether the view contains uncommitted text(e.g. a predictive suggestion is shown in grey). + private var hasUncommitedText: Bool { + textView.attributedText.htmlChars.withNBSP != committedAttributedText.htmlChars.withNBSP + } + // MARK: - Public public init(minHeight: CGFloat = 22, @@ -300,6 +311,7 @@ public extension WysiwygComposerViewModel { compressedHeight = min(maxCompressedHeight, max(minHeight, idealTextHeight)) } + // swiftlint:disable:next cyclomatic_complexity func replaceText(range: NSRange, replacementText: String) -> Bool { guard shouldReplaceText else { return false @@ -309,6 +321,15 @@ public extension WysiwygComposerViewModel { return true } + let nextTextUpdate = ReplaceTextUpdate(date: Date.now, range: range, text: replacementText) + // This is to specifically to work around an issue when tapping on an inline predictive text suggestion within the text view. + // Even though we have the delegate disabled during modifications to the textview we still get some duplicate + // calls to this method in this case specifically. It's very unlikely we would get a valid subsequent call + // with the same range and replacement text within such a short period of time, so should be safe. + if let lastReplaceTextUpdate, lastReplaceTextUpdate.shouldDebounce(with: nextTextUpdate) { + return true + } + // This fixes a bug where the attributed string keeps link and inline code formatting // when they are the last formatting to be deleted if textView.attributedText.length == 0 { @@ -316,21 +337,41 @@ public extension WysiwygComposerViewModel { } let update: ComposerUpdate - let shouldAcceptChange: Bool + let skipTextViewUpdate: Bool if range != attributedContent.selection { select(range: range) } - - if attributedContent.selection.length == 0, replacementText == "" { + + // The system handles certain auto-compelete use-cases with somewhat unusual replacementText/range + // combinations, some of those edge cases are handled below. + + // Are we replacing some selected text by tapping the suggestion toolbar + // When this happens a range/replacementText of this combination is sent. + let isReplacingWordWithSuggestion = replacementText == "" && !hasUncommitedText && range.length == 0 + + // A no-op rte side is required here + if isReplacingWordWithSuggestion { + return true + } + + // Are we backspacing from an inline predictive text suggestion. + // When this happens a range/replacementText of this combination is sent. + let isExitingPredictiveText = replacementText == "" + && hasUncommitedText + && range == attributedContent.selection && range.length == 0 + + let isNormalBackspace = attributedContent.selection.length == 0 && replacementText == "" + + if isNormalBackspace || isExitingPredictiveText { update = model.backspace() - shouldAcceptChange = false + skipTextViewUpdate = false } else if replacementText.count == 1, replacementText[String.Index(utf16Offset: 0, in: replacementText)].isNewline { update = createEnterUpdate() - shouldAcceptChange = false + skipTextViewUpdate = false } else { update = model.replaceText(newText: replacementText) - shouldAcceptChange = true + skipTextViewUpdate = true } // Reconciliates the model with the text any time the link state changes @@ -340,18 +381,18 @@ public extension WysiwygComposerViewModel { case let .update(newState): if newState[.link] != actionStates[.link] { applyUpdate(update, skipTextViewUpdate: true) - textView.apply(attributedContent) + applyAtributedContent() updateCompressedHeightIfNeeded() return false } default: break } - applyUpdate(update, skipTextViewUpdate: shouldAcceptChange) - - return shouldAcceptChange + applyUpdate(update, skipTextViewUpdate: skipTextViewUpdate) + lastReplaceTextUpdate = nextTextUpdate + return skipTextViewUpdate } - + func select(range: NSRange) { do { guard let text = textView.attributedText, !plainTextMode else { return } @@ -378,13 +419,91 @@ public extension WysiwygComposerViewModel { } plainTextContent = textView.attributedText } else { - reconciliateIfNeeded() + let updated = updateForDoubleSpaceToDotConversionIfNeeded() || updateDotAfterInlineTextPredicationIfNeeded() + if !updated { + reconciliateIfNeeded() + } applyPendingFormatsIfNeeded() } updateCompressedHeightIfNeeded() } + /// Checks if the editor automatically replaced two spaces with a dot and reconcile with the model if it did. + /// - Returns: Whether the an update was applied. + func updateForDoubleSpaceToDotConversionIfNeeded() -> Bool { + let text = textView.attributedText.htmlChars.withNBSP + let textSelection = textView.selectedRange + // Check if the last character in the editor is a dot. + guard text.count > 1, + textSelection.location > 1, + text.prefix(textSelection.location).hasSuffix(".") + else { + return false + } + // Check if the last 2 characters before the cursor in the model are whitespace. i.e. " |" + let content = attributedContent.text.htmlChars.withNBSP + let contentSelection = attributedContent.selection + guard content.count > 1, + contentSelection.location > 1, + String(content.prefix(contentSelection.location) + .suffix(2)) + .trimmingCharacters(in: .whitespaces) + .isEmpty + else { + return false + } + // replace the 2 whitespace characters just before and cursor with ". " + let replaceUpdate = model.replaceTextIn(newText: ". ", + start: UInt32(contentSelection.location - 2), + end: UInt32(contentSelection.location)) + applyUpdate(replaceUpdate, skipTextViewUpdate: true) + return true + } + + /// Checks if the editor automatically moved the cursor back a space when pressing a dot after + /// accepting an inline predictive text suggestion. It then reconciles with the model if it did. + /// - Returns: Whether the an update was applied. + func updateDotAfterInlineTextPredicationIfNeeded() -> Bool { + // This optimisation to predictive inline text only came in in 17.5 + guard #available(iOS 17.5, *) else { + return false + } + let text = textView.attributedText.htmlChars.withNBSP + let textSelection = textView.selectedRange + // Check if the last character in the editor is a dot. + guard text.count > 1, + textSelection.location > 1, + text.prefix(textSelection.location).hasSuffix(".") + else { + return false + } + let content = attributedContent.text.htmlChars.withNBSP + let contentSelection = attributedContent.selection + guard content.count > 1, + contentSelection.location > 1, + contentSelection.location + 1 <= content.count + else { + return false + } + let lastTwo = content.prefix(contentSelection.location + 1).suffix(2) + // Check if the characters just before and after the cursor are a dot and whitespace ie " |." + guard lastTwo.prefix(1) + .trimmingCharacters(in: .whitespaces) + .isEmpty, + lastTwo.suffix(1) == "." + else { + return false + } + + // replace characters just before and after the cursor with a dot + let replaceUpdate = model.replaceTextIn(newText: ".", + start: UInt32(contentSelection.location - 1), + end: UInt32(contentSelection.location + 1)) + applyUpdate(replaceUpdate, skipTextViewUpdate: true) + return true + } + func applyLinkOperation(_ linkOperation: WysiwygLinkOperation) { let update: ComposerUpdate switch linkOperation { @@ -422,6 +541,10 @@ public extension WysiwygComposerViewModel { return CGSize(width: width, height: maximised ? maxExpandedHeight : min(maxCompressedHeight, max(minHeight, idealHeight))) } + + func applyAtributedContent() { + textView.apply(attributedContent, committed: &committedAttributedText) + } } // MARK: - Private @@ -444,8 +567,12 @@ private extension WysiwygComposerViewModel { applyReplaceAll(codeUnits: codeUnits, start: start, end: end) // Note: this makes replaceAll act like .keep on cases where we expect the text // view to be properly updated by the system. - if !skipTextViewUpdate { - textView.apply(attributedContent) + if skipTextViewUpdate { + // We skip updating the text view as the system did that for us but that + // is not reflected in committedAttributedText yet, so update it. + committedAttributedText = attributedContent.text + } else { + applyAtributedContent() updateCompressedHeightIfNeeded() } case let .select(startUtf16Codeunit: start, @@ -560,7 +687,7 @@ private extension WysiwygComposerViewModel { plainTextContent = NSAttributedString() } } - + /// Reconciliate the content of the model with the content of the text view. func reconciliateIfNeeded() { do { @@ -569,6 +696,13 @@ private extension WysiwygComposerViewModel { to: textView.attributedText.htmlChars) else { return } + + // Don't use reconciliate if the replacement is only latin character languages + // as it shouldn't be needed. It is needed for CJK lanuages like Japanese Kana. + if replacement.text.containsLatinAndCommonCharactersOnly { + return + } + // Reconciliate Logger.viewModel.logDebug(["Reconciliate from \"\(attributedContent.text.string)\" to \"\(textView.text ?? "")\""], functionName: #function) @@ -588,9 +722,9 @@ private extension WysiwygComposerViewModel { case StringDifferError.tooComplicated, StringDifferError.insertionsDontMatchRemovals: // Restore from the model, as otherwise the composer will enter a broken state - textView.apply(attributedContent) + applyAtributedContent() updateCompressedHeightIfNeeded() - Logger.viewModel.logError(["Reconciliate failed, content has been restored from the model"], + Logger.viewModel.logError(["Reconciliate failed(\(error)), content has been restored from the model"], functionName: #function) case AttributedRangeError.outOfBoundsAttributedIndex, AttributedRangeError.outOfBoundsHtmlIndex: @@ -607,8 +741,7 @@ private extension WysiwygComposerViewModel { /// to apply (e.g. we hit the bold button with no selection). func applyPendingFormatsIfNeeded() { guard hasPendingFormats else { return } - - textView.apply(attributedContent) + applyAtributedContent() updateCompressedHeightIfNeeded() hasPendingFormats = false } @@ -618,7 +751,8 @@ private extension WysiwygComposerViewModel { /// - Returns: A markdown string. func computeMarkdownContent() -> String { let markdownContent: String - if let mentionReplacer, let attributedText = textView.attributedText { + if let mentionReplacer, + let attributedText = textView.attributedText { // `MentionReplacer` should restore altered content to valid markdown. markdownContent = mentionReplacer.restoreMarkdown(in: attributedText) } else { @@ -656,3 +790,18 @@ extension WysiwygComposerViewModel: ComposerModelWrapperDelegate { private extension Logger { static let viewModel = Logger(subsystem: subsystem, category: "ViewModel") } + +private struct ReplaceTextUpdate { + static let debounceThreshold = 0.1 + var date: Date + var range: NSRange + var text: String +} + +private extension ReplaceTextUpdate { + func shouldDebounce(with other: ReplaceTextUpdate) -> Bool { + range == other.range + && text == other.text + && fabs(date.timeIntervalSince(other.date)) < Self.debounceThreshold + } +} diff --git a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerViewModelProtocol.swift b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerViewModelProtocol.swift index d2a177293..2f4c85c95 100644 --- a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerViewModelProtocol.swift +++ b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerViewModelProtocol.swift @@ -32,6 +32,7 @@ public protocol WysiwygComposerViewModelProtocol: AnyObject { /// - Parameters: /// - range: Range to replace. /// - replacementText: Replacement text to apply. + /// - Returns: Whether the textView should continue with the insertion of the replacement text(within shouldChangeTextIn) or it should be left for a subquent model update to reflect the changes. func replaceText(range: NSRange, replacementText: String) -> Bool /// Select given range of text within the model. diff --git a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygTextView.swift b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygTextView.swift index 4172ef481..9a15b17c6 100644 --- a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygTextView.swift +++ b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygTextView.swift @@ -41,7 +41,6 @@ public protocol MentionDisplayHelper { } public class WysiwygTextView: UITextView { private(set) var isDictationRunning = false - /// Internal delegate for the text view. weak var wysiwygDelegate: WysiwygTextViewDelegate? @@ -72,9 +71,6 @@ public class WysiwygTextView: UITextView { } private func commonInit() { - if #available(iOS 17.0, *) { - inlinePredictionType = .no - } contentMode = .redraw NotificationCenter.default.addObserver(self, selector: #selector(dictationDidStart), @@ -115,13 +111,15 @@ public class WysiwygTextView: UITextView { /// /// - Parameters: /// - content: Content to apply. - func apply(_ content: WysiwygComposerAttributedContent) { + func apply(_ content: WysiwygComposerAttributedContent, committed: inout NSAttributedString) { guard content.text.length == 0 || content.text != attributedText || content.selection != selectedRange else { return } performWithoutDelegate { + // Update committed text at the same time we update the text view. + committed = content.text self.attributedText = content.text // Set selection to {0, 0} then to expected position // avoids an issue with autocapitalization. diff --git a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Extensions/String.swift b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Extensions/String.swift index 7f3accd9d..c430b715a 100644 --- a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Extensions/String.swift +++ b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Extensions/String.swift @@ -21,4 +21,13 @@ extension String { var utf16Length: Int { (self as NSString).length } + + /// Converts all whitespaces to NBSP to avoid diffs caused by HTML translations. + var withNBSP: String { + String(map { $0.isWhitespace ? Character.nbsp : $0 }) + } + + var containsLatinAndCommonCharactersOnly: Bool { + range(of: "[^\\p{Latin}\\p{Common}]", options: .regularExpression) == nil + } } diff --git a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Tools/StringDiffer.swift b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Tools/StringDiffer.swift index 81322d56a..7ad350c85 100644 --- a/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Tools/StringDiffer.swift +++ b/platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Tools/StringDiffer.swift @@ -115,11 +115,6 @@ private struct StringDiff { } private extension String { - /// Converts all whitespaces to NBSP to avoid diffs caused by HTML translations. - var withNBSP: String { - String(map { $0.isWhitespace ? Character.nbsp : $0 }) - } - /// Computes the diff from provided string to self. Outputs UTF16 locations and lengths. /// /// - Parameter otherString: Other string to compare. diff --git a/platforms/ios/lib/WysiwygComposer/Tests/WysiwygComposerTests/Components/WysiwygComposerView/WysiwygComposerViewModelTests.swift b/platforms/ios/lib/WysiwygComposer/Tests/WysiwygComposerTests/Components/WysiwygComposerView/WysiwygComposerViewModelTests.swift index 137ce33c1..09256e4c7 100644 --- a/platforms/ios/lib/WysiwygComposer/Tests/WysiwygComposerTests/Components/WysiwygComposerView/WysiwygComposerViewModelTests.swift +++ b/platforms/ios/lib/WysiwygComposer/Tests/WysiwygComposerTests/Components/WysiwygComposerView/WysiwygComposerViewModelTests.swift @@ -206,11 +206,11 @@ final class WysiwygComposerViewModelTests: XCTestCase { func testPendingFormatFlagAfterReselectingListItem() { viewModel.apply(.bold) viewModel.apply(.italic) - mockTrailingTyping("Text") + mockTrailingTyping("Text1") viewModel.enter() viewModel.apply(.orderedList) let inListSelection = viewModel.attributedContent.selection - let insertedText = "Text" + let insertedText = "Text2" mockTyping(insertedText, at: 0) // After re-selecting the empty list item, pending format flag is still on viewModel.select(range: NSRange(location: inListSelection.location + insertedText.utf16Length, @@ -280,7 +280,7 @@ extension WysiwygComposerViewModelTests { let shouldAcceptChange = viewModel.replaceText(range: range, replacementText: text) if shouldAcceptChange { // Force apply since the text view should've updated by itself - viewModel.textView.apply(viewModel.attributedContent) + viewModel.applyAtributedContent() viewModel.didUpdateText() } } @@ -304,7 +304,7 @@ extension WysiwygComposerViewModelTests { let shouldAcceptChange = viewModel.replaceText(range: range, replacementText: "") if shouldAcceptChange { // Force apply since the text view should've updated by itself - viewModel.textView.apply(viewModel.attributedContent) + viewModel.applyAtributedContent() viewModel.didUpdateText() } } diff --git a/platforms/ios/lib/WysiwygComposer/Tests/WysiwygComposerTests/Extensions/String+LatinLangugesTests.swift b/platforms/ios/lib/WysiwygComposer/Tests/WysiwygComposerTests/Extensions/String+LatinLangugesTests.swift new file mode 100644 index 000000000..2eeb9ab33 --- /dev/null +++ b/platforms/ios/lib/WysiwygComposer/Tests/WysiwygComposerTests/Extensions/String+LatinLangugesTests.swift @@ -0,0 +1,37 @@ +// +// Copyright 2024 The Matrix.org Foundation C.I.C +// +// 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. +// + +@testable import WysiwygComposer +import XCTest + +final class StringLatinLangugesTests: XCTestCase { + func testLatinLangugeCharacters() throws { + XCTAssertTrue("hello".containsLatinAndCommonCharactersOnly) + XCTAssertTrue("helló".containsLatinAndCommonCharactersOnly) + XCTAssertTrue("helló, ".containsLatinAndCommonCharactersOnly) + XCTAssertTrue("helló, ".containsLatinAndCommonCharactersOnly) + XCTAssertTrue("😄🛴🤯❤️".containsLatinAndCommonCharactersOnly) + // Test the object replacement character as defined in String+Character extension. + XCTAssertTrue(String.object.containsLatinAndCommonCharactersOnly) + XCTAssertTrue("!@££$%^&*()".containsLatinAndCommonCharactersOnly) + + XCTAssertFalse("你好".containsLatinAndCommonCharactersOnly) + XCTAssertFalse("感^".containsLatinAndCommonCharactersOnly) + XCTAssertFalse("Меня зовут Маша".containsLatinAndCommonCharactersOnly) + XCTAssertFalse("ฉันชอบกินข้าวผัด แต่เธอชอบกินผัดไทย".containsLatinAndCommonCharactersOnly) + XCTAssertFalse("ni3好^".containsLatinAndCommonCharactersOnly) + } +}