[Mobile] Highlight color fixes

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 )

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>

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';
 

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>

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>

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();
 	} );

packages/react-native-aztec/android/build.gradle

@@ -11,7 +11,7 @@ buildscript {
         espressoVersion = '3.0.1'
 
         // libs
-        aztecVersion = 'v1.9.0'
+        aztecVersion = '1073-9b97880c701a493dbce23c401e2d2453a6e1e089'
         wordpressUtilsVersion = '3.3.0'
 
         // main

packages/react-native-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);
     }

packages/react-native-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;

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")
         }
     }

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

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)
+        }
+    }
 }
 

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)

packages/react-native-editor/CHANGELOG.md

@@ -10,6 +10,7 @@ For each user feature we should also add a importance categorization label  to i
 -->
 
 ## Unreleased
+-   [**] Highlight color formatting style improvements [#57650]
 
 ## 1.115.0
 -   [*] Improve consistency of the block outline indicating the currently selected block [#59415]