HTMLInputView refactor to better share code.

[etoledom]

This PR tries to implement HTMLInputView in a way that more code is shared between platforms, and branching just the necessary parts.

Since there were challenges sharing the HTML TextView that is the main component, and needs a slightly different configuration, the final result is not as strait forward as one would have liked.

Ideally we can check this out, and see if we like it enough to merge.
If this is more complex than it helps, it's all good to not merge and keep HTMLInputView as it is.

[daniloercoli]

Ideally we can check this out, and see if we like it enough to merge.
If this is more complex than it helps, it's all good to not merge and keep HTMLInputView as it is.

I like it, and don't think it is way more complex, but I checked the code some times already, that i'm familiar with it.
Let's see @pinarol thoughts on this PR.

[pinarol]

hey @etoledom thank you for tackling this ! I can understand the code but I've been maintaining this code for the last 3-4 weeks, I've also written unit-tests for it so currently I almost memorized every part of it :) but for someone who'll see this code for the first time I don't think it is very intuitive.

Although, I'd want to keep most of the things about this PR.

I'd just try not to get away from standard way of adding a child component in React Native, which is just adding <Parent> <Child/> <Parent/>.

So instead of dynamically creating the content we can just:

	render() {
		return (
			<HTMLInputViewUI
				parentHeight={ this.props.parentHeight }>
				<View style={ styles.container }>
					<TextInput
						autoCorrect={ false }
						textAlignVertical="center"
						numberOfLines={ 1 }
						style={ styles.htmlViewTitle }
						value={ this.props.title }
						placeholder={ __( 'Add title' ) }
						onChangeText={ this.props.setTitleAction }
					/>
					<TextInput
						autoCorrect={ false }
						textAlignVertical="top"
						multiline
						style={ [styles.htmlView, this.additionalStyle()] }
						value={ this.state.value }
						onChangeText={ this.edit }
						onBlur={ this.stopEditing }
						placeholder={ __( 'Start writing…' ) }
						scrollEnabled={ this.scrollEnabled() }
						onContentSizeChange={ this.onContentSizeChange } //we can also add a platform check here
					/>
				</View>
			</HTMLInputViewUI>
		);
	}


	onContentSizeChange = ( event: NativeSyntheticEvent<TextInputContentSizeChangeEventData> ) => {
//Or we can add the platform check here
		this.setState( { contentHeight: event.nativeEvent.contentSize.height } );
	}

additionalStyle() {
  if ( this.isAndroid ) { 
      return  { height: this.state.height + 16 } 
  }
  return { } 
}

scrollEnabled() { 
  return !this.isAndroid;
}

I know this adds some isAndroid checks but I think it is much more simpler to understand. Personally, it feels more safe to me in terms of performance also, since it is the standard way of adding a child component(haven't calculated any metrics though).

In this case I think renaming HTMLInputViewUI as something like HTMLInputViewContainer would make sense.

[etoledom]

Thank you @daniloercoli and @pinarol for the review.

I'd just try not to get away from standard way of adding a child component in React Native, which is just adding .

This and the code sample gave me an idea to simplify everything a bit more.
I noticed that the dynamic height on Android is not really necessary. If we don't set the height, it will adjust automatically, and to add the extra bottom padding, we can do it directly on CSS.

@pinarol please let me know if that's what you had in mind :)
It does looks better to me 👍

[pinarol]

I noticed that the dynamic height on Android is not really necessary. If we don't set the height, it will adjust automatically, and to add the extra bottom padding, we can do it directly on CSS.

That's so interesting, sth must have changed on Android side 🤔But it is good news also. I tested on Android and it works all ok with the current version of the code!

[pinarol]

@pinarol please let me know if that's what you had in mind :)

yes exactly, even better if you consider we don't need the dynamic height anymore.

[pinarol]

Thanks @etoledom I think this looks awesome! I just added some very simple comments to remove unnecessary lines.

I tested with both ios and android, working pretty good! But it'd be good if @daniloercoli could also approve for Android side.

[etoledom]

Thank you @pinarol !
Removed unnecessary lines and now it looks even more simpler 🎉