From cc87383d7fbb387d843169c23ab16f35988309fd Mon Sep 17 00:00:00 2001 From: Boris Yankov Date: Mon, 13 Aug 2018 01:04:11 +0300 Subject: [PATCH 1/2] composebox: Unify two platform's code Remove the iOS code which was our old 'controlled' version and keep the new Android 'uncontrolled' version. While iOS does not have the extreme performance issues the Android app had because of bugs in React Native, it is still faster to not have the component controlled - a controlled compoennt would block any time the UI thread blocks. Another big benefit is removing the need to support two versions. Making the component uncontrolled fixes #2434 --- src/compose/ComposeBox.ios.js | 368 ------------------ .../{ComposeBox.android.js => ComposeBox.js} | 0 2 files changed, 368 deletions(-) delete mode 100644 src/compose/ComposeBox.ios.js rename src/compose/{ComposeBox.android.js => ComposeBox.js} (100%) diff --git a/src/compose/ComposeBox.ios.js b/src/compose/ComposeBox.ios.js deleted file mode 100644 index c71d7fe22ca..00000000000 --- a/src/compose/ComposeBox.ios.js +++ /dev/null @@ -1,368 +0,0 @@ -/* @flow */ -import React, { PureComponent } from 'react'; -import { View, TextInput, findNodeHandle } from 'react-native'; -import { connect } from 'react-redux'; -import TextInputReset from 'react-native-text-input-reset'; -import isEqual from 'lodash.isequal'; - -import type { - Auth, - Context, - Narrow, - EditMessage, - InputSelectionType, - User, - Dispatch, - Dimensions, - GlobalState, -} from '../types'; -import { - addToOutbox, - cancelEditMessage, - draftAdd, - draftRemove, - fetchTopicsForActiveStream, - sendTypingEvent, -} from '../actions'; -import { updateMessage } from '../api'; -import { FloatingActionButton, Input, MultilineInput } from '../common'; -import { showErrorAlert } from '../utils/info'; -import { IconDone, IconSend } from '../common/Icons'; -import { isStreamNarrow, isStreamOrTopicNarrow, topicNarrow } from '../utils/narrow'; -import ComposeMenu from './ComposeMenu'; -import AutocompleteViewWrapper from '../autocomplete/AutocompleteViewWrapper'; -import getComposeInputPlaceholder from './getComposeInputPlaceholder'; -import NotSubscribed from '../message/NotSubscribed'; -import AnnouncementOnly from '../message/AnnouncementOnly'; - -import { - getAuth, - getIsAdmin, - getSession, - canSendToActiveNarrow, - getLastMessageTopic, - getActiveUsers, - getShowMessagePlaceholders, -} from '../selectors'; -import { - getIsActiveStreamSubscribed, - getIsActiveStreamAnnouncementOnly, -} from '../subscriptions/subscriptionSelectors'; -import { getDraftForActiveNarrow } from '../drafts/draftsSelectors'; - -type Props = { - auth: Auth, - canSend: boolean, - narrow: Narrow, - users: User[], - draft: string, - lastMessageTopic: string, - isAdmin: boolean, - isAnnouncementOnly: boolean, - isSubscribed: boolean, - editMessage: EditMessage, - safeAreaInsets: Dimensions, - dispatch: Dispatch, - messageInputRef: (component: any) => void, -}; - -type State = { - isMessageFocused: boolean, - isTopicFocused: boolean, - isMenuExpanded: boolean, - topic: string, - message: string, - height: number, - selection: InputSelectionType, -}; - -class ComposeBox extends PureComponent { - context: Context; - props: Props; - state: State = { - isMessageFocused: false, - isTopicFocused: false, - isMenuExpanded: false, - height: 20, - topic: '', - message: this.props.draft, - selection: { start: 0, end: 0 }, - }; - - messageInput: TextInput = null; - topicInput: TextInput = null; - - static contextTypes = { - styles: () => null, - }; - - getCanSelectTopic = () => { - const { isMessageFocused, isTopicFocused } = this.state; - const { editMessage, narrow } = this.props; - if (editMessage) { - return isStreamOrTopicNarrow(narrow); - } - if (!isStreamNarrow(narrow)) { - return false; - } - return isMessageFocused || isTopicFocused; - }; - - handleComposeMenuToggle = () => { - this.setState(({ isMenuExpanded }) => ({ - isMenuExpanded: !isMenuExpanded, - })); - }; - - handleLayoutChange = (event: Object) => { - this.setState({ - height: event.nativeEvent.layout.height, - }); - }; - - handleTopicChange = (topic: string) => { - this.setState({ topic, isMenuExpanded: false }); - }; - - handleMessageChange = (message: string) => { - this.setState({ message, isMenuExpanded: false }); - const { dispatch, narrow } = this.props; - dispatch(sendTypingEvent(narrow)); - }; - - handleMessageSelectionChange = (event: Object) => { - const { selection } = event.nativeEvent; - this.setState({ selection }); - }; - - handleMessageFocus = () => { - const { topic } = this.state; - const { lastMessageTopic } = this.props; - this.setState({ - isMessageFocused: true, - isMenuExpanded: false, - }); - setTimeout(() => { - this.handleTopicChange(topic || lastMessageTopic); - }, 200); // wait, to hope the component is shown - }; - - handleMessageBlur = () => { - setTimeout(() => { - this.setState({ - isMessageFocused: false, - isMenuExpanded: false, - }); - }, 200); // give a chance to the topic input to get the focus - }; - - handleTopicFocus = () => { - const { dispatch, narrow } = this.props; - this.setState({ - isTopicFocused: true, - isMenuExpanded: false, - }); - dispatch(fetchTopicsForActiveStream(narrow)); - }; - - handleTopicBlur = () => { - setTimeout(() => { - this.setState({ - isTopicFocused: false, - isMenuExpanded: false, - }); - }, 200); // give a chance to the message input to get the focus - }; - - handleInputTouchStart = () => { - this.setState({ isMenuExpanded: false }); - }; - - clearMessageInput = () => { - if (this.messageInput) { - this.messageInput.clear(); - if (TextInputReset) { - TextInputReset.resetKeyboardInput(findNodeHandle(this.messageInput)); - } - } - - this.handleMessageChange(''); - }; - - handleSend = () => { - const { dispatch, narrow } = this.props; - const { topic, message } = this.state; - - const destinationNarrow = isStreamNarrow(narrow) - ? topicNarrow(narrow[0].operand, topic || '(no topic)') - : narrow; - - dispatch(addToOutbox(destinationNarrow, message)); - dispatch(draftRemove(narrow)); - - this.clearMessageInput(); - }; - - handleEdit = () => { - const { auth, editMessage, dispatch } = this.props; - const { message, topic } = this.state; - const content = editMessage.content !== message ? message : undefined; - const subject = topic !== editMessage.topic ? topic : undefined; - if (content || subject) { - updateMessage(auth, { content, subject }, editMessage.id).catch(error => { - showErrorAlert(error.message, 'Failed to edit message'); - }); - } - dispatch(cancelEditMessage()); - }; - - tryUpdateDraft = () => { - const { dispatch, draft, narrow } = this.props; - const { message } = this.state; - - if (draft.trim() === message.trim()) { - return; - } - - if (message.trim().length === 0) { - dispatch(draftRemove(narrow)); - } else { - dispatch(draftAdd(narrow, message)); - } - }; - - componentWillUnmount() { - this.tryUpdateDraft(); - } - - componentWillReceiveProps(nextProps: Props) { - if (nextProps.editMessage !== this.props.editMessage) { - const topic = - isStreamNarrow(nextProps.narrow) && nextProps.editMessage - ? nextProps.editMessage.topic - : ''; - this.handleMessageChange(nextProps.editMessage ? nextProps.editMessage.content : ''); - this.handleTopicChange(topic); - if (this.messageInput) { - this.messageInput.focus(); - } - } else if (!isEqual(nextProps.narrow, this.props.narrow)) { - this.tryUpdateDraft(); - - if (!nextProps.draft) { - this.clearMessageInput(); - } - - this.handleMessageChange(nextProps.draft); - } - } - - render() { - const { styles } = this.context; - const { isTopicFocused, isMenuExpanded, height, message, topic, selection } = this.state; - const { - auth, - canSend, - narrow, - users, - editMessage, - safeAreaInsets, - messageInputRef, - isAdmin, - isAnnouncementOnly, - isSubscribed, - } = this.props; - - if (!canSend) { - return null; - } - - if (!isSubscribed) { - return ; - } else if (isAnnouncementOnly && !isAdmin) { - return ; - } - - const placeholder = getComposeInputPlaceholder(narrow, auth.email, users); - - return ( - - - - - - - - {this.getCanSelectTopic() && ( - { - this.topicInput = component; - }} - onChangeText={this.handleTopicChange} - onFocus={this.handleTopicFocus} - onBlur={this.handleTopicBlur} - onTouchStart={this.handleInputTouchStart} - value={topic} - /> - )} - { - if (component) { - this.messageInput = component; - messageInputRef(component); - } - }} - value={message} - onBlur={this.handleMessageBlur} - onChange={this.handleMessageChange} - onFocus={this.handleMessageFocus} - onSelectionChange={this.handleMessageSelectionChange} - onTouchStart={this.handleInputTouchStart} - /> - - - - - - - ); - } -} - -export default connect((state: GlobalState, props) => ({ - auth: getAuth(state), - users: getActiveUsers(state), - safeAreaInsets: getSession(state).safeAreaInsets, - isAdmin: getIsAdmin(state), - isAnnouncementOnly: getIsActiveStreamAnnouncementOnly(props.narrow)(state), - isSubscribed: getIsActiveStreamSubscribed(props.narrow)(state), - canSend: canSendToActiveNarrow(props.narrow) && !getShowMessagePlaceholders(props.narrow)(state), - editMessage: getSession(state).editMessage, - draft: getDraftForActiveNarrow(props.narrow)(state), - lastMessageTopic: getLastMessageTopic(props.narrow)(state), -}))(ComposeBox); diff --git a/src/compose/ComposeBox.android.js b/src/compose/ComposeBox.js similarity index 100% rename from src/compose/ComposeBox.android.js rename to src/compose/ComposeBox.js From 5742d576b9df18bf6abcb8979818b7bbbaf75d75 Mon Sep 17 00:00:00 2001 From: Boris Yankov Date: Mon, 13 Aug 2018 23:03:51 +0300 Subject: [PATCH 2/2] composebox: Improve text resetting code, circumvent iOS bug Fixes: #2434 There is a simple hack/fix for the iOS-specific bug that was a blocker for us migrating the iOS version. If we set the text via `setNativeProps` twice in a row, it will indeed reset the contents, so the first time we use a space `' '` that looks empty but is different than the second time when we set it to an actually empty string '``'. We use `setTimeout` in order to give React Native a chance to update the native value before running the second time. For Android we don't need to call `setNativeProps` as the `TextInputReset.resetKeyboardInput` call does that. Also compared to the previous version we don't need to check if `TextInputReset` is defined as it isn't only for iOS and we `if` for that. --- src/compose/ComposeBox.js | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/compose/ComposeBox.js b/src/compose/ComposeBox.js index 63f15b1e9ea..e51d874aed7 100644 --- a/src/compose/ComposeBox.js +++ b/src/compose/ComposeBox.js @@ -1,6 +1,6 @@ /* @flow */ import React, { PureComponent } from 'react'; -import { View, TextInput, findNodeHandle } from 'react-native'; +import { View, Platform, TextInput, findNodeHandle } from 'react-native'; import { connect } from 'react-redux'; import TextInputReset from 'react-native-text-input-reset'; import isEqual from 'lodash.isequal'; @@ -76,6 +76,20 @@ type State = { selection: InputSelectionType, }; +const clearTextInput = (textInput: TextInput): void => { + if (Platform.OS === 'ios') { + // Calling `setNativeProps` twice works around the currently present iOS bug + // We need to call it with different values, the ' ' looks like no text + textInput.setNativeProps({ text: ' ' }); + setTimeout(() => { + textInput.setNativeProps({ text: '' }); + }, 0); + } else { + // Force reset to fix an issue with some Android custom keyboards + TextInputReset.resetKeyboardInput(findNodeHandle(textInput)); + } +}; + export const updateTextInput = (textInput: TextInput, text: string): void => { if (!textInput) { // Depending on the lifecycle events this function is called from, @@ -83,13 +97,14 @@ export const updateTextInput = (textInput: TextInput, text: string): void => { return; } - textInput.setNativeProps({ text }); - - if (text.length === 0 && TextInputReset) { - // React Native has problems with some custom keyboards when clearing - // the input's contents. Force reset to make sure it works. - TextInputReset.resetKeyboardInput(findNodeHandle(textInput)); + // Both iOS and Android have bugs related to clearing Input's contents + if (text.length === 0) { + clearTextInput(textInput); } + + setTimeout(() => { + textInput.setNativeProps({ text }); + }, 0); }; class ComposeBox extends PureComponent {