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

Fix: Composer not focused on click while editor's emoji modal is open #28238

Merged
merged 35 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
0f02054
fix: composer not focused on click
tienifr Sep 26, 2023
86d30a4
add focus check
tienifr Sep 26, 2023
27f6a05
reuse existing functions
tienifr Sep 26, 2023
24d1fa9
clear FocusManager on unmount
tienifr Sep 26, 2023
d262d65
Merge branch 'main' of https://github.com/tienifr/App into fix/25892
tienifr Sep 29, 2023
f9a54af
Merge branch 'main' of https://github.com/tienifr/App into fix/25892
tienifr Oct 2, 2023
83f7745
Merge branch 'main' of https://github.com/tienifr/App into fix/25892
tienifr Oct 4, 2023
801066c
focus main composer
tienifr Oct 4, 2023
0472812
re-focus the right composer on emoji modal hide
tienifr Oct 4, 2023
e7c2fb5
Merge branch 'main' of https://github.com/tienifr/App into fix/25892
tienifr Oct 4, 2023
b2dede8
refocus composer on context menu close
tienifr Oct 4, 2023
c6b17d5
Merge branch 'main' of https://github.com/tienifr/App into fix/25892
tienifr Oct 4, 2023
3f4e9d7
remove willBlurOnTapOutside check to re-focus the correct composer
tienifr Oct 4, 2023
fb99586
Merge branch 'main' of https://github.com/tienifr/App into fix/25892
tienifr Oct 4, 2023
6bcd089
clear focus callback on unmount
tienifr Oct 6, 2023
b1025c1
add comment
tienifr Oct 6, 2023
3439d49
add comment for removing callback
tienifr Oct 6, 2023
f100c71
Merge branch 'main' of https://github.com/tienifr/App into fix/25892
tienifr Oct 11, 2023
f51ed18
Merge branch 'main' of https://github.com/tienifr/App into fix/25892
tienifr Oct 11, 2023
fdfec45
Merge branch 'main' into fix/25892
tienifr Oct 12, 2023
97338fc
Merge branch 'main' into fix/25892
tienifr Dec 8, 2023
759b4a5
implement callback priority approach
tienifr Dec 8, 2023
a20eace
modify comment
tienifr Dec 8, 2023
bb9f370
Merge branch main into fix/25892
tienifr Dec 20, 2023
66ce543
fix lint
tienifr Dec 20, 2023
0ae8452
Merge branch 'main' of https://github.com/tienifr/App into fix/25892
tienifr Aug 1, 2024
f1b4e20
reapply changes
tienifr Aug 1, 2024
fe16fc0
fix lint
tienifr Aug 1, 2024
f6b25c8
focus to edit composer if press its emoji picker
tienifr Aug 2, 2024
21d8617
Merge branch 'main' into fix/25892
tienifr Aug 9, 2024
df83fc8
Merge branch 'main' into fix/25892
tienifr Aug 19, 2024
599fe46
resolve conflicts
tienifr Aug 21, 2024
031ab6c
Merge branch 'main' into fix/25892
tienifr Aug 30, 2024
dc73745
lint fix
tienifr Aug 30, 2024
b41e23c
Merge branch 'main' into fix/25892
tienifr Sep 6, 2024
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
22 changes: 0 additions & 22 deletions src/components/Composer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ import * as Browser from '@libs/Browser';
import updateIsFullComposerAvailable from '@libs/ComposerUtils/updateIsFullComposerAvailable';
import * as EmojiUtils from '@libs/EmojiUtils';
import * as FileUtils from '@libs/fileDownload/FileUtils';
import focusComposerWithDelay from '@libs/focusComposerWithDelay';
import isEnterWhileComposition from '@libs/KeyboardShortcut/isEnterWhileComposition';
import ReportActionComposeFocusManager from '@libs/ReportActionComposeFocusManager';
import CONST from '@src/CONST';
import type {ComposerProps} from './types';

