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

Standardize keyboard libs for determining when the keyboard is open #13629

Merged
merged 7 commits into from
Dec 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import ComposeProviders from './components/ComposeProviders';
import SafeArea from './components/SafeArea';
import * as Environment from './libs/Environment/Environment';
import {WindowDimensionsProvider} from './components/withWindowDimensions';
import {KeyboardStateProvider} from './components/withKeyboardState';

// For easier debugging and development, when we are in web we expose Onyx to the window, so you can more easily set data into Onyx
if (window && Environment.isDevelopment()) {
Expand All @@ -41,6 +42,7 @@ const App = () => (
LocaleContextProvider,
HTMLEngineProvider,
WindowDimensionsProvider,
KeyboardStateProvider,
]}
>
<CustomStatusBar />
Expand Down
13 changes: 7 additions & 6 deletions src/components/HeaderWithCloseButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import Navigation from '../libs/Navigation/Navigation';
import ROUTES from '../ROUTES';
import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import Tooltip from './Tooltip';
import ThreeDotsMenu, {ThreeDotsMenuItemPropTypes} from './ThreeDotsMenu';
import VirtualKeyboard from '../libs/VirtualKeyboard';
import getButtonState from '../libs/getButtonState';
import * as StyleUtils from '../styles/StyleUtils';
import withDelayToggleButtonState, {withDelayToggleButtonStatePropTypes} from './withDelayToggleButtonState';
import compose from '../libs/compose';
import ThreeDotsMenu, {ThreeDotsMenuItemPropTypes} from './ThreeDotsMenu';
import withDelayToggleButtonState, {withDelayToggleButtonStatePropTypes} from './withDelayToggleButtonState';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import withKeyboardState, {keyboardStatePropTypes} from './withKeyboardState';

const propTypes = {
/** Title of the Header */
Expand Down Expand Up @@ -79,8 +79,8 @@ const propTypes = {
}),

...withLocalizePropTypes,

...withDelayToggleButtonStatePropTypes,
...keyboardStatePropTypes,
};

