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 - Fix issue when backspacing in an empty Paragraph block #56496

Merged
merged 10 commits into from
Nov 28, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,8 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
}

moveFirstItemUp( rootClientId );
} else {
removeBlock( clientId );
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ function RichTextWrapper(
// an intentional user interaction distinguishing between Backspace and
// Delete to remove the empty field, but also to avoid merge & remove
// causing destruction of two fields (merge, then removed merged).
if ( onRemove && isEmpty( value ) && isReverse ) {
else if ( onRemove && isEmpty( value ) && isReverse ) {
onRemove( ! isReverse );
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,40 @@ export class RichText extends Component {
return shouldDrop;
}

/**
* Determines whether the text input should receive focus after an update.
* For cases where a RichText with a value is merged with an empty one.
*
* @param {Object} prevProps - The previous props of the component.
* @return {boolean} True if the text input should receive focus, false otherwise.
*/
shouldFocusTextInputAfterUpdate( prevProps ) {
const {
__unstableIsSelected: isSelected,
blockIsSelected,
selectionStart,
selectionEnd,
__unstableMobileNoFocusOnMount,
} = this.props;

const {
__unstableIsSelected: prevIsSelected,
blockIsSelected: prevBlockIsSelected,
} = prevProps;

const noSelectionValues =
selectionStart === undefined && selectionEnd === undefined;
const textInputWasNotFocused = ! prevIsSelected && ! isSelected;

return (
! __unstableMobileNoFocusOnMount &&
noSelectionValues &&
textInputWasNotFocused &&
! prevBlockIsSelected &&
blockIsSelected
);
}

onSelectionChangeFromAztec( start, end, text, event ) {
if ( this.shouldDropEventFromAztec( event, 'onSelectionChange' ) ) {
return;
Expand Down Expand Up @@ -843,9 +877,8 @@ export class RichText extends Component {
if ( this.props.value !== this.value ) {
this.value = this.props.value;
}
const { __unstableIsSelected: isSelected } = this.props;

const { __unstableIsSelected: prevIsSelected } = prevProps;
const { __unstableIsSelected: isSelected } = this.props;

if ( isSelected && ! prevIsSelected ) {
geriux marked this conversation as resolved.
Show resolved Hide resolved
this._editor.focus();
Expand All @@ -855,6 +888,16 @@ export class RichText extends Component {
this.props.selectionStart || 0,
this.props.selectionEnd || 0
);
} else if ( this.shouldFocusTextInputAfterUpdate( prevProps ) ) {
// Since this is happening when merging blocks, the selection should be at the last character position.
// As a fallback the internal selectionEnd value is used.
const lastCharacterPosition =
this.value?.length ?? this.selectionEnd;
this._editor.focus();
this.props.onSelectionChange(
lastCharacterPosition,
lastCharacterPosition
);
} else if ( ! isSelected && prevIsSelected ) {
this._editor.blur();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,3 @@ exports[`Buttons block when a button is shown removing button along with buttons
<p></p>
<!-- /wp:paragraph -->"
`;

exports[`Buttons block when a button is shown removing button along with buttons block removes the button and buttons block when deleting the block using the delete (backspace) key 1`] = `
"<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->"
`;
27 changes: 0 additions & 27 deletions packages/block-library/src/buttons/test/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
*/
import { getBlockTypes, unregisterBlockType } from '@wordpress/blocks';
import { registerCoreBlocks } from '@wordpress/block-library';
import { BACKSPACE } from '@wordpress/keycodes';

const BUTTONS_HTML = `<!-- wp:buttons -->
<div class="wp-block-buttons"><!-- wp:button /--></div>
Expand Down Expand Up @@ -238,32 +237,6 @@ describe( 'Buttons block', () => {

expect( getEditorHtml() ).toMatchSnapshot();
} );

it( 'removes the button and buttons block when deleting the block using the delete (backspace) key', async () => {
const screen = await initializeEditor( {
initialHtml: BUTTONS_HTML,
} );

// Get block
const buttonsBlock = await getBlock( screen, 'Buttons' );
triggerBlockListLayout( buttonsBlock );

// Get inner button block
const buttonBlock = await getBlock( screen, 'Button' );
fireEvent.press( buttonBlock );

const buttonInput =
within( buttonBlock ).getByLabelText( 'Text input. Empty' );

// Delete block
fireEvent( buttonInput, 'onKeyDown', {
nativeEvent: {},
preventDefault() {},
keyCode: BACKSPACE,
} );

expect( getEditorHtml() ).toMatchSnapshot();
} );
} );
} );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ exports[`Heading block inserts block 1`] = `
<!-- /wp:heading -->"
`;

exports[`Heading block should merge with an empty Paragraph block and keep being the Heading block 1`] = `
"<!-- wp:heading -->
<h2 class="wp-block-heading">A quick brown fox jumps over the lazy dog.</h2>
<!-- /wp:heading -->"
`;

exports[`Heading block should set a background color 1`] = `
"<!-- wp:heading {"backgroundColor":"luminous-vivid-orange"} -->
<h2 class="wp-block-heading has-luminous-vivid-orange-background-color has-background">A quick brown fox jumps over the lazy dog.</h2>
Expand Down
40 changes: 40 additions & 0 deletions packages/block-library/src/heading/test/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
*/
import { getBlockTypes, unregisterBlockType } from '@wordpress/blocks';
import { registerCoreBlocks } from '@wordpress/block-library';
import { BACKSPACE, ENTER } from '@wordpress/keycodes';

beforeAll( () => {
// Register all core blocks
Expand Down Expand Up @@ -134,4 +135,43 @@ describe( 'Heading block', () => {
)
).toBeVisible();
} );

it( 'should merge with an empty Paragraph block and keep being the Heading block', async () => {
// Arrange
const screen = await initializeEditor();
await addBlock( screen, 'Paragraph' );

// Act
const paragraphBlock = getBlock( screen, 'Paragraph' );
fireEvent.press( paragraphBlock );

const paragraphTextInput =
within( paragraphBlock ).getByPlaceholderText( 'Start writing…' );
fireEvent( paragraphTextInput, 'onKeyDown', {
nativeEvent: {},
preventDefault() {},
keyCode: ENTER,
} );

await addBlock( screen, 'Heading' );
const headingBlock = getBlock( screen, 'Heading', { rowIndex: 2 } );
fireEvent.press( headingBlock );

const headingTextInput =
within( headingBlock ).getByPlaceholderText( 'Heading' );
typeInRichText(
headingTextInput,
'A quick brown fox jumps over the lazy dog.',
{ finalSelectionStart: 0, finalSelectionEnd: 0 }
);

fireEvent( headingTextInput, 'onKeyDown', {
nativeEvent: {},
preventDefault() {},
keyCode: BACKSPACE,
} );

// Assert
expect( getEditorHtml() ).toMatchSnapshot();
} );
} );
36 changes: 35 additions & 1 deletion packages/block-library/src/paragraph/test/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import Clipboard from '@react-native-clipboard/clipboard';
/**
* WordPress dependencies
*/
import { ENTER } from '@wordpress/keycodes';
import { BACKSPACE, ENTER } from '@wordpress/keycodes';

/**
* Internal dependencies
Expand Down Expand Up @@ -685,4 +685,38 @@ describe( 'Paragraph block', () => {
<!-- /wp:paragraph -->"
` );
} );

it( 'should focus on the previous Paragraph block when backspacing in an empty Paragraph block', async () => {
// Arrange
const screen = await initializeEditor();
geriux marked this conversation as resolved.
Show resolved Hide resolved
await addBlock( screen, 'Paragraph' );

// Act
const paragraphBlock = getBlock( screen, 'Paragraph' );
fireEvent.press( paragraphBlock );
const paragraphTextInput =
within( paragraphBlock ).getByPlaceholderText( 'Start writing…' );
typeInRichText( paragraphTextInput, 'A quick brown fox jumps' );

await addBlock( screen, 'Paragraph' );
const secondParagraphBlock = getBlock( screen, 'Paragraph', {
rowIndex: 2,
} );
fireEvent.press( secondParagraphBlock );

const secondParagraphTextInput =
within( secondParagraphBlock ).getByPlaceholderText(
'Start writing…'
);
fireEvent( secondParagraphTextInput, 'onKeyDown', {
nativeEvent: {},
preventDefault() {},
keyCode: BACKSPACE,
} );

// Assert
// Check for formatting options to make sure the RichText Input is focused
const boldFormattingButton = screen.getByLabelText( 'Bold' );
expect( boldFormattingButton ).toBeVisible();
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
} );
} );
8 changes: 4 additions & 4 deletions test/native/integration-test-helpers/add-block.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Platform } from '@wordpress/element';
/**
* External dependencies
*/
import { act, fireEvent } from '@testing-library/react-native';
import { act, fireEvent, within } from '@testing-library/react-native';
import { AccessibilityInfo } from 'react-native';

/**
Expand All @@ -31,17 +31,17 @@ export const addBlock = async (
fireEvent.press( screen.getByLabelText( 'Add block' ) );
}

const blockList = screen.getByTestId( 'InserterUI-Blocks' );
const inserterModal = screen.getByTestId( 'InserterUI-Blocks' );
// onScroll event used to force the FlatList to render all items
fireEvent.scroll( blockList, {
fireEvent.scroll( inserterModal, {
nativeEvent: {
contentOffset: { y: 0, x: 0 },
contentSize: { width: 100, height: 100 },
layoutMeasurement: { width: 100, height: 100 },
},
} );

const blockButton = await screen.findByText( blockName );
const blockButton = await within( inserterModal ).findByText( blockName );
// Blocks can perform belated state updates after they are inserted.
// To avoid potential `act` warnings, we ensure that all timers and queued
// microtasks are executed.
Expand Down
Loading