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

Rnmobile/refactor rich text sizing code #14164

Merged
merged 6 commits into from
Mar 4, 2019
Merged

Rnmobile/refactor rich text sizing code #14164

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2991d5a
Make richText height changes contained to the rich text block.
SergioEstevao Feb 28, 2019
a53c580
Remove commented out code.
SergioEstevao Feb 28, 2019
8b705c9
Remove aztec height from state.
SergioEstevao Feb 28, 2019
c39453c
Allow minHeight to be optional.
SergioEstevao Feb 28, 2019
c18e528
Remove minHeight from postTitle.
SergioEstevao Mar 4, 2019
227b67a
Remove minHeight on heading block.
SergioEstevao Mar 4, 2019
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
16 changes: 2 additions & 14 deletions packages/block-library/src/heading/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,13 @@ import { createBlock } from '@wordpress/blocks';
import styles from './style.scss';

class HeadingEdit extends Component {
constructor( props ) {
super( props );

this.state = {
aztecHeight: 0,
};
}

render() {
const {
attributes,
setAttributes,
mergeBlocks,
insertBlocksAfter,
style,
} = this.props;

const {
Expand All @@ -57,12 +50,10 @@ class HeadingEdit extends Component {
tagName={ tagName }
value={ content }
isSelected={ this.props.isSelected }
style={ { ...style, minHeight } }
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not it be declared the other way round? style={ { minHeight, ...style } }

Otherwise if I set a style to a particular Heading block that has a minHeight different from the default value const minHeight = styles.blockText.minHeight; it will be override by this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct let me correct this.

onFocus={ this.props.onFocus } // always assign onFocus as a props
onBlur={ this.props.onBlur } // always assign onBlur as a props
onCaretVerticalPositionChange={ this.props.onCaretVerticalPositionChange }
style={ {
minHeight: Math.max( minHeight, this.state.aztecHeight ),
} }
onChange={ ( value ) => setAttributes( { content: value } ) }
onMerge={ mergeBlocks }
onSplit={
Expand All @@ -76,9 +67,6 @@ class HeadingEdit extends Component {
} :
undefined
}
onContentSizeChange={ ( event ) => {
this.setState( { aztecHeight: event.aztecHeight } );
} }
placeholder={ placeholder || __( 'Write heading…' ) }
/>
</View>
Expand Down
15 changes: 1 addition & 14 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 @@ -14,7 +14,6 @@ import { RichText } from '@wordpress/editor';
/**
* Internal dependencies
*/
import styles from './style.scss';

const name = 'core/paragraph';

Expand All @@ -23,10 +22,6 @@ class ParagraphEdit extends Component {
super( props );
this.splitBlock = this.splitBlock.bind( this );
this.onReplace = this.onReplace.bind( this );

this.state = {
aztecHeight: 0,
};
}

/**
Expand Down Expand Up @@ -99,8 +94,6 @@ class ParagraphEdit extends Component {
content,
} = attributes;

const minHeight = styles.blockText.minHeight;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear to me why minHeight is not used in here in Para block, instead it's still used in Heading and Title.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was testing if we actually needed this, and find out that we don't... we don't need to force a minHeight the component will size itself to whatever text size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the minHeight from all blocks that were using it. So that way the block code will be less aware of the platform.


return (
<View>
<RichText
Expand All @@ -110,10 +103,7 @@ class ParagraphEdit extends Component {
onFocus={ this.props.onFocus } // always assign onFocus as a props
onBlur={ this.props.onBlur } // always assign onBlur as a props
onCaretVerticalPositionChange={ this.props.onCaretVerticalPositionChange }
style={ {
...style,
minHeight: Math.max( minHeight, this.state.aztecHeight ),
} }
style={ style }
onChange={ ( nextContent ) => {
setAttributes( {
content: nextContent,
Expand All @@ -122,9 +112,6 @@ class ParagraphEdit extends Component {
onSplit={ this.splitBlock }
onMerge={ mergeBlocks }
onReplace={ this.onReplace }
onContentSizeChange={ ( event ) => {
this.setState( { aztecHeight: event.aztecHeight } );
} }
placeholder={ placeholder || __( 'Start writing…' ) }
/>
</View>
Expand Down
7 changes: 1 addition & 6 deletions packages/editor/src/components/post-title/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,12 @@ class PostTitle extends Component {
onFocus={ this.onSelect }
onBlur={ this.props.onBlur } // always assign onBlur as a props
multiline={ false }
style={ [ style, {
minHeight: Math.max( minHeight, this.state.aztecHeight ),
} ] }
style={ { ...style, minHeight } }
fontSize={ 24 }
fontWeight={ 'bold' }
onChange={ ( value ) => {
this.props.onUpdate( value );
} }
onContentSizeChange={ ( event ) => {
this.setState( { aztecHeight: event.aztecHeight } );
} }
placeholder={ decodedPlaceholder }
value={ title }
onSplit={ this.props.onEnterPress }
Expand Down
14 changes: 10 additions & 4 deletions packages/editor/src/components/rich-text/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export class RichText extends Component {
start: 0,
end: 0,
formatPlaceholder: null,
height: 0,
};
}

Expand Down Expand Up @@ -236,9 +237,7 @@ export class RichText extends Component {

onContentSizeChange( contentSize ) {
const contentHeight = contentSize.height;
this.props.onContentSizeChange( {
aztecHeight: contentHeight,
} );
this.setState( { height: contentHeight } );
}

// eslint-disable-next-line no-unused-vars
Expand Down Expand Up @@ -501,6 +500,10 @@ export class RichText extends Component {
html = '';
this.lastEventCount = undefined; // force a refresh on the native side
}
let minHeight = 0;
if ( style && style.minHeight ) {
minHeight = style.minHeight;
}

return (
<View>
Expand All @@ -517,6 +520,10 @@ export class RichText extends Component {
this.props.setRef( ref );
}
} }
style={ {
...style,
minHeight: Math.max( minHeight, this.state.height ),
} }
text={ { text: html, eventCount: this.lastEventCount } }
placeholder={ this.props.placeholder }
placeholderTextColor={ this.props.placeholderTextColor || styles[ 'editor-rich-text' ].textDecorationColor }
Expand All @@ -534,7 +541,6 @@ export class RichText extends Component {
blockType={ { tag: tagName } }
color={ 'black' }
maxImagesWidth={ 200 }
style={ style }
fontFamily={ this.props.fontFamily || styles[ 'editor-rich-text' ].fontFamily }
fontSize={ this.props.fontSize }
fontWeight={ this.props.fontWeight }
Expand Down