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.

Sign up for GitHub

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

[RNMobile][Android] Fix jumping keyboard when adding a new paragraph using "return" key #28101

Closed
wants to merge 11 commits into from
Closed

[RNMobile][Android] Fix jumping keyboard when adding a new paragraph using "return" key #28101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
406a796
POC keep clientID in the original paragraph
dratwas Jan 11, 2021
a31797f
fix docs param
dratwas Jan 11, 2021
81d4594
little refactor
dratwas Jan 11, 2021
77fc22a
add fix for non rich-text blocks
dratwas Jan 12, 2021
d2e8e06
fix lint error
dratwas Jan 12, 2021
8b81cd4
Fix heading block
dratwas Jan 12, 2021
206376f
add docs changes
dratwas Jan 13, 2021
16fd756
remove id from createBlock and use original block instead
dratwas Jan 25, 2021
665f37e
revert changes in factory.js
dratwas Jan 25, 2021
84cb343
add additional check for editor before ivoking blur
dratwas Jan 25, 2021
ee8b359
implement web version of paragraph
dratwas Jan 26, 2021
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
4 changes: 3 additions & 1 deletion packages/block-editor/src/components/rich-text/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,9 @@ function RichTextWrapper(
toHTMLString( {
value: before,
multilineTag,
} )
} ),
// Set true to not recreate the original block
true
)
);
lastPastedBlockIndex += 1;
Expand Down
20 changes: 19 additions & 1 deletion packages/block-library/src/heading/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
} from '@wordpress/block-editor';
import { ToolbarGroup } from '@wordpress/components';

import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
Expand All @@ -27,8 +29,14 @@ function HeadingEdit( {
mergeBlocks,
onReplace,
mergedStyle,
clientId,
} ) {
const { textAlign, content, level, placeholder } = attributes;
const block = useSelect(
( select ) => select( 'core/block-editor' ).getBlock( clientId ),
[]
);

const tagName = 'h' + level;
const blockProps = useBlockProps( {
className: classnames( {
Expand Down Expand Up @@ -61,7 +69,17 @@ function HeadingEdit( {
value={ content }
onChange={ ( value ) => setAttributes( { content: value } ) }
onMerge={ mergeBlocks }
onSplit={ ( value ) => {
onSplit={ ( value, keepId ) => {
if ( keepId ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to have the "keepId" argument? I mean why not just replace the createBlock( 'core/heading' call a few lines down here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we still need the keepId since it determines if we should use the original block or create a new one. W/o that we wouldn't know if we should create a new block or make changes in the "original" one. I am wondering if I could use getBlock in the rich-text component and use onSplit only for creating new blocks but I'm not sure if it's a good way because we would lose the "flexibility" in that mechanism (currently it could work differently for heading, paragraph, list etc).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding keepId is a horrible API. We should go for one or the other for all cases. Any alternatives we can explore here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding keepId is a horrible API. We should go for one or the other for all cases.

Can you elaborate a bit? :)

IMHO it is still better than removing the original block and replacing it. Even from the performance perspective update is faster than unmount + mount (we use the clientId as a key in our block-list).

Any alternatives we can explore here?

As I mentioned I was wondering if it could be done inside rich-text/index.js. We could get block and change attributes inside instead of calling onSplit for the original block but i haven't investigated it.

return {
...block,
attributes: {
...block.attributes,
content: value || '',
},
};
}

if ( ! value ) {
return createBlock( 'core/paragraph' );
}
Expand Down
22 changes: 18 additions & 4 deletions packages/block-library/src/paragraph/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ function ParagraphBlock( {
onReplace,
onRemove,
setAttributes,
clientId,
} ) {
const {
align,
Expand All @@ -85,10 +86,14 @@ function ParagraphBlock( {
const isDropCapFeatureEnabled = useEditorFeature( 'typography.dropCap' );
const ref = useRef();
const inlineFontSize = style?.fontSize;
const size = useSelect(
const { size, block } = useSelect(
( select ) => {
const { fontSizes } = select( 'core/block-editor' ).getSettings();
return getFontSize( fontSizes, fontSize, inlineFontSize ).size;
const { getBlock, getSettings } = select( 'core/block-editor' );
const { fontSizes } = getSettings();
return {
size: getFontSize( fontSizes, fontSize, inlineFontSize ).size,
block: getBlock( clientId ),
};
},
[ fontSize, inlineFontSize ]
);
Expand Down Expand Up @@ -147,7 +152,16 @@ function ParagraphBlock( {
onChange={ ( newContent ) =>
setAttributes( { content: newContent } )
}
onSplit={ ( value ) => {
onSplit={ ( value, keepId ) => {
if ( keepId ) {
return {
...block,
attributes: {
...block.attributes,
content: value || '',
},
};
}
if ( ! value ) {
return createBlock( name );
}
Expand Down
21 changes: 17 additions & 4 deletions packages/block-library/src/paragraph/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ function ParagraphBlock( {
setAttributes,
mergedStyle,
style,
clientId,
} ) {
const isRTL = useSelect( ( select ) => {
return !! select( 'core/block-editor' ).getSettings().isRTL;
const { isRTL, block } = useSelect( ( select ) => {
const { getBlock, getSettings } = select( 'core/block-editor' );
return {
isRTL: !! getSettings().isRTL,
block: getBlock( clientId ),
};
}, [] );

const { align, content, placeholder } = attributes;

const styles = {
Expand Down Expand Up @@ -53,7 +57,16 @@ function ParagraphBlock( {
content: nextContent,
} );
} }
onSplit={ ( value ) => {
onSplit={ ( value, keepId ) => {
if ( keepId ) {
return {
...block,
attributes: {
...block.attributes,
content: value || '',
},
};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do the same for the web version of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we still need the keepId, unless you have a different concept for it.

I already added it to the web version :)

if ( ! value ) {
return createBlock( name );
}
Expand Down
12 changes: 7 additions & 5 deletions packages/rich-text/src/component/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* WordPress dependencies
*/
import RCTAztecView from '@wordpress/react-native-aztec';
import { View, Platform } from 'react-native';
import { View, Platform, InteractionManager } from 'react-native';
import {
showUserSuggestions,
showXpostSuggestions,
Expand Down Expand Up @@ -730,8 +730,8 @@ export class RichText extends Component {
}

componentWillUnmount() {
if ( this._editor.isFocused() ) {
this._editor.blur();
if ( this._editor?.isFocused() ) {
this._editor?.blur();
}
}

Expand All @@ -745,15 +745,17 @@ export class RichText extends Component {
const { __unstableIsSelected: prevIsSelected } = prevProps;

if ( isSelected && ! prevIsSelected ) {
this._editor.focus();
this._editor?.focus();
// Update selection props explicitly when component is selected as Aztec won't call onSelectionChange
// if its internal value hasn't change. When created, default value is 0, 0
this.onSelectionChange(
this.props.selectionStart || 0,
this.props.selectionEnd || 0
);
} else if ( ! isSelected && prevIsSelected ) {
this._editor.blur();
InteractionManager.runAfterInteractions( () => {
this._editor?.blur();
} );
}
}

Expand Down