Skip to content

Commit

Permalink
Merge pull request #23994 from ntdiary/fix-13922
Browse files Browse the repository at this point in the history
fix keyboard flashing while clicking "Add attachment"
  • Loading branch information
Joel Bettner authored Aug 23, 2023
2 parents 76d72e4 + 67d3228 commit 431d23d
Show file tree
Hide file tree
Showing 11 changed files with 146 additions and 62 deletions.
18 changes: 18 additions & 0 deletions patches/react-native+0.72.3+004+ModalKeyboardFlashing.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
diff --git a/node_modules/react-native/React/Views/RCTModalHostViewManager.m b/node_modules/react-native/React/Views/RCTModalHostViewManager.m
index 4b9f9ad..b72984c 100644
--- a/node_modules/react-native/React/Views/RCTModalHostViewManager.m
+++ b/node_modules/react-native/React/Views/RCTModalHostViewManager.m
@@ -79,6 +79,13 @@ RCT_EXPORT_MODULE()
if (self->_presentationBlock) {
self->_presentationBlock([modalHostView reactViewController], viewController, animated, completionBlock);
} else {
+ // In our App, If an input is blurred and a modal is opened, the rootView will become the firstResponder, which
+ // will cause system to retain a wrong keyboard state, and then the keyboard to flicker when the modal is closed.
+ // We first resign the rootView to avoid this problem.
+ UIWindow *window = RCTKeyWindow();
+ if (window && window.rootViewController && [window.rootViewController.view isFirstResponder]) {
+ [window.rootViewController.view resignFirstResponder];
+ }
[[modalHostView reactViewController] presentViewController:viewController
animated:animated
completion:completionBlock];
13 changes: 11 additions & 2 deletions src/components/AttachmentPicker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ function getAcceptableFileTypes(type) {
function AttachmentPicker(props) {
const fileInput = useRef();
const onPicked = useRef();
const onCanceled = useRef(() => {});

return (
<>
<input
Expand All @@ -46,13 +48,20 @@ function AttachmentPicker(props) {
}}
// We are stopping the event propagation because triggering the `click()` on the hidden input
// causes the event to unexpectedly bubble up to anything wrapping this component e.g. Pressable
onClick={(e) => e.stopPropagation()}
onClick={(e) => {
e.stopPropagation();
if (!fileInput.current) {
return;
}
fileInput.current.addEventListener('cancel', () => onCanceled.current(), {once: true});
}}
accept={getAcceptableFileTypes(props.type)}
/>
{props.children({
openPicker: ({onPicked: newOnPicked}) => {
openPicker: ({onPicked: newOnPicked, onCanceled: newOnCanceled = () => {}}) => {
onPicked.current = newOnPicked;
fileInput.current.click();
onCanceled.current = newOnCanceled;
},
})}
</>
Expand Down
13 changes: 10 additions & 3 deletions src/components/AttachmentPicker/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ function AttachmentPicker({type, children}) {

const completeAttachmentSelection = useRef();
const onModalHide = useRef();
const onCanceled = useRef();

const {translate} = useLocalize();
const {isSmallScreenWidth} = useWindowDimensions();
Expand Down Expand Up @@ -216,9 +217,11 @@ function AttachmentPicker({type, children}) {
* Opens the attachment modal
*
* @param {function} onPickedHandler A callback that will be called with the selected attachment
* @param {function} onCanceledHandler A callback that will be called without a selected attachment
*/
const open = (onPickedHandler) => {
const open = (onPickedHandler, onCanceledHandler = () => {}) => {
completeAttachmentSelection.current = onPickedHandler;
onCanceled.current = onCanceledHandler;
setIsVisible(true);
};

Expand All @@ -239,6 +242,7 @@ function AttachmentPicker({type, children}) {
const pickAttachment = useCallback(
(attachments = []) => {
if (attachments.length === 0) {
onCanceled.current();
return Promise.resolve();
}

Expand Down Expand Up @@ -308,13 +312,16 @@ function AttachmentPicker({type, children}) {
*/
const renderChildren = () =>
children({
openPicker: ({onPicked}) => open(onPicked),
openPicker: ({onPicked, onCanceled: newOnCanceled}) => open(onPicked, newOnCanceled),
});

return (
<>
<Popover
onClose={close}
onClose={() => {
close();
onCanceled.current();
}}
isVisible={isVisible}
anchorPosition={styles.createMenuPosition}
onModalHide={onModalHide.current}
Expand Down
8 changes: 8 additions & 0 deletions src/components/Modal/BaseModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {propTypes as modalPropTypes, defaultProps as modalDefaultProps} from './
import * as Modal from '../../libs/actions/Modal';
import getModalStyles from '../../styles/getModalStyles';
import variables from '../../styles/variables';
import ComposerFocusManager from '../../libs/ComposerFocusManager';

const propTypes = {
...modalPropTypes,
Expand Down Expand Up @@ -73,6 +74,9 @@ class BaseModal extends PureComponent {
this.props.onModalHide();
}
Modal.onModalDidClose();
if (!this.props.fullscreen) {
ComposerFocusManager.setReadyToFocus();
}
}

render() {
Expand Down Expand Up @@ -109,6 +113,9 @@ class BaseModal extends PureComponent {
// Note: Escape key on web/desktop will trigger onBackButtonPress callback
// eslint-disable-next-line react/jsx-props-no-multi-spaces
onBackButtonPress={this.props.onClose}
onModalWillShow={() => {
ComposerFocusManager.resetReadyToFocus();
}}
onModalShow={() => {
if (this.props.shouldSetModalVisibility) {
Modal.setModalVisibility(true);
Expand All @@ -117,6 +124,7 @@ class BaseModal extends PureComponent {
}}
propagateSwipe={this.props.propagateSwipe}
onModalHide={this.hideModal}
onDismiss={() => ComposerFocusManager.setReadyToFocus()}
onSwipeComplete={this.props.onClose}
swipeDirection={swipeDirection}
isVisible={this.props.isVisible}
Expand Down
10 changes: 10 additions & 0 deletions src/components/Modal/index.android.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import React from 'react';
import {AppState} from 'react-native';
import withWindowDimensions from '../withWindowDimensions';
import BaseModal from './BaseModal';
import {propTypes, defaultProps} from './modalPropTypes';
import ComposerFocusManager from '../../libs/ComposerFocusManager';

AppState.addEventListener('focus', () => {
ComposerFocusManager.setReadyToFocus();
});

AppState.addEventListener('blur', () => {
ComposerFocusManager.resetReadyToFocus();
});

// Only want to use useNativeDriver on Android. It has strange flashes issue on IOS
// https://github.com/react-native-modal/react-native-modal#the-modal-flashes-in-a-weird-way-when-animating
Expand Down
23 changes: 23 additions & 0 deletions src/libs/ComposerFocusManager.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
let isReadyToFocusPromise = Promise.resolve();
let resolveIsReadyToFocus;

function resetReadyToFocus() {
isReadyToFocusPromise = new Promise((resolve) => {
resolveIsReadyToFocus = resolve;
});
}
function setReadyToFocus() {
if (!resolveIsReadyToFocus) {
return;
}
resolveIsReadyToFocus();
}
function isReadyToFocus() {
return isReadyToFocusPromise;
}

export default {
resetReadyToFocus,
setReadyToFocus,
isReadyToFocus,
};
35 changes: 35 additions & 0 deletions src/libs/focusWithDelay.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import {InteractionManager} from 'react-native';
import ComposerFocusManager from './ComposerFocusManager';

/**
* Create a function that focuses a text input.
* @param {Object} textInput the text input to focus
* @returns {Function} a function that focuses the text input with a configurable delay
*/
function focusWithDelay(textInput) {
/**
* Focus the text input
* @param {Boolean} [shouldDelay=false] Impose delay before focusing the text input
*/
return (shouldDelay = false) => {
// There could be other animations running while we trigger manual focus.
// This prevents focus from making those animations janky.
InteractionManager.runAfterInteractions(() => {
if (!textInput) {
return;
}
if (!shouldDelay) {
textInput.focus();
return;
}
ComposerFocusManager.isReadyToFocus().then(() => {
if (!textInput) {
return;
}
textInput.focus();
});
});
};
}

export default focusWithDelay;
40 changes: 0 additions & 40 deletions src/libs/focusWithDelay/focusWithDelay.js

This file was deleted.

7 changes: 0 additions & 7 deletions src/libs/focusWithDelay/index.js

This file was deleted.

6 changes: 0 additions & 6 deletions src/libs/focusWithDelay/index.native.js

This file was deleted.

35 changes: 31 additions & 4 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,17 @@ function ReportActionCompose({
focusWithDelay(textInputRef.current)(shouldDelay);
}, []);

const isNextModalWillOpenRef = useRef(false);
const isKeyboardVisibleWhenShowingModalRef = useRef(false);

const restoreKeyboardState = useCallback(() => {
if (!isKeyboardVisibleWhenShowingModalRef.current) {
return;
}
focus(true);
isKeyboardVisibleWhenShowingModalRef.current = false;
}, [focus]);

/**
* Update the value of the comment in Onyx
*
Expand Down Expand Up @@ -943,7 +954,8 @@ function ReportActionCompose({
shouldBlockEmojiCalc.current = false;
shouldBlockMentionCalc.current = false;
setIsAttachmentPreviewActive(false);
}, []);
restoreKeyboardState();
}, [restoreKeyboardState]);

useEffect(() => {
const unsubscribeNavigationBlur = navigation.addListener('blur', () => KeyDownListener.removeKeyDownPressListner(focusComposerOnKeyPress));
Expand Down Expand Up @@ -982,10 +994,13 @@ function ReportActionCompose({
const prevIsModalVisible = usePrevious(modal.isVisible);
const prevIsFocused = usePrevious(isFocusedProp);
useEffect(() => {
if (modal.isVisible && !prevIsModalVisible) {
isNextModalWillOpenRef.current = false;
}
// We want to focus or refocus the input when a modal has been closed or the underlying screen is refocused.
// We avoid doing this on native platforms since the software keyboard popping
// open creates a jarring and broken UX.
if (!(willBlurTextInputOnTapOutside && !modal.isVisible && isFocusedProp && (prevIsModalVisible || !prevIsFocused))) {
if (!(willBlurTextInputOnTapOutside && !isNextModalWillOpenRef.current && !modal.isVisible && isFocusedProp && (prevIsModalVisible || !prevIsFocused))) {
return;
}

Expand Down Expand Up @@ -1063,8 +1078,10 @@ function ReportActionCompose({
shouldBlockEmojiCalc.current = true;
shouldBlockMentionCalc.current = true;
}
isNextModalWillOpenRef.current = true;
openPicker({
onPicked: displayFileInModal,
onCanceled: restoreKeyboardState,
});
};
const menuItems = [
Expand Down Expand Up @@ -1133,6 +1150,10 @@ function ReportActionCompose({
ref={actionButtonRef}
onPress={(e) => {
e.preventDefault();
if (!willBlurTextInputOnTapOutside) {
isKeyboardVisibleWhenShowingModalRef.current = textInputRef.current.isFocused();
}
textInputRef.current.blur();

// Drop focus to avoid blue focus ring.
actionButtonRef.current.blur();
Expand All @@ -1150,7 +1171,10 @@ function ReportActionCompose({
<PopoverMenu
animationInTiming={CONST.ANIMATION_IN_TIMING}
isVisible={isMenuVisible}
onClose={() => setMenuVisibility(false)}
onClose={() => {
setMenuVisibility(false);
restoreKeyboardState();
}}
onItemSelected={(item, index) => {
setMenuVisibility(false);

Expand Down Expand Up @@ -1185,9 +1209,12 @@ function ReportActionCompose({
style={[styles.textInputCompose, isComposerFullSize ? styles.textInputFullCompose : styles.flex4]}
maxLines={maxComposerLines}
onFocus={() => setIsFocused(true)}
onBlur={() => {
onBlur={(e) => {
setIsFocused(false);
resetSuggestions();
if (e.relatedTarget && e.relatedTarget === actionButtonRef.current) {
isKeyboardVisibleWhenShowingModalRef.current = true;
}
}}
onClick={() => {
shouldBlockEmojiCalc.current = false;
Expand Down

0 comments on commit 431d23d

Please sign in to comment.