Skip to content

Commit

Permalink
refactor: ReportActionCompose stop relying on global modal state to p…
Browse files Browse the repository at this point in the history
…revent bugs

Unwanted side effects coming from `componentDidUpdate` and global `modal.isVisible`

Details: #1913 (comment)
  • Loading branch information
kidroca committed Mar 27, 2021
1 parent 382c1fc commit 0395526
Showing 1 changed file with 12 additions and 25 deletions.
37 changes: 12 additions & 25 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,6 @@ const propTypes = {
// The ID of the report actions will be created for
reportID: PropTypes.number.isRequired,

// Details about any modals being used
modal: PropTypes.shape({
// Indicates if there is a modal currently visible or not
isVisible: PropTypes.bool,
}),

// The report currently being looked at
report: PropTypes.shape({

Expand All @@ -48,7 +42,6 @@ const propTypes = {

const defaultProps = {
comment: '',
modal: {},
};

class ReportActionCompose extends React.Component {
Expand Down Expand Up @@ -78,11 +71,8 @@ class ReportActionCompose extends React.Component {
this.comment = this.props.comment;
}

// When any modal goes from visible to hidden or when the report ID changes, bring focus to the compose field
if (
(prevProps.modal.isVisible && !this.props.modal.isVisible)
|| (prevProps.reportID !== this.props.reportID)
) {
// When the report ID changes bring focus to the compose field
if (prevProps.reportID !== this.props.reportID) {
this.setIsFocused(true);
}
}
Expand All @@ -93,10 +83,11 @@ class ReportActionCompose extends React.Component {
* @param {Boolean} shouldHighlight
*/
setIsFocused(shouldHighlight) {
this.setState({isFocused: shouldHighlight});
if (shouldHighlight && this.textInput) {
if (shouldHighlight && this.textInput && !this.state.isFocused) {
this.textInput.focus();
}

this.setState({isFocused: shouldHighlight});
}

/**
Expand All @@ -109,7 +100,7 @@ class ReportActionCompose extends React.Component {
}

/**
* Updates the visiblity state of the menu
* Updates the visibility state of the menu
*
* @param {Boolean} isMenuVisible
*/
Expand Down Expand Up @@ -208,6 +199,7 @@ class ReportActionCompose extends React.Component {
addAction(this.props.reportID, '', file);
this.setTextInputShouldClear(false);
}}
onModalHide={() => this.setIsFocused(true)}
>
{({displayFileInModal}) => (
<>
Expand All @@ -228,13 +220,11 @@ class ReportActionCompose extends React.Component {
isVisible={this.state.isMenuVisible}
onClose={() => this.setMenuVisibility(false)}
onAttachmentPickerSelected={() => {
setTimeout(() => {
openPicker({
onPicked: (file) => {
displayFileInModal({file});
},
});
}, 10);
openPicker({
onPicked: (file) => {
if (file) { displayFileInModal({file}); }
},
});
}}
onItemSelected={() => this.setMenuVisibility(false)}
menuOptions={[CONST.MENU_ITEM_KEYS.ATTACHMENT_PICKER]}
Expand Down Expand Up @@ -314,9 +304,6 @@ export default compose(
comment: {
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`,
},
modal: {
key: ONYXKEYS.MODAL,
},
report: {
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
},
Expand Down

0 comments on commit 0395526

Please sign in to comment.