-
Notifications
You must be signed in to change notification settings - Fork 3k
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
16098 — selecting chat message and pressing any key focus the message instead of switch the focus to the composer #21583
Merged
PauloGasparSv
merged 21 commits into
Expensify:main
from
robertKozik:16098-selecting-chat-message-and-pressing-any-key-focus-the-message-instead-of-switch-the-focus-to-the-composer
Jul 12, 2023
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
82fbeb0
fix parseReportRouteParams so it can be used with getActiveRoute func…
robertKozik 81695ef
create KeyDownPress listner lib
robertKozik 4764004
create isEmojiPickerVisible function
robertKozik 0a5873c
add keyDown and paste listners docuemnt-scope in order to focus composer
robertKozik 32e021f
Merge branch 'main' into 16098-selecting-chat-message-and-pressing-an…
robertKozik 6eea6aa
manage listeners on focus/blur basis
robertKozik 28240d8
prettier
robertKozik 8270068
remove unused imports
robertKozik 8e453c7
Merge remote-tracking branch 'upstream/main' into 16098-selecting-cha…
robertKozik 8ae5a56
add emojiPicker visiblity state to imperative handle
robertKozik e979d2b
conditionally add trailing space in the replaceSelectionWithText method
robertKozik 42e6bc8
Merge branch 'main' into 16098-selecting-chat-message-and-pressing-an…
robertKozik 3d12ed7
refactor navigation unsubscribers names
robertKozik 99edfa1
rephrase comment to me more precise
robertKozik 889dd18
move FAB ref to separate file, create function to check create menu v…
robertKozik eb26193
Consider FAB popover menu status while checking composer visibility
robertKozik 834fb72
prettier
robertKozik 1f4bf0b
remove unused useRef import
robertKozik 2eac511
move FloatingActionButtonAndPopoverUtils to libs directory
robertKozik 8b49b86
detect popover status based on modal isVisible onyx state
robertKozik 93654fc
Merge branch 'main' into 16098-selecting-chat-message-and-pressing-an…
robertKozik File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
function addKeyDownPressListner(callbackFunction) { | ||
document.addEventListener('keydown', callbackFunction); | ||
} | ||
|
||
function removeKeyDownPressListner(callbackFunction) { | ||
document.removeEventListener('keydown', callbackFunction); | ||
} | ||
|
||
export {addKeyDownPressListner, removeKeyDownPressListner}; |
4 changes: 4 additions & 0 deletions
4
src/libs/KeyboardShortcut/KeyDownPressListener/index.native.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
function addKeyDownPressListner() {} | ||
function removeKeyDownPressListner() {} | ||
|
||
export {addKeyDownPressListner, removeKeyDownPressListner}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,8 @@ import * as Task from '../../../libs/actions/Task'; | |
import * as Browser from '../../../libs/Browser'; | ||
import * as IOU from '../../../libs/actions/IOU'; | ||
import PressableWithFeedback from '../../../components/Pressable/PressableWithFeedback'; | ||
import * as KeyDownListener from '../../../libs/KeyboardShortcut/KeyDownPressListener'; | ||
import * as EmojiPickerActions from '../../../libs/actions/EmojiPickerAction'; | ||
|
||
const propTypes = { | ||
/** Beta features list */ | ||
|
@@ -168,7 +170,9 @@ class ReportActionCompose extends React.Component { | |
this.setIsFocused = this.setIsFocused.bind(this); | ||
this.setIsFullComposerAvailable = this.setIsFullComposerAvailable.bind(this); | ||
this.focus = this.focus.bind(this); | ||
this.addEmojiToTextBox = this.addEmojiToTextBox.bind(this); | ||
this.replaceSelectionWithText = this.replaceSelectionWithText.bind(this); | ||
this.focusComposerOnKeyPress = this.focusComposerOnKeyPress.bind(this); | ||
this.checkComposerVisibility = this.checkComposerVisibility.bind(this); | ||
this.onSelectionChange = this.onSelectionChange.bind(this); | ||
this.isEmojiCode = this.isEmojiCode.bind(this); | ||
this.isMentionCode = this.isMentionCode.bind(this); | ||
|
@@ -186,6 +190,8 @@ class ReportActionCompose extends React.Component { | |
this.comment = props.comment; | ||
this.insertedEmojis = []; | ||
|
||
this.attachmentModalRef = React.createRef(); | ||
|
||
// 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(); | ||
|
@@ -205,6 +211,9 @@ class ReportActionCompose extends React.Component { | |
// so we need to ensure that it is only updated after focus. | ||
const isMobileSafari = Browser.isMobileSafari(); | ||
|
||
this.unsubscribeNavigationBlur = () => null; | ||
this.unsubscribeNavigationFocus = () => null; | ||
|
||
this.state = { | ||
isFocused: this.shouldFocusInputOnScreenFocus && !this.props.modal.isVisible && !this.props.modal.willAlertModalBecomeVisible && this.props.shouldShowComposeInput, | ||
isFullComposerAvailable: props.isComposerFullSize, | ||
|
@@ -238,6 +247,10 @@ class ReportActionCompose extends React.Component { | |
this.focus(false); | ||
}); | ||
|
||
this.unsubscribeNavigationBlur = this.props.navigation.addListener('blur', () => KeyDownListener.removeKeyDownPressListner(this.focusComposerOnKeyPress)); | ||
this.unsubscribeNavigationFocus = this.props.navigation.addListener('focus', () => KeyDownListener.addKeyDownPressListner(this.focusComposerOnKeyPress)); | ||
KeyDownListener.addKeyDownPressListner(this.focusComposerOnKeyPress); | ||
|
||
this.updateComment(this.comment); | ||
|
||
// Shows Popover Menu on Workspace Chat at first sign-in | ||
|
@@ -276,6 +289,10 @@ class ReportActionCompose extends React.Component { | |
|
||
componentWillUnmount() { | ||
ReportActionComposeFocusManager.clear(); | ||
|
||
KeyDownListener.removeKeyDownPressListner(this.focusComposerOnKeyPress); | ||
this.unsubscribeNavigationBlur(); | ||
this.unsubscribeNavigationFocus(); | ||
} | ||
|
||
onSelectionChange(e) { | ||
|
@@ -664,20 +681,56 @@ class ReportActionCompose extends React.Component { | |
} | ||
|
||
/** | ||
* Callback for the emoji picker to add whatever emoji is chosen into the main input | ||
* | ||
* @param {String} emoji | ||
* Callback to add whatever text is chosen into the main input (used f.e as callback for the emoji picker) | ||
* @param {String} text | ||
* @param {Boolean} shouldAddTrailSpace | ||
*/ | ||
addEmojiToTextBox(emoji) { | ||
this.updateComment(ComposerUtils.insertText(this.comment, this.state.selection, `${emoji} `)); | ||
replaceSelectionWithText(text, shouldAddTrailSpace = true) { | ||
const updatedText = shouldAddTrailSpace ? `${text} ` : text; | ||
const selectionSpaceLength = shouldAddTrailSpace ? CONST.SPACE_LENGTH : 0; | ||
this.updateComment(ComposerUtils.insertText(this.comment, this.state.selection, updatedText)); | ||
this.setState((prevState) => ({ | ||
selection: { | ||
start: prevState.selection.start + emoji.length + CONST.SPACE_LENGTH, | ||
end: prevState.selection.start + emoji.length + CONST.SPACE_LENGTH, | ||
start: prevState.selection.start + text.length + selectionSpaceLength, | ||
end: prevState.selection.start + text.length + selectionSpaceLength, | ||
}, | ||
})); | ||
} | ||
|
||
/** | ||
* Check if the composer is visible. Returns true if the composer is not covered up by emoji picker or menu. False otherwise. | ||
* @returns {Boolean} | ||
*/ | ||
checkComposerVisibility() { | ||
const isComposerCoveredUp = EmojiPickerActions.isEmojiPickerVisible() || this.state.isMenuVisible || this.props.modal.isVisible; | ||
return !isComposerCoveredUp; | ||
} | ||
|
||
focusComposerOnKeyPress(e) { | ||
const isComposerVisible = this.checkComposerVisibility(); | ||
if (!isComposerVisible) { | ||
return; | ||
} | ||
|
||
// If the key pressed is non-character keys like Enter, Shift, ... do not focus | ||
if (e.key.length > 1) { | ||
return; | ||
} | ||
|
||
// If a key is pressed in combination with Meta, Control or Alt do not focus | ||
if (e.metaKey || e.ctrlKey || e.altKey) { | ||
return; | ||
} | ||
|
||
// if we're typing on another input/text area, do not focus | ||
if (['INPUT', 'TEXTAREA'].includes(e.target.nodeName)) { | ||
return; | ||
} | ||
|
||
this.focus(); | ||
this.replaceSelectionWithText(e.key, false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This call was redundant. Later it caused #33710. |
||
} | ||
|
||
/** | ||
* Focus the composer text input | ||
* @param {Boolean} [shouldelay=false] Impose delay before focusing the composer | ||
|
@@ -1084,6 +1137,7 @@ class ReportActionCompose extends React.Component { | |
disabled={this.props.disabled} | ||
> | ||
<Composer | ||
checkComposerVisibility={() => this.checkComposerVisibility()} | ||
autoFocus={this.shouldAutoFocus} | ||
multiline | ||
ref={this.setTextInputRef} | ||
|
@@ -1136,7 +1190,7 @@ class ReportActionCompose extends React.Component { | |
onModalHide={() => { | ||
this.focus(true); | ||
}} | ||
onEmojiSelected={this.addEmojiToTextBox} | ||
onEmojiSelected={this.replaceSelectionWithText} | ||
/> | ||
)} | ||
<View | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We forgot to consider the
space
as the key which also shouldn't focus the composer. This lead to issue #23508