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 for: mWeb/Chrome - Payment - The keyboard overlaps the "Make default payment method" button #14725

Merged
merged 15 commits into from
Feb 9, 2023
Merged
1 change: 1 addition & 0 deletions src/components/Modal/BaseModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class BaseModal extends PureComponent {
},
this.props.popoverAnchorPosition,
this.props.containerStyle,
this.props.extraModalStyles,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really like the name extraModalStyles, can we come up with something a bit more descriptive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, any suggestions?

Copy link
Contributor Author

@Ollyws Ollyws Feb 3, 2023

Choose a reason for hiding this comment

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

@jasperhuangg customModalStyles?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aimane-chnaif Any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is already Modal, no need to label Modal I think.
So maybe something like customContainerStyle?
All yours @jasperhuangg

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for your patience guys have been out sick.

I like outerStyle? And I think we can rename containerStyle to innerContainerStyle since the container those styles are being applied to lives inside the modal.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jasperhuangg sorry, hope you'd feel better now.
Good suggestion. I agree with that approach. But how about contentContainerStyle since this name is more common and already used in other places?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jasperhuangg No worries, hope you're feeling better.
outerStyle and innerContainerStyle make sense to me, just give me the word and I'll implement these changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jasperhuangg sorry bump, can you please confirm the preferred name soon? Hope delay for waiting this won't affect timeline

Copy link
Contributor

Choose a reason for hiding this comment

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

