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.

Sign up for GitHub

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

[CP Staging] Fix send button popover position #29361

Merged
merged 6 commits into from
Oct 22, 2023
Merged

[CP Staging] Fix send button popover position #29361

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c2283d9
Fix send button popover position
Nikhil-Vats Oct 11, 2023
9564f49
Fix lint error
Nikhil-Vats Oct 11, 2023
dbe041c
Merge branch 'main' into 29332_send_payment_popover
Nikhil-Vats Oct 11, 2023
b7fc294
Merge branch 'main' into 29332_send_payment_popover
Nikhil-Vats Oct 13, 2023
aaeef6f
Merge branch 'main' into 29332_send_payment_popover
Nikhil-Vats Oct 16, 2023
a2bdeea
Merge branch 'main' into pr/29361
thienlnam Oct 22, 2023
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
4 changes: 0 additions & 4 deletions src/components/MoneyReportHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ function MoneyReportHeader({session, personalDetails, policy, chatReport, report
shouldShowPaymentOptions
style={[styles.pv2]}
formattedAmount={formattedAmount}
anchorAlignment={{
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP,
}}
Comment on lines -124 to -127
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't need to pass props unnecessarily as the default props are enough here

/>
</View>
)}
Expand Down
6 changes: 5 additions & 1 deletion src/components/MoneyRequestConfirmationList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,11 @@ function MoneyRequestConfirmationList(props) {
policyID={props.policyID}
shouldShowPaymentOptions
buttonSize={CONST.DROPDOWN_BUTTON_SIZE.LARGE}
anchorAlignment={{
kycWallAnchorAlignment={{
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
}}
paymentMethodDropdownAnchorAlignment={{
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.RIGHT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
}}
Comment on lines +509 to 516
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this button is at bottom of page so vertical anchor alignment is BOTTOM (popover above button). the horizontal anchor alignment for second one is RIGHT because caret icon is at right

Expand Down
6 changes: 5 additions & 1 deletion src/components/ReportActionItem/ReportPreview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,14 @@ function ReportPreview(props) {
enablePaymentsRoute={ROUTES.ENABLE_PAYMENTS}
addBankAccountRoute={bankAccountRoute}
style={[styles.requestPreviewBox]}
anchorAlignment={{
kycWallAnchorAlignment={{
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
}}
paymentMethodDropdownAnchorAlignment={{
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.RIGHT,
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.

same reason as above

/>
)}
{shouldShowSubmitButton && (
Expand Down
25 changes: 18 additions & 7 deletions src/components/SettlementButton.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,14 @@ const propTypes = {
/** Whether we should show a loading state for the main button */
isLoading: PropTypes.bool,

/** The anchor alignment of the popover menu */
anchorAlignment: PropTypes.shape({
/** The anchor alignment of the popover menu for payment method dropdown */
paymentMethodDropdownAnchorAlignment: PropTypes.shape({
horizontal: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL)),
vertical: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_VERTICAL)),
}),

/** The anchor alignment of the popover menu for KYC wall popover */
kycWallAnchorAlignment: PropTypes.shape({
horizontal: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL)),
vertical: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_VERTICAL)),
}),
Expand All @@ -90,16 +96,21 @@ const defaultProps = {
policyID: '',
formattedAmount: '',
buttonSize: CONST.DROPDOWN_BUTTON_SIZE.MEDIUM,
anchorAlignment: {
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.RIGHT,
kycWallAnchorAlignment: {
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT, // button is at left, so horizontal anchor is at LEFT
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP, // we assume that popover menu opens below the button, anchor is at TOP
},
paymentMethodDropdownAnchorAlignment: {
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.RIGHT, // caret for dropdown is at right, so horizontal anchor is at RIGHT
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 we need to have different props for both because for dropdown button (payment method options), caret is aligned at right so horizontal anchor should be right.

For button, we need the horizontal anchor alignment to be LEFT as the button is aligned towards left.

vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP, // we assume that popover menu opens below the button, anchor is at TOP
},
};

function SettlementButton({
addDebitCardRoute,
addBankAccountRoute,
anchorAlignment,
kycWallAnchorAlignment,
paymentMethodDropdownAnchorAlignment,
Comment on lines +118 to +119
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 if you have any better prop name suggestions, please let me know

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine with those namings

betas,
buttonSize,
chatReportID,
Expand Down Expand Up @@ -201,7 +212,7 @@ function SettlementButton({
isDisabled={isOffline}
chatReportID={chatReportID}
iouReport={iouReport}
anchorAlignment={anchorAlignment}
anchorAlignment={kycWallAnchorAlignment}
>
{(triggerKYCFlow, buttonRef) => (
<ButtonWithDropdownMenu
Expand All @@ -212,7 +223,7 @@ function SettlementButton({
options={paymentButtonOptions}
style={style}
buttonSize={buttonSize}
anchorAlignment={anchorAlignment}
anchorAlignment={paymentMethodDropdownAnchorAlignment}
/>
)}
</KYCWall>
Expand Down
Loading