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 popover position for payment buttons #28744

Merged
merged 5 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
12 changes: 12 additions & 0 deletions src/components/AddPaymentMethodMenu.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'underscore';
import React from 'react';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
Expand All @@ -24,6 +25,12 @@ const propTypes = {
vertical: PropTypes.number,
}),

/** Where the popover should be positioned relative to the anchor points. */
anchorAlignment: PropTypes.shape({
horizontal: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL)),
vertical: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_VERTICAL)),
}),

/** List of betas available to current user */
betas: PropTypes.arrayOf(PropTypes.string),

Expand All @@ -35,6 +42,10 @@ const propTypes = {

const defaultProps = {
anchorPosition: {},
anchorAlignment: {
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
},
betas: [],
anchorRef: () => {},
};
Expand All @@ -45,6 +56,7 @@ function AddPaymentMethodMenu(props) {
isVisible={props.isVisible}
onClose={props.onClose}
anchorPosition={props.anchorPosition}
anchorAlignment={props.anchorAlignment}
anchorRef={props.anchorRef}
onItemSelected={props.onClose}
menuItems={[
Expand Down
1 change: 1 addition & 0 deletions src/components/ButtonWithDropdownMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ function ButtonWithDropdownMenu(props) {
) : (
<Button
success
ref={props.buttonRef}
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 I added this to fix this bug because I also found it during testing.

pressOnEnter={props.pressOnEnter}
isDisabled={props.isDisabled}
style={[styles.w100, ...props.style]}
Expand Down
11 changes: 7 additions & 4 deletions src/components/KYCWall/BaseKYCWall.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ class KYCWall extends React.Component {
* @returns {Object}
*/
getAnchorPosition(domRect) {
if (this.props.popoverPlacement === 'bottom') {
if (this.props.anchorAlignment.vertical === CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

explanation here -

props.anchorAlignment.vertical === CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP
? y + h + CONST.MODAL.POPOVER_MENU_PADDING // if vertical anchorAlignment is TOP, menu will open below the button and we need to add the height of button and padding
: y - CONST.MODAL.POPOVER_MENU_PADDING, // if it is BOTTOM, menu will open above the button so NO need to add height but DO subtract padding
});

return {
anchorPositionVertical: domRect.top + (domRect.height - 2),
anchorPositionVertical: domRect.top + domRect.height + CONST.MODAL.POPOVER_MENU_PADDING,
anchorPositionHorizontal: domRect.left + 20,
};
}
Expand Down Expand Up @@ -100,7 +100,9 @@ class KYCWall extends React.Component {
this.setState({shouldShowAddPaymentMenu: false});
return;
}
this.setState({transferBalanceButton: event.nativeEvent.target});
// Use event target as fallback if anchorRef is null for safety
marcochavezf marked this conversation as resolved.
Show resolved Hide resolved
const targetElement = this.anchorRef.current || event.nativeEvent.target;
this.setState({transferBalanceButton: targetElement});
const isExpenseReport = ReportUtils.isExpenseReport(this.props.iouReport);
const paymentCardList = this.props.fundList || {};

Expand All @@ -110,7 +112,7 @@ class KYCWall extends React.Component {
(!isExpenseReport && !PaymentUtils.hasExpensifyPaymentMethod(paymentCardList, this.props.bankAccountList))
) {
Log.info('[KYC Wallet] User does not have valid payment method');
const clickedElementLocation = getClickedTargetLocation(event.nativeEvent.target);
const clickedElementLocation = getClickedTargetLocation(targetElement);
const position = this.getAnchorPosition(clickedElementLocation);
this.setPositionAddPaymentMenu(position);
this.setState({
Expand Down Expand Up @@ -142,6 +144,7 @@ class KYCWall extends React.Component {
vertical: this.state.anchorPositionVertical,
horizontal: this.state.anchorPositionHorizontal,
}}
anchorAlignment={this.props.anchorAlignment}
onItemSelected={(item) => {
this.setState({shouldShowAddPaymentMenu: false});
if (item === CONST.PAYMENT_METHODS.BANK_ACCOUNT) {
Expand Down
16 changes: 12 additions & 4 deletions src/components/KYCWall/kycWallPropTypes.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import _ from 'underscore';
import PropTypes from 'prop-types';
import userWalletPropTypes from '../../pages/EnablePayments/userWalletPropTypes';
import bankAccountPropTypes from '../bankAccountPropTypes';
import cardPropTypes from '../cardPropTypes';
import iouReportPropTypes from '../../pages/iouReportPropTypes';
import reimbursementAccountPropTypes from '../../pages/ReimbursementAccount/ReimbursementAccountDraftPropTypes';
import CONST from '../../CONST';

const propTypes = {
/** Route for the Add Bank Account screen for a given navigation stack */
Expand All @@ -15,9 +17,6 @@ const propTypes = {
/** Route for the KYC enable payments screen for a given navigation stack */
enablePaymentsRoute: PropTypes.string.isRequired,

/** Where to place the popover */
popoverPlacement: PropTypes.string,

/** Listen for window resize event on web and desktop */
shouldListenForResize: PropTypes.bool,

Expand All @@ -44,11 +43,16 @@ const propTypes = {

/** The reimbursement account linked to the Workspace */
reimbursementAccount: reimbursementAccountPropTypes,

/** Where the popover should be positioned relative to the anchor points. */
anchorAlignment: PropTypes.shape({
horizontal: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL)),
vertical: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_VERTICAL)),
}),
};

const defaultProps = {
userWallet: {},
popoverPlacement: 'top',
shouldListenForResize: false,
isDisabled: false,
chatReportID: '',
Expand All @@ -58,6 +62,10 @@ const defaultProps = {
reimbursementAccount: {},
addDebitCardRoute: '',
iouReport: {},
anchorAlignment: {
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
},
};

export {propTypes, defaultProps};
4 changes: 4 additions & 0 deletions src/components/MoneyReportHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ function MoneyReportHeader({session, personalDetails, policy, chatReport, report
shouldShowPaymentOptions
style={[styles.pv2]}
formattedAmount={formattedAmount}
anchorAlignment={{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In header button, popover should open below button (anchor should be at top)

horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP,
}}
/>
</View>
)}
Expand Down
4 changes: 4 additions & 0 deletions src/components/ReportActionItem/ReportPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ function ReportPreview(props) {
enablePaymentsRoute={ROUTES.ENABLE_PAYMENTS}
addBankAccountRoute={bankAccountRoute}
style={[styles.requestPreviewBox]}
anchorAlignment={{
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In ReportPreview, popover should open above button so anchor is bottom.

}}
/>
)}
</View>
Expand Down
1 change: 1 addition & 0 deletions src/components/SettlementButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ function SettlementButton({
isDisabled={isOffline}
chatReportID={chatReportID}
iouReport={iouReport}
anchorAlignment={anchorAlignment}
>
{(triggerKYCFlow, buttonRef) => (
<ButtonWithDropdownMenu
Expand Down
5 changes: 4 additions & 1 deletion src/pages/settings/Wallet/WalletPage/BaseWalletPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,10 @@ function BaseWalletPage(props) {
enablePaymentsRoute={ROUTES.SETTINGS_ENABLE_PAYMENTS}
addBankAccountRoute={ROUTES.SETTINGS_ADD_BANK_ACCOUNT}
addDebitCardRoute={ROUTES.SETTINGS_ADD_DEBIT_CARD}
popoverPlacement="bottom"
anchorAlignment={{
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

popover should open below button, so vertical anchor position is top.

}}
>
{(triggerKYCFlow, buttonRef) => (
<MenuItem
Expand Down
Loading