Skip to content

Fix mWeb Compose Focus bug #14230

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.

Sign up for GitHub

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

Merged
bondydaa merged 12 commits into from
Jan 31, 2023
Merged

Fix mWeb Compose Focus bug #14230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9360d83
add log line to help confirm if this code is ever run
bondydaa Jan 10, 2023
4ea07c9
hot fix a bug so that mWeb beahaves like Web in regard to giving focu…
bondydaa Jan 12, 2023
64b83e0
remove unneeded log line, add comment
bondydaa Jan 12, 2023
6bb07d6
Merge branch 'main' of github.com:Expensify/App into bondy-fix-mweb-f…
bondydaa Jan 12, 2023
4884bdc
re-add this logic so we dont cause regression on sign in pages
bondydaa Jan 12, 2023
9f33b93
set variable based on platform type so we know if we need to refocus …
bondydaa Jan 12, 2023
638a35c
remove unused import now
bondydaa Jan 12, 2023
80ce3c3
remove unused import
bondydaa Jan 12, 2023
57528b4
use separate native files to determine booleans, fix naming convencti…
bondydaa Jan 13, 2023
0e1d944
remove unused import
bondydaa Jan 13, 2023
f479419
Merge branch 'main' of github.com:Expensify/App into bondy-fix-mweb-f…
bondydaa Jan 19, 2023
89722bf
rename variable and files, thanks chatgpt
bondydaa Jan 19, 2023
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
1 change: 1 addition & 0 deletions src/libs/willBlurTextInputOnTapOutside/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => true;
1 change: 1 addition & 0 deletions src/libs/willBlurTextInputOnTapOutside/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => false;
18 changes: 11 additions & 7 deletions src/pages/home/report/ReportActionCompose.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import ReportTypingIndicator from './ReportTypingIndicator';
import AttachmentModal from '../../../components/AttachmentModal';
import compose from '../../../libs/compose';
import PopoverMenu from '../../../components/PopoverMenu';
import willBlurTextInputOnTapOutside from '../../../libs/willBlurTextInputOnTapOutside';
import CONST from '../../../CONST';
import canFocusInputOnScreenFocus from '../../../libs/canFocusInputOnScreenFocus';
import Permissions from '../../../libs/Permissions';
import Navigation from '../../../libs/Navigation/Navigation';
import ROUTES from '../../../ROUTES';
Expand Down Expand Up @@ -134,12 +134,14 @@ class ReportActionCompose extends React.Component {
this.getInputPlaceholder = this.getInputPlaceholder.bind(this);
this.getIOUOptions = this.getIOUOptions.bind(this);
this.addAttachment = this.addAttachment.bind(this);

this.comment = props.comment;
this.shouldFocusInputOnScreenFocus = canFocusInputOnScreenFocus();

// React Native will retain focus on an input for native devices but web/mWeb behave differently so we have some focus management
// code that will refocus the compose input after a user closes a modal or some other actions, see usage of ReportActionComposeFocusManager
this.willBlurTextInputOnTapOutside = willBlurTextInputOnTapOutside();

this.state = {
isFocused: this.shouldFocusInputOnScreenFocus && !this.props.modal.isVisible && !this.props.modal.willAlertModalBecomeVisible,
isFocused: this.willBlurTextInputOnTapOutside && !this.props.modal.isVisible && !this.props.modal.willAlertModalBecomeVisible,
isFullComposerAvailable: props.isComposerFullSize,
textInputShouldClear: false,
isCommentEmpty: props.comment.length === 0,
Expand All @@ -157,8 +159,10 @@ class ReportActionCompose extends React.Component {
}

componentDidMount() {
// This callback is used in the contextMenuActions to manage giving focus back to the compose input.
// TODO: we should clean up this convoluted code and instead move focus management to something like ReportFooter.js or another higher up component
ReportActionComposeFocusManager.onComposerFocus(() => {
if (!this.shouldFocusInputOnScreenFocus || !this.props.isFocused) {
if (!this.willBlurTextInputOnTapOutside || !this.props.isFocused) {
return;
}

Expand All @@ -177,7 +181,7 @@ class ReportActionCompose extends React.Component {
// We want to focus or refocus the input when a modal has been closed and the underlying screen is focused.
// We avoid doing this on native platforms since the software keyboard popping
// open creates a jarring and broken UX.
if (this.shouldFocusInputOnScreenFocus && this.props.isFocused
if (this.willBlurTextInputOnTapOutside && this.props.isFocused
&& prevProps.modal.isVisible && !this.props.modal.isVisible) {
this.focus();
}
Expand Down Expand Up @@ -661,7 +665,7 @@ class ReportActionCompose extends React.Component {
disabled={this.props.disabled}
>
<Composer
autoFocus={!this.props.modal.isVisible && (this.shouldFocusInputOnScreenFocus || this.isEmptyChat())}
autoFocus={!this.props.modal.isVisible && (this.willBlurTextInputOnTapOutside || this.isEmptyChat())}

@0xmiroslav 0xmiroslav May 25, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Coming from #17187:
This was overlooked while replacing all shouldFocusInputOnScreenFocus occurrences. Actually no changes needed here. Native and mWeb should be consistent in autofocus behavior.

multiline
ref={this.setTextInputRef}
textAlignVertical="top"
Expand Down