@Ollyws yeah go ahead! Sorry for the delay

);
return (
<ReactNativeModal
Expand Down
3 changes: 3 additions & 0 deletions src/components/PasswordPopover/BasePasswordPopover.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import TextInput from '../TextInput';
import KeyboardSpacer from '../KeyboardSpacer';
import {propTypes as passwordPopoverPropTypes, defaultProps as passwordPopoverDefaultProps} from './passwordPopoverPropTypes';
import Button from '../Button';
import withViewportOffsetTop from '../withViewportOffsetTop';

const propTypes = {
/** Whether we should wait before focusing the TextInput, useful when using transitions on Android */
Expand Down Expand Up @@ -55,6 +56,7 @@ class BasePasswordPopover extends Component {
onClose={this.props.onClose}
anchorPosition={this.props.anchorPosition}
onModalShow={this.focusInput}
extraModalStyles={{maxHeight: this.props.windowHeight, marginTop: this.props.viewportOffsetTop}}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be careful to use maxHeight here so it doesn't cause any regressions.
ref: #14716 (comment)

Copy link
Contributor Author

@Ollyws Ollyws Feb 3, 2023

Choose a reason for hiding this comment

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

@aimane-chnaif Agreed, but this component is only used used in BasePaymentsPage so every instance of it can be easily tested.

>
<View
style={[
Expand Down Expand Up @@ -98,6 +100,7 @@ class BasePasswordPopover extends Component {
BasePasswordPopover.propTypes = propTypes;
BasePasswordPopover.defaultProps = defaultProps;
export default compose(
withViewportOffsetTop,
withWindowDimensions,
withLocalize,
)(BasePasswordPopover);
72 changes: 72 additions & 0 deletions src/components/withViewportOffsetTop.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import React, {Component} from 'react';
import PropTypes from 'prop-types';
import lodashGet from 'lodash/get';
import getComponentDisplayName from '../libs/getComponentDisplayName';
import addViewportResizeListener from '../libs/VisualViewport';

const viewportOffsetTopPropTypes = {
// viewportOffsetTop returns the offset of the top edge of the visual viewport from the
// top edge of the layout viewport in CSS pixels, when the visual viewport is resized.

viewportOffsetTop: PropTypes.number.isRequired,
jasperhuangg marked this conversation as resolved.
Show resolved Hide resolved
};

export default function (WrappedComponent) {
class WithViewportOffsetTop extends Component {
constructor(props) {
super(props);

this.updateDimensions = this.updateDimensions.bind(this);

this.state = {
viewportOffsetTop: 0,
};
}

componentDidMount() {
this.removeViewportResizeListener = addViewportResizeListener(this.updateDimensions);
}

componentWillUnmount() {
this.removeViewportResizeListener();
}

/**
* @param {SyntheticEvent} e
*/
updateDimensions(e) {
const viewportOffsetTop = lodashGet(e, 'target.offsetTop', 0);
this.setState({viewportOffsetTop});
}

render() {
return (
<WrappedComponent
// eslint-disable-next-line react/jsx-props-no-spreading
{...this.props}
ref={this.props.forwardedRef}
viewportOffsetTop={this.state.viewportOffsetTop}
/>
);
}
}

WithViewportOffsetTop.displayName = `WithViewportOffsetTop(${getComponentDisplayName(WrappedComponent)})`;
WithViewportOffsetTop.propTypes = {
forwardedRef: PropTypes.oneOfType([
PropTypes.func,
PropTypes.shape({current: PropTypes.instanceOf(React.Component)}),
]),
};
WithViewportOffsetTop.defaultProps = {
forwardedRef: undefined,
};
return React.forwardRef((props, ref) => (
// eslint-disable-next-line react/jsx-props-no-spreading
<WithViewportOffsetTop {...props} forwardedRef={ref} />
));
}

export {
viewportOffsetTopPropTypes,
};
22 changes: 4 additions & 18 deletions src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import CONST from '../../CONST';
import ReportActionsSkeletonView from '../../components/ReportActionsSkeletonView';
import reportActionPropTypes from './report/reportActionPropTypes';
import toggleReportActionComposeView from '../../libs/toggleReportActionComposeView';
import addViewportResizeListener from '../../libs/VisualViewport';
import {withNetwork} from '../../components/OnyxProvider';
import compose from '../../libs/compose';
import networkPropTypes from '../../components/networkPropTypes';
Expand All @@ -33,6 +32,7 @@ import withLocalize from '../../components/withLocalize';
import reportPropTypes from '../reportPropTypes';
import FullPageNotFoundView from '../../components/BlockingViews/FullPageNotFoundView';
import ReportHeaderSkeletonView from '../../components/ReportHeaderSkeletonView';
import withViewportOffsetTop, {viewportOffsetTopPropTypes} from '../../components/withViewportOffsetTop';

const propTypes = {
/** Navigation route context info provided by react navigation */
Expand Down Expand Up @@ -73,6 +73,7 @@ const propTypes = {

...windowDimensionsPropTypes,
...withDrawerPropTypes,
...viewportOffsetTopPropTypes,
};

const defaultProps = {
Expand Down Expand Up @@ -107,22 +108,18 @@ class ReportScreen extends React.Component {
super(props);

this.onSubmitComment = this.onSubmitComment.bind(this);
this.updateViewportOffsetTop = this.updateViewportOffsetTop.bind(this);
this.chatWithAccountManager = this.chatWithAccountManager.bind(this);
this.dismissBanner = this.dismissBanner.bind(this);
this.removeViewportResizeListener = () => {};

this.state = {
skeletonViewContainerHeight: reportActionsListViewHeight,
viewportOffsetTop: 0,
isBannerVisible: true,
};
}

componentDidMount() {
this.fetchReportIfNeeded();
toggleReportActionComposeView(true);
this.removeViewportResizeListener = addViewportResizeListener(this.updateViewportOffsetTop);
}

componentDidUpdate(prevProps) {
Expand All @@ -134,10 +131,6 @@ class ReportScreen extends React.Component {
toggleReportActionComposeView(true);
}

componentWillUnmount() {
this.removeViewportResizeListener();
}

/**
* @param {String} text
*/
Expand Down Expand Up @@ -171,14 +164,6 @@ class ReportScreen extends React.Component {
Report.openReport(reportIDFromPath);
}

/**
* @param {SyntheticEvent} e
*/
updateViewportOffsetTop(e) {
const viewportOffsetTop = lodashGet(e, 'target.offsetTop', 0);
this.setState({viewportOffsetTop});
}

dismissBanner() {
this.setState({isBannerVisible: false});
}
Expand Down Expand Up @@ -214,7 +199,7 @@ class ReportScreen extends React.Component {
const reportID = getReportID(this.props.route);
const addWorkspaceRoomOrChatPendingAction = lodashGet(this.props.report, 'pendingFields.addWorkspaceRoom') || lodashGet(this.props.report, 'pendingFields.createChat');
const addWorkspaceRoomOrChatErrors = lodashGet(this.props.report, 'errorFields.addWorkspaceRoom') || lodashGet(this.props.report, 'errorFields.createChat');
const screenWrapperStyle = [styles.appContent, styles.flex1, {marginTop: this.state.viewportOffsetTop}];
const screenWrapperStyle = [styles.appContent, styles.flex1, {marginTop: this.props.viewportOffsetTop}];

// There are no reportActions at all to display and we are still in the process of loading the next set of actions.
const isLoadingInitialReportActions = _.isEmpty(this.props.reportActions) && this.props.report.isLoadingReportActions;
Expand Down Expand Up @@ -332,6 +317,7 @@ ReportScreen.propTypes = propTypes;
ReportScreen.defaultProps = defaultProps;

export default compose(
withViewportOffsetTop,
withLocalize,
withWindowDimensions,
withDrawerState,
Expand Down
3 changes: 2 additions & 1 deletion src/styles/getModalStyles/getBaseModalStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import CONST from '../../CONST';
import variables from '../variables';
import themeColors from '../themes/default';

export default (type, windowDimensions, popoverAnchorPosition = {}, containerStyle = {}) => {
export default (type, windowDimensions, popoverAnchorPosition = {}, containerStyle = {}, extraModalStyles = {}) => {
const {isSmallScreenWidth, windowWidth} = windowDimensions;

let modalStyle = {
margin: 0,
...extraModalStyles,
};

let modalContainerStyle;
Expand Down