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

[Mobile] Highlight color fixes #57650

Merged
merged 31 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
fb7f568
Fix resetting the color
Jan 8, 2024
1e066b1
Temp: Update Aztec reference
Jan 8, 2024
f4ee1d1
Temp: Update Aztec reference
Jan 8, 2024
0ab9509
RichText remove mark as activeFormats
Jan 12, 2024
25c8fec
TextColor use getActiveColors from the native variant
Jan 12, 2024
2a270aa
TextColor - Refactor manual logic to apply the Mark formatting and no…
Jan 12, 2024
0a88e7b
AztecView, don't call toggleMark when activeFormats changes
Jan 12, 2024
2f94e40
Add iOS native methods to set the mark formatting and to remove it
Jan 12, 2024
b21fa81
Merge branch 'trunk' into fix/ios-highlight-text-functionality
Jan 12, 2024
7ab4120
Update Aztec ref
Jan 12, 2024
9fd8150
Merge branch 'trunk' into fix/ios-highlight-text-functionality
Jan 17, 2024
031207c
Add text-color formatting in activeFormats
Jan 19, 2024
d5564a6
Handle onMarkFormatting on the Android side as well
Jan 19, 2024
19c09da
Removes highlight text test when there's no selection since this is m…
Jan 19, 2024
0dc3f95
Merge branch 'trunk' into fix/ios-highlight-text-functionality
Jan 19, 2024
05d67ae
Update Aztec ref
Jan 19, 2024
c644e9a
Update Aztec ref
Jan 19, 2024
ef3a25e
Fix onMarkFormatting logic
Jan 22, 2024
4c545f6
Update Aztec version
Jan 22, 2024
3958f69
Palette screen - Add accessibility label
Jan 22, 2024
d8c6e5a
E2E Helpers - Adds typeKeyString and toggleHighlightColor
Jan 22, 2024
207d93b
Update Aztec ref
Jan 23, 2024
620ced0
Bring back calling toggleMark when activeFormats is set for the Mark …
Jan 24, 2024
950c8c5
Merge branch 'trunk' into fix/ios-highlight-text-functionality
Mar 7, 2024
93b6ea6
Update Changelog
Mar 7, 2024
b9697d5
Update Podfile
Mar 7, 2024
16c9aea
Merge branch 'trunk' into fix/ios-highlight-text-functionality
Mar 18, 2024
99b9fad
Update Podfile.lock
Mar 18, 2024
8bf9e20
getFormatColors - Fix appending undefined values
Mar 18, 2024
40ce1d8
Update Aztec version
Mar 21, 2024
ccdb629
Update Aztec to 1.19.11
Mar 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function getFormatColors( formats, colors ) {
);
const currentStyles = currentFormat?.attributes?.style;
if (
colorObject &&
colorObject?.color &&
( ! currentStyles ||
currentStyles?.indexOf( colorObject.color ) ===
-1 )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion packages/format-library/src/text-color/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { usePreferredColorSchemeStyle } from '@wordpress/compose';
/**
* Internal dependencies
*/
import { getActiveColors } from './inline.js';
import { getActiveColors } from './inline.native.js';
dcalhoun marked this conversation as resolved.
Show resolved Hide resolved
import { default as InlineColorUI } from './inline';
import styles from './style.scss';

Expand Down
76 changes: 27 additions & 49 deletions packages/format-library/src/text-color/inline.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand All @@ -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 );
}

Expand Down Expand Up @@ -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 );
}
dcalhoun marked this conversation as resolved.
Show resolved Hide resolved
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 ),
Expand All @@ -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
Expand All @@ -175,6 +152,7 @@ export default function InlineColorUI( { name, value, onChange, onClose } ) {
name={ name }
value={ value }
onChange={ onChange }
contentRef={ contentRef }
/>
</BottomSheet.NavigationScreen>
</BottomSheet.NavigationContainer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
43 changes: 4 additions & 39 deletions packages/format-library/src/text-color/test/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,45 +64,6 @@ describe( 'Text color', () => {
expect( textColorButton ).toBeDefined();
} );

it( 'allows toggling the highlight color feature to type new text', async () => {
geriux marked this conversation as resolved.
Show resolved Hide resolved
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';
Expand Down Expand Up @@ -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();
} );
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-aztec/RNTAztecView.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion packages/react-native-aztec/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ buildscript {
espressoVersion = '3.0.1'

// libs
aztecVersion = 'v1.9.0'
aztecVersion = 'v2.1.0'
wordpressUtilsVersion = '3.3.0'

// main
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -124,6 +125,7 @@ protected ReactAztecText createViewInstance(ThemedReactContext reactContext) {
Color.parseColor("#016087"), true)
));
aztecText.addPlugin(new CssUnderlinePlugin());
aztecText.addPlugin(new MarkPlugin());
return aztecText;
}

Expand Down Expand Up @@ -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;
dcalhoun marked this conversation as resolved.
Show resolved Hide resolved
}
super.receiveCommand(parent, commandType, args);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -580,6 +595,7 @@ public void setActiveFormats(Iterable<String> newFormats) {
break;
case "underline":
newFormatsSet.add(AztecTextFormat.FORMAT_UNDERLINE);
break;
dcalhoun marked this conversation as resolved.
Show resolved Hide resolved
case "mark":
newFormatsSet.add(AztecTextFormat.FORMAT_MARK);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
dcalhoun marked this conversation as resolved.
Show resolved Hide resolved
return
}
toggleMark(range: emptyRange, color: nil, resetColor: true)
default: print("Format not recognized")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

17 changes: 17 additions & 0 deletions packages/react-native-aztec/src/AztecView.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import {
findNodeHandle,
requireNativeComponent,
UIManager,
Pressable,
Expand Down Expand Up @@ -234,6 +235,22 @@ class AztecView extends Component {
return focusedElement && focusedElement === this.aztecViewRef.current;
}

onRemoveMarkFormatting() {
UIManager.dispatchViewManagerCommand(
findNodeHandle( this.aztecViewRef.current ),
'onRemoveMarkFormatting',
[]
);
geriux marked this conversation as resolved.
Show resolved Hide resolved
}

onMarkFormatting( color ) {
UIManager.dispatchViewManagerCommand(
findNodeHandle( this.aztecViewRef.current ),
'onMarkFormatting',
[ color ]
);
geriux marked this conversation as resolved.
Show resolved Hide resolved
}

_onPress( event ) {
if ( ! this.isFocused() ) {
this.focus(); // Call to move the focus in RN way (TextInputState)
Expand Down
Loading
Loading