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.

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

[IS-3186] Fixed cursor position issue on Android #3374

Merged
merged 6 commits into from
Jun 8, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
79 changes: 79 additions & 0 deletions src/components/TextInputFocusable/index.android.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import React from 'react';
import {TextInput} from 'react-native';
import PropTypes from 'prop-types';
import _ from 'underscore';
import themeColors from '../../styles/themes/default';

/**
* On native layers we like to have the Text Input not focused so the user can read new chats without they keyboard in
* the way of the view
*/

const propTypes = {
/** If the input should clear, it actually gets intercepted instead of .clear() */
shouldClear: PropTypes.bool,

/** A ref to forward to the text input */
forwardedRef: PropTypes.func,

/** When the input has cleared whoever owns this input should know about it */
onClear: PropTypes.func,

/** Set focus to this component the first time it renders.
* Override this in case you need to set focus on one field out of many, or when you want to disable autoFocus */
autoFocus: PropTypes.bool,

/** Prevent edits and interactions like focus for this input. */
isDisabled: PropTypes.bool,

};

const defaultProps = {
shouldClear: false,
onClear: () => {},
autoFocus: false,
isDisabled: false,
forwardedRef: null,
};

class TextInputFocusable extends React.Component {
componentDidMount() {
// This callback prop is used by the parent component using the constructor to
// get a ref to the inner textInput element e.g. if we do
// <constructor ref={el => this.textInput = el} /> this will not
// return a ref to the component, but rather the HTML element by default
if (this.props.forwardedRef && _.isFunction(this.props.forwardedRef)) {
this.props.forwardedRef(this.textInput);
}
}

componentDidUpdate(prevProps) {
if (!prevProps.shouldClear && this.props.shouldClear) {
this.textInput.clear();
this.props.onClear();
}
}

render() {
return (
<TextInput
placeholderTextColor={themeColors.placeholderText}
ref={el => this.textInput = el}
maxHeight={116}
rejectResponderTermination={false}
editable={!this.props.isDisabled}
/* eslint-disable-next-line react/jsx-props-no-spreading */
{...this.props}
/>
);
}
}

TextInputFocusable.displayName = 'TextInputFocusable';
TextInputFocusable.propTypes = propTypes;
TextInputFocusable.defaultProps = defaultProps;

export default React.forwardRef((props, ref) => (
/* eslint-disable-next-line react/jsx-props-no-spreading */
<TextInputFocusable {...props} forwardedRef={ref} />
));
1 change: 1 addition & 0 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ class ReportActionCompose extends React.Component {
* @param {String} newComment
*/
updateComment(newComment) {
this.textInput.setNativeProps({text: newComment});
Copy link
Member

Choose a reason for hiding this comment

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

@aliabbasmalik8 Sorry you ping you this way but I just want to know why did you add this? I am doing some changes to the component and I just want to know was there an issue, that required this. It's pretty urgent we have a couple of deploy blockers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

t's used to update comments ASAP because in android there is some issue with the selection position when we update without this.
What issue you are facing?

Copy link
Member

Choose a reason for hiding this comment

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

There are multiple things I am working on so I was trying to understand the old code. There is an issue when the cursor jumps. There is an issue when the app crashes......so on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, this code working perfectly and tested as well. you can mention PR here on which you are working so can help.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I could be wrong, but this does not seem like a correct usage of setNativeProps()... seems like there are various ways we clear the text input and I'm having a really hard time understanding why... please leave more context in the code in the future.

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 can use some other ways as well but you can see in some other issue conversation reviewer like the way of direct manipulation because uncontrolled input used in the app.

this.setState({
isCommentEmpty: newComment.length === 0,
});
Expand Down