Make AddPaymentMenu dynamic

[kevinksullivan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Login to NewDot on web
2. Select profile > payments
3. Select add payment method
4. Change screen width

Expected Result:

The payment method dropdown should show move along with the rest of the screen

Actual Result:

The payment method dropdown is in a fixed position on the screen.

Platform:

Web

Reproducible in staging?: Yes
Reproducible in production?: Tes

Notes/Photos/Videos:

http://g.recordit.co/PSG52y2iA6.gif

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/191203

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @alexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[MelvinBot]

Triggered auto assignment to @MariaHCD (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MelvinBot]

Triggered auto assignment to @laurenreidexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[parasharrajat]

cc: @mateusbra

[mateusbra]

@parasharrajat working on that, I'll try to post proposal soon thank you!

[laurenreidexpensify]

https://www.upwork.com/jobs/~014fa34661ee0b416d

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (Exported)

[MelvinBot]

Triggered auto assignment to @deetergp (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[phivh]

Proposal
Local at src/pages/settings/Payments/PaymentsPage.js - line 104

1. Add window event resize to re-calculate position of Add Payment Button

paymentMethodPressed(nativeEvent, accountType, account) {
        window.addEventListener('resize', () => {
            const resizePosition = getClickedElementLocation(nativeEvent);
            this.setState({
                shouldShowAddPaymentMenu: true,
                anchorPositionTop: resizePosition.bottom,

                // We want the position to be 13px to the right of the left border
                anchorPositionLeft: resizePosition.left + 13,
            });
        });
}

2. Remove event when method list close

3. The issue also affects to Transfer Balance of Payment page, so i will apply this solution to it as well if the proposal acceptable.

Demo:

2022-01-28_17-38-58.mp4

[rushatgabhane]

@phivh thanks for your proposal, but can you help me test this out as I'm getting an error on Chrome.

let resizePosition = getClickedElementLocation(nativeEvent);

window.addEventListener('resize', () => {
    resizePosition = getClickedElementLocation(nativeEvent); // error here
});

error: Cannot read properties of null (reading 'getBoundingClientRect') at getClickedElementLocation

[phivh]

@rushatgabhane not sure why you got this. The event should call right after the button click action then it will assign nativeEvent element right after that. It should not null. You can check the video attached.

2022-01-29_10-29-11.mp4

Maybe we need to remove listener event after closing the modal as the second point on my proposal.

/**
     * Hide the add payment modal
     */
    hideAddPaymentMenu() {
        window.removeEventListener('resize', null); <===== Add this 
        this.setState({shouldShowAddPaymentMenu: false});
    }

[rushatgabhane]

@phivh Really appreciate it! I followed the steps, but the menu isn't dynamic :(

Here is my diff - https://github.com/rushatgabhane/Expensify-App/commit/3f4c1e946edad6c8b14ffc088e4af32ef93f9293 please let me know what I'm doing wrong :)

[deetergp]

Thanks @rushatgabhane, @phivh's proposal looks good to me as well.

[phivh]

Thanks you both. I will submit the PR soon.

[laurenreidexpensify]

@phivh can you apply to the post in Upwork so I can hire you? Thanks! (https://www.upwork.com/ab/applicants/1486714447462305792/job-details)

[MelvinBot]

📣 @phivh You have been assigned to this job by @laurenreidexpensify!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[phivh]

@laurenreidexpensify applied for the job on Upwork. (https://www.upwork.com/jobs/~014fa34661ee0b416d)
The PR will be submitted before 02/05.

[phivh]

Hi @deetergp @rushatgabhane I'm going to submit the PR, just want to make sure if we are able to wrap the condition by using Platform below instead of creating a new .native. file.

if(Platform.OS === 'web') {
    window.addEventListener('resize', () => {
        ....
    });
}

[rushatgabhane]

@phivh let's create seperate files. That's the standard we follow https://github.com/Expensify/App#platform-specific-file-extensions

[phivh]

Hi @rushatgabhane @deetergp The PR just submitted.

Hi @laurenreidexpensify , I applied for the Upwork job. Could you please re-check if there is something wrong?

[phivh]

@deetergp @laurenreidexpensify The PR nearly merge. I'm still have not got hired on Upwork. Could you please check?

Job applied: https://www.upwork.com/jobs/~014fa34661ee0b416d

[ghost]

I do not know. I didn't get any news from Upwork.

[laurenreidexpensify]

@phivh all sorted 👍🏽 hired in upwork

[deetergp]

Not overdue, code review is ongoing. Should have added the label to ward off @MelvinBot's passive aggression.

[MelvinBot]

This issue has not been updated in over 15 days. @deetergp, @phivh, @rushatgabhane, @laurenreidexpensify eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[rushatgabhane]

PR in staging

[phivh]

@laurenreidexpensify do we need to wait for regression on this? it seems deployed to production 8 days.
#7847 (comment)

[phivh]

@deetergp @laurenreidexpensify is there anything we need to wait for the payment? It seems we missing around 13 days for this task.

[rushatgabhane]

@laurenreidexpensify bump ^^ #7385 (comment)

[laurenreidexpensify]

oh weird some of the automation didn'tn take effect on this so I missed it. Looking into it now.

[laurenreidexpensify]

Sorry @rushatgabhane @phivh something went really wrong here in the automation alerts for our team. I've paid this now. Thanks for your patience!