Expand Down Expand Up @@ -72,7 +70,6 @@ function Composer(
start: 0,
end: 0,
},
isReportActionCompose = false,
isComposerFullSize = false,
shouldContainScroll = true,
isGroupPolicyReport = false,
Expand Down Expand Up @@ -277,14 +274,6 @@ function Composer(

useEffect(() => {
setIsRendered(true);

return () => {
if (isReportActionCompose) {
return;
}
ReportActionComposeFocusManager.clear();
};
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, []);

const clear = useCallback(() => {
Expand Down Expand Up @@ -408,17 +397,6 @@ function Composer(
}}
disabled={isDisabled}
onKeyPress={handleKeyPress}
onFocus={(e) => {
ReportActionComposeFocusManager.onComposerFocus(() => {
if (!textInput.current) {
return;
}

focusComposerWithDelay(textInput.current)(true);
});

props.onFocus?.(e);
}}
/>
{shouldCalculateCaretPosition && renderElementForCaretPosition}
</>
Expand Down
3 changes: 0 additions & 3 deletions src/components/Composer/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ type ComposerProps = Omit<TextInputProps, 'onClear'> & {
/** Function to check whether composer is covered up or not */
checkComposerVisibility?: () => boolean;

/** Whether this is the report action compose */
isReportActionCompose?: boolean;

/** Whether the sull composer is open */
isComposerFullSize?: boolean;

Expand Down
9 changes: 7 additions & 2 deletions src/components/EmojiPicker/EmojiPickerButton.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {useIsFocused} from '@react-navigation/native';
import React, {memo, useEffect, useRef} from 'react';
import type {GestureResponderEvent} from 'react-native';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
Expand All @@ -21,6 +22,9 @@ type EmojiPickerButtonProps = {
/** Unique id for emoji picker */
emojiPickerID?: string;

/** A callback function when the button is pressed */
onPress?: (event?: GestureResponderEvent | KeyboardEvent) => void;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onPress?: (event?: GestureResponderEvent | KeyboardEvent) => void;
onTriggerPress?: (event?: GestureResponderEvent | KeyboardEvent) => void;


/** Emoji popup anchor offset shift vertical */
shiftVertical?: number;

Expand All @@ -29,7 +33,7 @@ type EmojiPickerButtonProps = {
onEmojiSelected: EmojiPickerAction.OnEmojiSelected;
};

function EmojiPickerButton({isDisabled = false, id = '', emojiPickerID = '', shiftVertical = 0, onModalHide, onEmojiSelected}: EmojiPickerButtonProps) {
function EmojiPickerButton({isDisabled = false, id = '', emojiPickerID = '', shiftVertical = 0, onPress, onModalHide, onEmojiSelected}: EmojiPickerButtonProps) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const emojiPopoverAnchor = useRef(null);
Expand All @@ -44,7 +48,7 @@ function EmojiPickerButton({isDisabled = false, id = '', emojiPickerID = '', shi
ref={emojiPopoverAnchor}
style={({hovered, pressed}) => [styles.chatItemEmojiButton, StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed))]}
disabled={isDisabled}
onPress={() => {
onPress={(e) => {
if (!isFocused) {
return;
}
Expand All @@ -64,6 +68,7 @@ function EmojiPickerButton({isDisabled = false, id = '', emojiPickerID = '', shi
} else {
EmojiPickerAction.emojiPickerRef.current.hideEmojiPicker();
}
onPress?.(e);
}}
id={id}
accessibilityLabel={translate('reportActionCompose.emoji')}
Expand Down
32 changes: 17 additions & 15 deletions src/libs/ReportActionComposeFocusManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,20 @@ type FocusCallback = (shouldFocusForNonBlurInputOnTapOutside?: boolean) => void;

const composerRef: MutableRefObject<TextInput | null> = React.createRef<TextInput>();
const editComposerRef = React.createRef<TextInput>();
// There are two types of composer: general composer (edit composer) and main composer.
// The general composer callback will take priority if it exists.
// There are two types of focus callbacks: priority and general
// Priority callback would take priority if it existed
let priorityFocusCallback: FocusCallback | null = null;
let focusCallback: FocusCallback | null = null;
let mainComposerFocusCallback: FocusCallback | null = null;

/**
* Register a callback to be called when focus is requested.
* Typical uses of this would be call the focus on the ReportActionComposer.
*
* @param callback callback to register
*/
function onComposerFocus(callback: FocusCallback | null, isMainComposer = false) {
if (isMainComposer) {
mainComposerFocusCallback = callback;
function onComposerFocus(callback: FocusCallback | null, isPriorityCallback = false) {
if (isPriorityCallback) {
priorityFocusCallback = callback;
} else {
focusCallback = callback;
}
Expand All @@ -39,24 +39,26 @@ function focus(shouldFocusForNonBlurInputOnTapOutside?: boolean) {
return;
}

if (typeof focusCallback !== 'function') {
if (typeof mainComposerFocusCallback !== 'function') {
return;
}
if (typeof priorityFocusCallback !== 'function' && typeof focusCallback !== 'function') {
return;
}

mainComposerFocusCallback(shouldFocusForNonBlurInputOnTapOutside);
if (typeof priorityFocusCallback === 'function') {
priorityFocusCallback(shouldFocusForNonBlurInputOnTapOutside);
return;
}

focusCallback();
if (typeof focusCallback === 'function') {
focusCallback();
}
}

/**
* Clear the registered focus callback
*/
function clear(isMainComposer = false) {
if (isMainComposer) {
mainComposerFocusCallback = null;
function clear(isPriorityCallback = false) {
if (isPriorityCallback) {
priorityFocusCallback = null;
} else {
focusCallback = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -551,16 +551,22 @@ function ComposerWithSuggestions(
focusComposerWithDelay(textInputRef.current)(shouldDelay);
}, []);

const setUpComposeFocusManager = useCallback(() => {
// This callback is used in the contextMenuActions to manage giving focus back to the compose input.
ReportActionComposeFocusManager.onComposerFocus((shouldFocusForNonBlurInputOnTapOutside = false) => {
if ((!willBlurTextInputOnTapOutside && !shouldFocusForNonBlurInputOnTapOutside) || !isFocused) {
return;
}
/**
* Set focus callback
* @param shouldTakeOverFocus - Whether this composer should gain focus priority
*/
const setUpComposeFocusManager = useCallback(
(shouldTakeOverFocus = false) => {
ReportActionComposeFocusManager.onComposerFocus((shouldFocusForNonBlurInputOnTapOutside = false) => {
if ((!willBlurTextInputOnTapOutside && !shouldFocusForNonBlurInputOnTapOutside) || !isFocused) {
return;
}

focus(true);
}, true);
}, [focus, isFocused]);
focus(true);
}, shouldTakeOverFocus);
},
[focus, isFocused],
);

/**
* Check if the composer is visible. Returns true if the composer is not covered up by emoji picker or menu. False otherwise.
Expand Down Expand Up @@ -623,7 +629,7 @@ function ComposerWithSuggestions(
setUpComposeFocusManager();

return () => {
ReportActionComposeFocusManager.clear(true);
ReportActionComposeFocusManager.clear();

KeyDownListener.removeKeyDownPressListener(focusComposerOnKeyPress);
unsubscribeNavigationBlur();
Expand Down Expand Up @@ -756,7 +762,11 @@ function ComposerWithSuggestions(
textAlignVertical="top"
style={[styles.textInputCompose, isComposerFullSize ? styles.textInputFullCompose : styles.textInputCollapseCompose]}
maxLines={maxComposerLines}
onFocus={onFocus}
onFocus={() => {
// The last composer that had focus should re-gain focus
setUpComposeFocusManager(true);
onFocus();
}}
onBlur={onBlur}
onClick={setShouldBlockSuggestionCalcToFalse}
onPasteFile={(file) => {
Expand All @@ -765,7 +775,6 @@ function ComposerWithSuggestions(
}}
onClear={onClear}
isDisabled={isBlockedFromConcierge || disabled}
isReportActionCompose
selection={selection}
onSelectionChange={onSelectionChange}
isFullComposerAvailable={isFullComposerAvailable}
Expand Down
38 changes: 30 additions & 8 deletions src/pages/home/report/ReportActionItemMessageEdit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,29 @@ function ReportActionItemMessageEdit(
[action.reportActionID],
);

/**
* Focus the composer text input
* @param shouldDelay - Impose delay before focusing the composer
*/
const focus = useCallback((shouldDelay = false, forcedSelectionRange?: Selection) => {
focusComposerWithDelay(textInputRef.current)(shouldDelay, forcedSelectionRange);
}, []);

// Take over focus priority
const setUpComposeFocusManager = useCallback(() => {
ReportActionComposeFocusManager.onComposerFocus(() => {
focus(true, emojiPickerSelectionRef.current ? {...emojiPickerSelectionRef.current} : undefined);
}, true);
}, [focus]);

useEffect(
// Remove focus callback on unmount to avoid stale callbacks
() => () => {
ReportActionComposeFocusManager.clear(true);
},
[],
);

useEffect(
() => {
if (isInitialMount.current) {
Expand Down Expand Up @@ -274,8 +297,9 @@ function ReportActionItemMessageEdit(
Report.deleteReportActionDraft(reportID, action);

if (isActive()) {
ReportActionComposeFocusManager.clear();
ReportActionComposeFocusManager.focus();
ReportActionComposeFocusManager.clear(true);
// Wait for report action compose re-mounting on mWeb
InteractionManager.runAfterInteractions(() => ReportActionComposeFocusManager.focus());
Comment on lines +300 to +302
Copy link
Member

Choose a reason for hiding this comment

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

Why are we clearing before focusing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When users edit message, the edit composer will be the priority composer. Then when they cancel edit mode, we need to remove the edit composer focus callback first, so when trigger ReportActionComposeFocusManager.focus(), the main composer will be focused.

}

// Scroll to the last comment after editing to make sure the whole comment is clearly visible in the report.
Expand Down Expand Up @@ -424,11 +448,6 @@ function ReportActionItemMessageEdit(
[],
);

/**
* Focus the composer text input
*/
const focus = focusComposerWithDelay(textInputRef.current);

useEffect(() => {
validateCommentMaxLength(draft, {reportID});
}, [draft, reportID, validateCommentMaxLength]);
Expand Down Expand Up @@ -503,6 +522,8 @@ function ReportActionItemMessageEdit(
});
});
setShouldShowComposeInputKeyboardAware(false);
// The last composer that had focus should re-gain focus
setUpComposeFocusManager();

// Clear active report action when another action gets focused
if (!EmojiPickerAction.isActive(action.reportActionID)) {
Expand Down Expand Up @@ -546,11 +567,12 @@ function ReportActionItemMessageEdit(
<EmojiPickerButton
isDisabled={shouldDisableEmojiPicker}
onModalHide={() => {
focus(true, emojiPickerSelectionRef.current ? {...emojiPickerSelectionRef.current} : undefined);
ReportActionComposeFocusManager.focus();
}}
onEmojiSelected={addEmojiToTextBox}
id={emojiButtonID}
emojiPickerID={action.reportActionID}
onPress={setUpComposeFocusManager}
/>
</View>

Expand Down
Loading