From ba7e56bff03c104736ea67ddce4499831623838d Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Fri, 19 May 2023 22:55:52 +0800 Subject: [PATCH 1/5] Fix the blue frame caused by selection prop --- src/components/Composer/index.js | 39 ++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/components/Composer/index.js b/src/components/Composer/index.js index 0e198f03d036..91bbedd27ece 100755 --- a/src/components/Composer/index.js +++ b/src/components/Composer/index.js @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import _ from 'underscore'; import ExpensiMark from 'expensify-common/lib/ExpensiMark'; import Str from 'expensify-common/lib/str'; +import {withOnyx} from 'react-native-onyx'; import RNTextInput from '../RNTextInput'; import withLocalize, {withLocalizePropTypes} from '../withLocalize'; import Growl from '../../libs/Growl'; @@ -16,6 +17,7 @@ import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDime import compose from '../../libs/compose'; import styles from '../../styles/styles'; import isEnterWhileComposition from '../../libs/KeyboardShortcut/isEnterWhileComposition'; +import ONYXKEYS from '../../ONYXKEYS'; const propTypes = { /** Maximum number of lines in the text input */ @@ -74,6 +76,12 @@ const propTypes = { /** Whether the composer is full size */ isComposerFullSize: PropTypes.bool, + /** Details about any modals being used */ + modal: PropTypes.shape({ + /** Indicates when an Alert modal is about to be visible */ + willAlertModalBecomeVisible: PropTypes.bool, + }), + ...withLocalizePropTypes, ...windowDimensionsPropTypes, @@ -93,6 +101,7 @@ const defaultProps = { autoFocus: false, forwardedRef: null, onSelectionChange: () => {}, + modal: {}, selection: { start: 0, end: 0, @@ -367,16 +376,32 @@ class Composer extends React.Component { render() { const propStyles = StyleSheet.flatten(this.props.style); propStyles.outline = 'none'; - const propsWithoutStyles = _.omit(this.props, 'style'); + const propsWithoutStyles = _.omit(this.props, 'style', 'selection'); // We're disabling autoCorrect for iOS Safari until Safari fixes this issue. See https://github.com/Expensify/App/issues/8592 + const isMobileSafari = Browser.isMobileSafari(); + + const start = this.textInput ? this.textInput.selectionStart : 0; + const end = this.textInput ? this.textInput.selectionEnd : 0; + const selection = this.props.modal.willAlertModalBecomeVisible ? {start, end} : this.props.selection; return ( (this.textInput = el)} - selection={this.state.selection} + ref={(el) => { + this.textInput = el; + if ( + !el || + el.isFocused() || + (el.selectionStart === this.props.selection.start && el.selectionEnd === this.props.selection.end) || + this.props.modal.willAlertModalBecomeVisible || + !isMobileSafari + ) { + return; + } + el.focus(); + }} onChange={this.shouldCallUpdateNumberOfLines} onSelectionChange={this.onSelectionChange} style={[ @@ -388,6 +413,7 @@ class Composer extends React.Component { ]} /* eslint-disable-next-line react/jsx-props-no-spreading */ {...propsWithoutStyles} + selection={selection} numberOfLines={this.state.numberOfLines} disabled={this.props.isDisabled} onKeyPress={this.handleKeyPress} @@ -402,6 +428,11 @@ Composer.defaultProps = defaultProps; export default compose( withLocalize, withWindowDimensions, + withOnyx({ + modal: { + key: ONYXKEYS.MODAL, + }, + }), )( React.forwardRef((props, ref) => ( Date: Sun, 21 May 2023 10:56:06 +0800 Subject: [PATCH 2/5] make sure selection is updated after focus --- src/components/Composer/index.js | 41 +++---------------- src/pages/home/report/ReportActionCompose.js | 14 +++++-- .../report/ReportActionItemMessageEdit.js | 15 ++++++- 3 files changed, 29 insertions(+), 41 deletions(-) diff --git a/src/components/Composer/index.js b/src/components/Composer/index.js index 91bbedd27ece..ca7435341902 100755 --- a/src/components/Composer/index.js +++ b/src/components/Composer/index.js @@ -4,7 +4,6 @@ import PropTypes from 'prop-types'; import _ from 'underscore'; import ExpensiMark from 'expensify-common/lib/ExpensiMark'; import Str from 'expensify-common/lib/str'; -import {withOnyx} from 'react-native-onyx'; import RNTextInput from '../RNTextInput'; import withLocalize, {withLocalizePropTypes} from '../withLocalize'; import Growl from '../../libs/Growl'; @@ -17,7 +16,6 @@ import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDime import compose from '../../libs/compose'; import styles from '../../styles/styles'; import isEnterWhileComposition from '../../libs/KeyboardShortcut/isEnterWhileComposition'; -import ONYXKEYS from '../../ONYXKEYS'; const propTypes = { /** Maximum number of lines in the text input */ @@ -55,7 +53,7 @@ const propTypes = { isDisabled: PropTypes.bool, /** 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 */ + 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, /** Update selection position on change */ @@ -76,12 +74,6 @@ const propTypes = { /** Whether the composer is full size */ isComposerFullSize: PropTypes.bool, - /** Details about any modals being used */ - modal: PropTypes.shape({ - /** Indicates when an Alert modal is about to be visible */ - willAlertModalBecomeVisible: PropTypes.bool, - }), - ...withLocalizePropTypes, ...windowDimensionsPropTypes, @@ -101,7 +93,6 @@ const defaultProps = { autoFocus: false, forwardedRef: null, onSelectionChange: () => {}, - modal: {}, selection: { start: 0, end: 0, @@ -376,32 +367,16 @@ class Composer extends React.Component { render() { const propStyles = StyleSheet.flatten(this.props.style); propStyles.outline = 'none'; - const propsWithoutStyles = _.omit(this.props, 'style', 'selection'); + const propsWithoutStyles = _.omit(this.props, 'style'); // We're disabling autoCorrect for iOS Safari until Safari fixes this issue. See https://github.com/Expensify/App/issues/8592 - const isMobileSafari = Browser.isMobileSafari(); - - const start = this.textInput ? this.textInput.selectionStart : 0; - const end = this.textInput ? this.textInput.selectionEnd : 0; - const selection = this.props.modal.willAlertModalBecomeVisible ? {start, end} : this.props.selection; return ( { - this.textInput = el; - if ( - !el || - el.isFocused() || - (el.selectionStart === this.props.selection.start && el.selectionEnd === this.props.selection.end) || - this.props.modal.willAlertModalBecomeVisible || - !isMobileSafari - ) { - return; - } - el.focus(); - }} + ref={(el) => (this.textInput = el)} + selection={this.state.selection} onChange={this.shouldCallUpdateNumberOfLines} onSelectionChange={this.onSelectionChange} style={[ @@ -413,7 +388,6 @@ class Composer extends React.Component { ]} /* eslint-disable-next-line react/jsx-props-no-spreading */ {...propsWithoutStyles} - selection={selection} numberOfLines={this.state.numberOfLines} disabled={this.props.isDisabled} onKeyPress={this.handleKeyPress} @@ -428,11 +402,6 @@ Composer.defaultProps = defaultProps; export default compose( withLocalize, withWindowDimensions, - withOnyx({ - modal: { - key: ONYXKEYS.MODAL, - }, - }), )( React.forwardRef((props, ref) => ( { + return { + selection: { + start: prevState.draft.length, + end: prevState.draft.length, + }, + }; + }); + } + componentWillUnmount() { // Skip if this is not the focused message so the other edit composer stays focused. if (!this.state.isFocused) { From 8ff517ca77a455269769dbdb723652f01bd9d7e2 Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Sun, 21 May 2023 10:58:43 +0800 Subject: [PATCH 3/5] typo --- src/components/Composer/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Composer/index.js b/src/components/Composer/index.js index ca7435341902..0e198f03d036 100755 --- a/src/components/Composer/index.js +++ b/src/components/Composer/index.js @@ -53,7 +53,7 @@ const propTypes = { isDisabled: PropTypes.bool, /** 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 */ + 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, /** Update selection position on change */ From 0779d23bc3a82e22b661e27988eb91764d6aeada Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Sun, 21 May 2023 11:10:11 +0800 Subject: [PATCH 4/5] fix lint error --- .../home/report/ReportActionItemMessageEdit.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/pages/home/report/ReportActionItemMessageEdit.js b/src/pages/home/report/ReportActionItemMessageEdit.js index f7a745c5606c..7deed147aa3f 100644 --- a/src/pages/home/report/ReportActionItemMessageEdit.js +++ b/src/pages/home/report/ReportActionItemMessageEdit.js @@ -110,14 +110,12 @@ class ReportActionItemMessageEdit extends React.Component { } componentDidMount() { - this.setState((prevState) => { - return { - selection: { - start: prevState.draft.length, - end: prevState.draft.length, - }, - }; - }); + this.setState((prevState) => ({ + selection: { + start: prevState.draft.length, + end: prevState.draft.length, + }, + })); } componentWillUnmount() { From 3e9e7ad40255bc942ce60d863373ff8bea9dfe1a Mon Sep 17 00:00:00 2001 From: ntdiary <2471314@gmail.com> Date: Wed, 24 May 2023 06:36:08 +0800 Subject: [PATCH 5/5] add a comment --- src/pages/home/report/ReportActionItemMessageEdit.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pages/home/report/ReportActionItemMessageEdit.js b/src/pages/home/report/ReportActionItemMessageEdit.js index f246c3210d3a..56ccfbe81ca2 100644 --- a/src/pages/home/report/ReportActionItemMessageEdit.js +++ b/src/pages/home/report/ReportActionItemMessageEdit.js @@ -111,6 +111,9 @@ class ReportActionItemMessageEdit extends React.Component { } componentDidMount() { + // For mobile Safari, updating the selection prop on an unfocused input will cause it to automatically gain focus + // and subsequent programmatic focus shifts (e.g., modal focus trap) to show the blue frame (:focus-visible style), + // so we need to ensure that it is only updated after focus. this.setState((prevState) => ({ selection: { start: prevState.draft.length,