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

Jump to bottom

Don't send empty paragraph tags to view on Android #22561

Merged
merged 14 commits into from
Aug 31, 2020
Merged

Don't send empty paragraph tags to view on Android #22561

Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ca2cf3b
Don't send empty paragraph tags to view on Android
cameronvoell May 23, 2020
c7cb54a
lint fixes
cameronvoell May 23, 2020
eace29c
Limit getHtmlForRecordCreation calls to Android
cameronvoell May 26, 2020
90f8442
Removed unnecessary out of bounds check
cameronvoell May 26, 2020
47494b5
Try setting quote tagName to p instead of removing empty paragraph ta…
cameronvoell May 28, 2020
4bd5342
Revert setting tagName to p
cameronvoell Aug 13, 2020
242c6e0
Merge branch 'master' into rnmobile/fix-missing-placeholder-quote-blocks
cameronvoell Aug 13, 2020
f664971
Re-add multiline tag when no longer empty on edit
cameronvoell Aug 13, 2020
c645872
Merge branch 'master' into rnmobile/fix-missing-placeholder-quote-blocks
cameronvoell Aug 21, 2020
76cb481
Merge branch 'master' into rnmobile/fix-missing-placeholder-quote-blocks
cameronvoell Aug 28, 2020
2eaf1ec
Only re-add p tag from onTextUpdate
cameronvoell Aug 28, 2020
ddfea57
Distinguish props.onChange from onChangeFromAztec
cameronvoell Aug 28, 2020
8f9bd43
Updated constant to more precise name
cameronvoell Aug 31, 2020
b3c66fe
Merge branch 'master' into rnmobile/fix-missing-placeholder-quote-blocks
cameronvoell Aug 31, 2020
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
37 changes: 29 additions & 8 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 @@ -59,6 +59,8 @@ const gutenbergFormatNamesToAztec = {
'core/strikethrough': 'strikethrough',
};

const EMPTY_PARAGRAPH_TAGS = '<p></p>';

export class RichText extends Component {
constructor( {
value,
Expand All @@ -80,7 +82,8 @@ export class RichText extends Component {

this.isIOS = Platform.OS === 'ios';
this.createRecord = this.createRecord.bind( this );
this.onChange = this.onChange.bind( this );
this.restoreParagraphTags = this.restoreParagraphTags.bind( this );
this.onChangeFromAztec = this.onChangeFromAztec.bind( this );
this.onKeyDown = this.onKeyDown.bind( this );
this.handleEnter = this.handleEnter.bind( this );
this.handleDelete = this.handleDelete.bind( this );
Expand Down Expand Up @@ -264,7 +267,7 @@ export class RichText extends Component {
/*
* Handles any case where the content of the AztecRN instance has changed
*/
onChange( event ) {
onChangeFromAztec( event ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Updated method name to help distinguish from this.props.onChange which is used many times elsewhere in this class.

const contentWithoutRootTag = this.removeRootTagsProduceByAztec(
unescapeSpaces( event.nativeEvent.text )
);
Expand All @@ -283,14 +286,29 @@ export class RichText extends Component {
const contentWithoutRootTag = this.removeRootTagsProduceByAztec(
unescapeSpaces( event.nativeEvent.text )
);
let formattedContent = contentWithoutRootTag;
if ( ! this.isIOS ) {
formattedContent = this.restoreParagraphTags(
Copy link
Member Author

@cameronvoell cameronvoell Aug 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Text with removed paragraph tags from https://github.com/WordPress/gutenberg/pull/22561/files#r478829512 can not pass from RCTAztecView to the RichText value without coming through this call in the onTextUpdate function.

contentWithoutRootTag,
this.multilineTag
);
}

this.debounceCreateUndoLevel();
const refresh = this.value !== contentWithoutRootTag;
this.value = contentWithoutRootTag;
const refresh = this.value !== formattedContent;
this.value = formattedContent;

// we don't want to refresh if our goal is just to create a record
if ( refresh ) {
this.props.onChange( contentWithoutRootTag );
this.props.onChange( formattedContent );
}
}

restoreParagraphTags( value, tag ) {
if ( tag === 'p' && ( ! value || ! value.startsWith( '<p>' ) ) ) {
return '<p>' + value + '</p>';
}
return value;
}

/*
Expand Down Expand Up @@ -708,8 +726,11 @@ export class RichText extends Component {
value = '';
}
// On android if content is empty we need to send no content or else the placeholder will not show.
if ( ! this.isIOS && value === '' ) {
return value;
if (
! this.isIOS &&
( value === '' || value === EMPTY_PARAGRAPH_TAGS )
) {
return '';
Copy link
Member Author

@cameronvoell cameronvoell Aug 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This ensures that empty paragraph tags do not get passed to the RCTAztecView (thus hiding the placeholder)

}

if ( tagName ) {
Expand Down Expand Up @@ -848,7 +869,7 @@ export class RichText extends Component {
defaultPlaceholderTextColor
}
deleteEnter={ this.props.deleteEnter }
onChange={ this.onChange }
onChange={ this.onChangeFromAztec }
onFocus={ this.onFocus }
onBlur={ this.onBlur }
onKeyDown={ this.onKeyDown }
Expand Down