const defaultProps = {
Expand Down Expand Up @@ -142,7 +142,7 @@ class HeaderWithCloseButton extends Component {
<Tooltip text={this.props.translate('common.back')}>
<Pressable
onPress={() => {
if (VirtualKeyboard.isOpen()) {
if (this.props.isKeyboardShown) {
Keyboard.dismiss();
}
this.props.onBackButtonPress();
Expand Down Expand Up @@ -224,4 +224,5 @@ HeaderWithCloseButton.defaultProps = defaultProps;
export default compose(
withLocalize,
withDelayToggleButtonState,
withKeyboardState,
)(HeaderWithCloseButton);
2 changes: 1 addition & 1 deletion src/components/PDFView/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class PDFView extends Component {
}

componentDidUpdate() {
this.props.onToggleKeyboard(this.props.isShown);
this.props.onToggleKeyboard(this.props.isKeyboardShown);
}

handleFailureToLoadPDF(error) {
Expand Down
2 changes: 2 additions & 0 deletions src/components/PDFView/pdfViewPropTypes.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import PropTypes from 'prop-types';
import stylePropTypes from '../../styles/stylePropTypes';
import {windowDimensionsPropTypes} from '../withWindowDimensions';
import {keyboardStatePropTypes} from '../withKeyboardState';

const propTypes = {
/** URL to full-sized image */
Expand All @@ -13,6 +14,7 @@ const propTypes = {
onToggleKeyboard: PropTypes.func,

...windowDimensionsPropTypes,
...keyboardStatePropTypes,
Copy link
Member

@rushatgabhane rushatgabhane Feb 6, 2023

Choose a reason for hiding this comment

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

Hey, we caused a console error with this change. #13862

Why? keyboardStatePropTypes contains native only props but being used for all platforms.

};

const defaultProps = {
Expand Down
113 changes: 64 additions & 49 deletions src/components/withKeyboardState.js
Original file line number Diff line number Diff line change
@@ -1,64 +1,79 @@
import React, {Component} from 'react';
/* eslint-disable react/no-unused-state */
import React, {forwardRef, createContext} from 'react';
import PropTypes from 'prop-types';
import {Keyboard} from 'react-native';
import getComponentDisplayName from '../libs/getComponentDisplayName';

const withKeyboardStatePropTypes = {
/** Returns whether keyboard is open */
isShown: PropTypes.bool.isRequired,
const KeyboardStateContext = createContext(null);
const keyboardStatePropTypes = {
/** Whether or not the keyboard is open */
isKeyboardShown: PropTypes.bool.isRequired,
};

export default function withKeyboardState(WrappedComponent) {
const WithKeyboardState = class extends Component {
constructor(props) {
super(props);
this.state = {
isShown: false,
};
}
const keyboardStateProviderPropTypes = {
/* Actual content wrapped by this component */
children: PropTypes.node.isRequired,
};

class KeyboardStateProvider extends React.Component {
constructor(props) {
super(props);

this.state = {
isKeyboardShown: false,
};
}

componentDidMount() {
this.keyboardDidShowListener = Keyboard.addListener(
'keyboardDidShow',
() => {
this.setState({isShown: true});
},
);
this.keyboardDidHideListener = Keyboard.addListener(
'keyboardDidHide',
() => {
this.setState({isShown: false});
},
);
}
componentDidMount() {
this.keyboardDidShowListener = Keyboard.addListener(
'keyboardDidShow',
() => {
this.setState({isKeyboardShown: true});
},
);
this.keyboardDidHideListener = Keyboard.addListener(
'keyboardDidHide',
() => {
this.setState({isKeyboardShown: false});
},
);
}

componentWillUnmount() {
this.keyboardDidShowListener.remove();
this.keyboardDidHideListener.remove();
}
componentWillUnmount() {
this.keyboardDidShowListener.remove();
this.keyboardDidHideListener.remove();
}

render() {
// eslint-disable-next-line react/jsx-props-no-spreading
return <WrappedComponent {...this.props} isShown={this.state.isShown} />;
}
};
render() {
return (
<KeyboardStateContext.Provider value={this.state}>
{this.props.children}
</KeyboardStateContext.Provider>
);
}
}

KeyboardStateProvider.propTypes = keyboardStateProviderPropTypes;

WithKeyboardState.displayName = `WithKeyboardState(${getComponentDisplayName(WrappedComponent)})`;
WithKeyboardState.propTypes = {
forwardedRef: PropTypes.oneOfType([
PropTypes.func,
PropTypes.shape({current: PropTypes.instanceOf(React.Component)}),
]),
};
WithKeyboardState.defaultProps = {
forwardedRef: undefined,
};
return React.forwardRef((props, ref) => (
// eslint-disable-next-line react/jsx-props-no-spreading
<WithKeyboardState {...props} forwardedRef={ref} />
/**
* @param {React.Component} WrappedComponent
* @returns {React.Component}
*/
export default function withKeyboardState(WrappedComponent) {
const WithKeyboardState = forwardRef((props, ref) => (
<KeyboardStateContext.Consumer>
{keyboardStateProps => (
// eslint-disable-next-line react/jsx-props-no-spreading
<WrappedComponent {...keyboardStateProps} {...props} ref={ref} />
)}
</KeyboardStateContext.Consumer>
));

WithKeyboardState.displayName = `withKeyboardState(${getComponentDisplayName(WrappedComponent)})`;
return WithKeyboardState;
}

export {
withKeyboardStatePropTypes,
KeyboardStateProvider,
keyboardStatePropTypes,
};
31 changes: 0 additions & 31 deletions src/libs/VirtualKeyboard/index.js

This file was deleted.

33 changes: 0 additions & 33 deletions src/libs/VirtualKeyboard/index.native.js

This file was deleted.

13 changes: 8 additions & 5 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,8 @@ import ReportTypingIndicator from './ReportTypingIndicator';
import AttachmentModal from '../../../components/AttachmentModal';
import compose from '../../../libs/compose';
import PopoverMenu from '../../../components/PopoverMenu';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
import withDrawerState from '../../../components/withDrawerState';
import CONST from '../../../CONST';
import canFocusInputOnScreenFocus from '../../../libs/canFocusInputOnScreenFocus';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import Permissions from '../../../libs/Permissions';
import Navigation from '../../../libs/Navigation/Navigation';
import ROUTES from '../../../ROUTES';
Expand All @@ -39,7 +36,6 @@ import {withNetwork, withPersonalDetails} from '../../../components/OnyxProvider
import * as User from '../../../libs/actions/User';
import Tooltip from '../../../components/Tooltip';
import EmojiPickerButton from '../../../components/EmojiPicker/EmojiPickerButton';
import VirtualKeyboard from '../../../libs/VirtualKeyboard';
import canUseTouchScreen from '../../../libs/canUseTouchscreen';
import toggleReportActionComposeView from '../../../libs/toggleReportActionComposeView';
import OfflineIndicator from '../../../components/OfflineIndicator';
Expand All @@ -49,6 +45,10 @@ import * as EmojiUtils from '../../../libs/EmojiUtils';
import reportPropTypes from '../../reportPropTypes';
import ReportDropUI from './ReportDropUI';
import DragAndDrop from '../../../components/DragAndDrop';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
import withDrawerState from '../../../components/withDrawerState';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import withKeyboardState, {keyboardStatePropTypes} from '../../../components/withKeyboardState';

const propTypes = {
/** Beta features list */
Expand Down Expand Up @@ -99,6 +99,7 @@ const propTypes = {
...windowDimensionsPropTypes,
...withLocalizePropTypes,
...withCurrentUserPersonalDetailsPropTypes,
...keyboardStatePropTypes,
};

const defaultProps = {
Expand Down Expand Up @@ -430,7 +431,8 @@ class ReportActionCompose extends React.Component {
* @param {Object} e
*/
triggerHotkeyActions(e) {
if (!e || VirtualKeyboard.shouldAssumeIsOpen()) {
// Do not trigger actions for mobileWeb or native clients that have the keyboard open because for those devices, we want the return key to insert newlines rather than submit the form
if (!e || this.props.isSmallScreenWidth || this.props.isKeyboardShown) {
Copy link
Member

@parasharrajat parasharrajat Dec 28, 2022

Choose a reason for hiding this comment

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

Did we test it against resized the Desktop app? I think we do not want that to behave like mobile app as that does not have Virtual Keyboard.

cc: @thesahindia

Copy link
Member

Choose a reason for hiding this comment

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

It is expected (#13629 (comment), slack thread)

Copy link
Member

@parasharrajat parasharrajat Dec 28, 2022

Choose a reason for hiding this comment

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

Thanks for sharing. Hmm, I can't give my full approval here without testing but let's hope there are no regressions from this. Anyways, you approved so this should be OK.

Copy link
Member

Choose a reason for hiding this comment

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

Hi, just dropping a tiny tiny note that when a desktop window is resized to mobile, the Enter key behaviour changes and isn't consistent with full sized window.

Causing bug - #17206

return;
}

Expand Down Expand Up @@ -741,6 +743,7 @@ export default compose(
withNetwork(),
withPersonalDetails(),
withCurrentUserPersonalDetails,
withKeyboardState,
withOnyx({
betas: {
key: ONYXKEYS.BETAS,
Expand Down
Loading