[Mobile] Highlight color fixes (WordPress#57650)

* Fix resetting the color * Temp: Update Aztec reference * Temp: Update Aztec reference * RichText remove mark as activeFormats * TextColor use getActiveColors from the native variant * TextColor - Refactor manual logic to apply the Mark formatting and now it is handled by the native code * AztecView, don't call toggleMark when activeFormats changes * Add iOS native methods to set the mark formatting and to remove it * Update Aztec ref * Add text-color formatting in activeFormats * Handle onMarkFormatting on the Android side as well * Removes highlight text test when there's no selection since this is managed by the native Aztec library * Update Aztec ref * Update Aztec ref * Fix onMarkFormatting logic * Update Aztec version * Palette screen - Add accessibility label * E2E Helpers - Adds typeKeyString and toggleHighlightColor * Update Aztec ref * Bring back calling toggleMark when activeFormats is set for the Mark for matting when there are no selected characters. This is important to remove the format when it shouldn't be active * Update Changelog * Update Podfile * Update Podfile.lock * getFormatColors - Fix appending undefined values * Update Aztec version * Update Aztec to 1.19.11
carstingaxion · Mar 27, 2024 · 352e0ee · 352e0ee
1 parent 2db4581
commit 352e0ee
Show file tree

Hide file tree

Showing 17 changed files with 146 additions and 105 deletions.
diff --git a/packages/block-editor/src/components/rich-text/native/get-format-colors.native.js b/packages/block-editor/src/components/rich-text/native/get-format-colors.native.js
@@ -27,7 +27,7 @@ export function getFormatColors( formats, colors ) {
 						);
 						const currentStyles = currentFormat?.attributes?.style;
 						if (
-							colorObject &&
+							colorObject?.color &&
 							( ! currentStyles ||
 								currentStyles?.indexOf( colorObject.color ) ===
 									-1 )

diff --git a/packages/components/src/mobile/color-settings/palette.screen.native.js b/packages/components/src/mobile/color-settings/palette.screen.native.js
@@ -112,7 +112,11 @@ const PaletteScreen = () => {
 
 	function getClearButton() {
 		return (
-			<TouchableWithoutFeedback onPress={ onClear } hitSlop={ HIT_SLOP }>
+			<TouchableWithoutFeedback
+				accessibilityLabel={ __( 'Clear selected color' ) }
+				onPress={ onClear }
+				hitSlop={ HIT_SLOP }
+			>
 				<View style={ styles.clearButtonContainer }>
 					<Text style={ clearButtonStyle }>{ __( 'Reset' ) }</Text>
 				</View>

diff --git a/packages/format-library/src/text-color/index.native.js b/packages/format-library/src/text-color/index.native.js
@@ -25,7 +25,7 @@ import { usePreferredColorSchemeStyle } from '@wordpress/compose';
 /**
  * Internal dependencies
  */
-import { getActiveColors } from './inline.js';
+import { getActiveColors } from './inline.native.js';
 import { default as InlineColorUI } from './inline';
 import styles from './style.scss';
 

diff --git a/packages/format-library/src/text-color/inline.native.js b/packages/format-library/src/text-color/inline.native.js
@@ -36,7 +36,7 @@ function parseCSS( css = '' ) {
 	}, {} );
 }
 
-function getActiveColors( value, name, colorSettings ) {
+export function getActiveColors( value, name, colorSettings ) {
 	const activeColorFormat = getActiveFormat( value, name );
 
 	if ( ! activeColorFormat ) {
@@ -49,13 +49,14 @@ function getActiveColors( value, name, colorSettings ) {
 	};
 }
 
-function setColors( value, name, colorSettings, colors ) {
+function setColors( value, name, colorSettings, colors, contentRef ) {
 	const { color, backgroundColor } = {
 		...getActiveColors( value, name, colorSettings ),
 		...colors,
 	};
 
-	if ( ! color && ! backgroundColor ) {
+	if ( ! color ) {
+		contentRef?.onRemoveMarkFormatting();
 		return removeFormat( value, name );
 	}
 
@@ -86,62 +87,31 @@ function setColors( value, name, colorSettings, colors ) {
 
 	const format = { type: name, attributes };
 	const hasNoSelection = value.start === value.end;
-	const isAtTheEnd = value.end === value.text.length;
-	const previousCharacter = value.text.charAt( value.end - 1 );
-
-	// Force formatting due to limitations in the native implementation
-	if (
-		hasNoSelection &&
-		( value.text.length === 0 ||
-			( previousCharacter === ' ' && isAtTheEnd ) )
-	) {
-		// For cases where there's no text selected, there's a space before
-		// the current caret position and it's at the end of the text.
-		return applyFormat( value, format, value.start - 1, value.end + 1 );
-	} else if ( hasNoSelection && isAtTheEnd ) {
-		// If there's no selection and is at the end of the text
-		// manually add the format within the current caret position.
-		const newFormat = applyFormat( value, format );
-		const { activeFormats } = newFormat;
-		newFormat.formats[ value.start ] = [
-			...( activeFormats?.filter(
-				( { type } ) => type !== format.type
-			) || [] ),
-			format,
-		];
-		return newFormat;
-	} else if ( hasNoSelection ) {
-		return removeFormat( value, format );
-	}
 
+	if ( hasNoSelection ) {
+		contentRef?.onMarkFormatting( color );
+	}
 	return applyFormat( value, format );
 }
 
-function ColorPicker( { name, value, onChange } ) {
+function ColorPicker( { name, value, onChange, contentRef } ) {
 	const property = 'color';
 	const colors = useMobileGlobalStylesColors();
 	const colorSettings = useMultipleOriginColorsAndGradients();
 
 	const onColorChange = useCallback(
 		( color ) => {
-			if ( color !== '' ) {
-				onChange(
-					setColors( value, name, colors, { [ property ]: color } )
-				);
-				// Remove formatting if the color was reset, there's no
-				// current selection and the previous character is a space
-			} else if (
-				value?.start === value?.end &&
-				value.text?.charAt( value?.end - 1 ) === ' '
-			) {
-				onChange(
-					removeFormat( value, name, value.end - 1, value.end )
-				);
-			} else {
-				onChange( removeFormat( value, name ) );
-			}
+			onChange(
+				setColors(
+					value,
+					name,
+					colors,
+					{ [ property ]: color },
+					contentRef
+				)
+			);
 		},
-		[ colors, onChange, property ]
+		[ colors, contentRef, name, onChange, value ]
 	);
 	const activeColors = useMemo(
 		() => getActiveColors( value, name, colors ),
@@ -152,13 +122,20 @@ function ColorPicker( { name, value, onChange } ) {
 		<ColorSettings
 			colorValue={ activeColors[ property ] }
 			onColorChange={ onColorChange }
+			onColorCleared={ onColorChange }
 			defaultSettings={ colorSettings }
 			hideNavigation
 		/>
 	);
 }
 
-export default function InlineColorUI( { name, value, onChange, onClose } ) {
+export default function InlineColorUI( {
+	name,
+	value,
+	onChange,
+	onClose,
+	contentRef,
+} ) {
 	return (
 		<BottomSheet
 			isVisible
@@ -175,6 +152,7 @@ export default function InlineColorUI( { name, value, onChange, onClose } ) {
 						name={ name }
 						value={ value }
 						onChange={ onChange }
+						contentRef={ contentRef }
 					/>
 				</BottomSheet.NavigationScreen>
 			</BottomSheet.NavigationContainer>

diff --git a/packages/format-library/src/text-color/test/__snapshots__/index.native.js.snap b/packages/format-library/src/text-color/test/__snapshots__/index.native.js.snap
@@ -6,12 +6,6 @@ exports[`Text color allows toggling the highlight color feature to selected text
 <!-- /wp:paragraph -->"
 `;
 
-exports[`Text color allows toggling the highlight color feature to type new text 1`] = `
-"<!-- wp:paragraph -->
-<p><mark style="background-color:rgba(0, 0, 0, 0);color:#f78da7" class="has-inline-color has-pale-pink-color"></mark></p>
-<!-- /wp:paragraph -->"
-`;
-
 exports[`Text color creates a paragraph block with the text color format 1`] = `
 "<!-- wp:paragraph -->
 <p>Hello <mark style="background-color:rgba(0,0,0,0);color:#cf2e2e" class="has-inline-color has-vivid-red-color">this is a test</mark></p>

diff --git a/packages/format-library/src/text-color/test/index.native.js b/packages/format-library/src/text-color/test/index.native.js
@@ -64,45 +64,6 @@ describe( 'Text color', () => {
 		expect( textColorButton ).toBeDefined();
 	} );
 
-	it( 'allows toggling the highlight color feature to type new text', async () => {
-		const screen = await initializeEditor();
-
-		// Wait for the editor placeholder
-		const paragraphPlaceholder = await screen.findByLabelText(
-			'Add paragraph block'
-		);
-		expect( paragraphPlaceholder ).toBeDefined();
-		fireEvent.press( paragraphPlaceholder );
-
-		// Wait for the block to be created
-		const [ paragraphBlock ] = await screen.findAllByLabelText(
-			/Paragraph Block\. Row 1/
-		);
-		expect( paragraphBlock ).toBeDefined();
-
-		// Look for the highlight text color button
-		const textColorButton = await screen.findByLabelText( 'Text color' );
-		expect( textColorButton ).toBeDefined();
-		fireEvent.press( textColorButton );
-
-		// Wait for Inline color modal to be visible
-		const inlineTextColorModal = screen.getByTestId(
-			'inline-text-color-modal'
-		);
-		await waitFor( () => inlineTextColorModal.props.isVisible );
-
-		// Look for the pink color button
-		const pinkColorButton = await screen.findByA11yHint( COLOR_PINK );
-		expect( pinkColorButton ).toBeDefined();
-		fireEvent.press( pinkColorButton );
-		// TODO(jest-console): Fix the warning and remove the expect below.
-		expect( console ).toHaveWarnedWith(
-			`Non-serializable values were found in the navigation state. Check:\n\ntext-color > Palette > params.onColorChange (Function)\n\nThis can break usage such as persisting and restoring state. This might happen if you passed non-serializable values such as function, class instances etc. in params. If you need to use components with callbacks in your options, you can use 'navigation.setOptions' instead. See https://reactnavigation.org/docs/troubleshooting#i-get-the-warning-non-serializable-values-were-found-in-the-navigation-state for more details.`
-		);
-
-		expect( getEditorHtml() ).toMatchSnapshot();
-	} );
-
 	it( 'allows toggling the highlight color feature to selected text', async () => {
 		const screen = await initializeEditor();
 		const text = 'Hello this is a test';
@@ -145,6 +106,10 @@ describe( 'Text color', () => {
 		const pinkColorButton = await screen.findByA11yHint( COLOR_PINK );
 		expect( pinkColorButton ).toBeDefined();
 		fireEvent.press( pinkColorButton );
+		// TODO(jest-console): Fix the warning and remove the expect below.
+		expect( console ).toHaveWarnedWith(
+			`Non-serializable values were found in the navigation state. Check:\n\ntext-color > Palette > params.onColorChange (Function)\n\nThis can break usage such as persisting and restoring state. This might happen if you passed non-serializable values such as function, class instances etc. in params. If you need to use components with callbacks in your options, you can use 'navigation.setOptions' instead. See https://reactnavigation.org/docs/troubleshooting#i-get-the-warning-non-serializable-values-were-found-in-the-navigation-state for more details.`
+		);
 
 		expect( getEditorHtml() ).toMatchSnapshot();
 	} );

diff --git a/packages/react-native-aztec/RNTAztecView.podspec b/packages/react-native-aztec/RNTAztecView.podspec
@@ -22,5 +22,5 @@ Pod::Spec.new do |s|
   s.dependency 'React-Core'
   # Intentionally locked because of how it's integrated.
   # See https://github.com/WordPress/gutenberg/pull/54453#discussion_r1325582749
-  s.dependency 'WordPress-Aztec-iOS', '1.19.9'
+  s.dependency 'WordPress-Aztec-iOS', '1.19.11'
 end
diff --git a/packages/react-native-aztec/android/build.gradle b/packages/react-native-aztec/android/build.gradle
@@ -11,7 +11,7 @@ buildscript {
         espressoVersion = '3.0.1'
 
         // libs
-        aztecVersion = 'v1.9.0'
+        aztecVersion = 'v2.1.0'
         wordpressUtilsVersion = '3.3.0'
 
         // main

diff --git a/...-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecManager.java b/...-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecManager.java
@@ -51,6 +51,7 @@
 import org.wordpress.aztec.glideloader.GlideImageLoader;
 import org.wordpress.aztec.glideloader.GlideVideoThumbnailLoader;
 import org.wordpress.aztec.plugins.CssUnderlinePlugin;
+import org.wordpress.aztec.plugins.MarkPlugin;
 import org.wordpress.aztec.plugins.shortcodes.AudioShortcodePlugin;
 import org.wordpress.aztec.plugins.shortcodes.CaptionShortcodePlugin;
 import org.wordpress.aztec.plugins.shortcodes.VideoShortcodePlugin;
@@ -124,6 +125,7 @@ protected ReactAztecText createViewInstance(ThemedReactContext reactContext) {
                         Color.parseColor("#016087"), true)
         ));
         aztecText.addPlugin(new CssUnderlinePlugin());
+        aztecText.addPlugin(new MarkPlugin());
         return aztecText;
     }
 
@@ -651,6 +653,21 @@ public void run() {
         } else if (commandType.equals("blur")) {
             parent.clearFocusFromJS();
             return;
+        } else if (commandType.equals("onMarkFormatting")) {
+            String colorString;
+            Boolean resetColor;
+
+            if (args != null && args.getString(0) != null) {
+                colorString = args.getString(0);
+            } else {
+                colorString = "";
+            }
+
+            parent.onMarkFormatting(colorString);
+            return;
+        } else if (commandType.equals("onRemoveMarkFormatting")) {
+            // This is handled by setActiveFormats
+            return;
         }
         super.receiveCommand(parent, commandType, args);
     }

diff --git a/...ive-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecText.java b/...ive-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecText.java
@@ -238,6 +238,21 @@ void clearFocusFromJS() {
         clearFocus();
     }
 
+    public void onMarkFormatting(String colorString) {
+        inlineFormatter.setMarkStyleColor(colorString);
+
+        Set<ITextFormat> selectedStylesSet = new HashSet<>(getSelectedStyles());
+        Set<ITextFormat> newFormatsSet = new HashSet<>();
+        newFormatsSet.add(AztecTextFormat.FORMAT_MARK);
+
+        selectedStylesSet.removeAll(typingFormatsMap.keySet());
+        selectedStylesSet.addAll(newFormatsSet);
+
+        ArrayList<ITextFormat> newStylesList = new ArrayList<>(selectedStylesSet);
+        setSelectedStyles(newStylesList);
+        updateToolbarButtons(newStylesList);
+    }
+
     @Override
     public void clearFocus() {
         setFocusableInTouchMode(false);
@@ -580,6 +595,7 @@ public void setActiveFormats(Iterable<String> newFormats) {
                     break;
                 case "underline":
                     newFormatsSet.add(AztecTextFormat.FORMAT_UNDERLINE);
+                    break;
                 case "mark":
                     newFormatsSet.add(AztecTextFormat.FORMAT_MARK);
                     break;

diff --git a/packages/react-native-aztec/ios/RNTAztecView/RCTAztecView.swift b/packages/react-native-aztec/ios/RNTAztecView/RCTAztecView.swift
@@ -717,7 +717,13 @@ class RCTAztecView: Aztec.TextView {
         case "bold": toggleBold(range: emptyRange)
         case "italic": toggleItalic(range: emptyRange)
         case "strikethrough": toggleStrikethrough(range: emptyRange)
-        case "mark": toggleMark(range: emptyRange)
+        case "mark":
+            // When there's a selection the formatting is applied from the RichText library.
+            // If not, it will toggle the active mark format if needed.
+            if selectedRange.length > 0 {
+                return
+            }
+            toggleMark(range: emptyRange, color: nil, resetColor: true)
         default: print("Format not recognized")
         }
     }

diff --git a/packages/react-native-aztec/ios/RNTAztecView/RCTAztecViewManager.m b/packages/react-native-aztec/ios/RNTAztecView/RCTAztecViewManager.m
@@ -37,6 +37,7 @@ @interface RCT_EXTERN_MODULE(RCTAztecViewManager, NSObject)
 
 RCT_EXTERN_METHOD(focus:(nonnull NSNumber *)viewTag)
 RCT_EXTERN_METHOD(blur:(nonnull NSNumber *)viewTag)
-
+RCT_EXTERN_METHOD(onMarkFormatting:(nonnull NSNumber *)viewTag : NSString)
+RCT_EXTERN_METHOD(onRemoveMarkFormatting:(nonnull NSNumber *)viewTag)
 
 @end
diff --git a/packages/react-native-aztec/ios/RNTAztecView/RCTAztecViewManager.swift b/packages/react-native-aztec/ios/RNTAztecView/RCTAztecViewManager.swift
@@ -91,5 +91,21 @@ public class RCTAztecViewManager: RCTViewManager {
             aztecView.reactBlur()
         }
     }
+
+    @objc
+    func onMarkFormatting(_ viewTag: NSNumber, _ color: String) {
+        self.executeBlock(viewTag: viewTag) { (aztecView) in
+            let range = NSRange(location: aztecView.selectedRange.location, length: 0)
+            aztecView.toggleMark(range: range, color: color, resetColor: false)
+        }
+    }
+
+    @objc
+    func onRemoveMarkFormatting(_ viewTag: NSNumber) {
+        self.executeBlock(viewTag: viewTag) { (aztecView) in
+            let range = NSRange(location: aztecView.selectedRange.location, length: 0)
+            aztecView.toggleMark(range: range, color: nil, resetColor: true)
+        }
+    }
 }
 
diff --git a/packages/react-native-aztec/src/AztecView.js b/packages/react-native-aztec/src/AztecView.js
@@ -2,6 +2,7 @@
  * External dependencies
  */
 import {
+	findNodeHandle,
 	requireNativeComponent,
 	UIManager,
 	Pressable,
@@ -234,6 +235,22 @@ class AztecView extends Component {
 		return focusedElement && focusedElement === this.aztecViewRef.current;
 	}
 
+	onRemoveMarkFormatting() {
+		UIManager.dispatchViewManagerCommand(
+			findNodeHandle( this.aztecViewRef.current ),
+			'onRemoveMarkFormatting',
+			[]
+		);
+	}
+
+	onMarkFormatting( color ) {
+		UIManager.dispatchViewManagerCommand(
+			findNodeHandle( this.aztecViewRef.current ),
+			'onMarkFormatting',
+			[ color ]
+		);
+	}
+
 	_onPress( event ) {
 		if ( ! this.isFocused() ) {
 			this.focus(); // Call to move the focus in RN way (TextInputState)