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

[HOLD for payment 2024-05-16] [$250] mWeb - BA - Buttons are not visible at first glance on Onfido terms page #40941

Closed
1 of 6 tasks
lanitochka17 opened this issue Apr 24, 2024 · 19 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Apr 24, 2024

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


Version Number: 1.4.65-4
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4507599
Email or phone of affected tester (no customers): applausetester+vd_mweb042424@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

Pre-requisite: the user must be logged in, have created a collect Workspace and have enabled Workflows

  1. Go to Workspace > Workflows > Connect bank account
  2. Select "Connect manually" and use the testing routing number and account number
  3. On Personal info page, enter "First" and "Last" for first name and last name
  4. Enter any other information on the rest of the Personal info pages and confirm
  5. On the Onfido terms page, verify the "Accept" and "Do not accept" buttons are not visible at first glance
  6. Scroll down the text to the bottom, verify that the buttons are not visible yet
  7. Scroll down the page by scrolling from the bottom of the page (under the text)
  8. Verify the buttons are now visible.

Expected Result:

The buttons should be completely visible at first glance

Actual Result:

The buttons are hidden until the user scrolls down the page

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6460586_1713981059185.Msls9407_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a29e77d6361f0ee0
  • Upwork Job ID: 1783556114373173248
  • Last Price Increase: 2024-04-25
  • Automatic offers:
    • samilabud | Contributor | 0
Issue OwnerCurrent Issue Owner: @garrettmknight
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 24, 2024
Copy link

melvin-bot bot commented Apr 24, 2024

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@lanitochka17
Copy link
Author

@garrettmknight FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@garrettmknight
Copy link
Contributor

Reproduced - I think this is an important bug to fix to enable users to connect bank accounts.

@garrettmknight garrettmknight added the External Added to denote the issue can be worked on by a contributor label Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01a29e77d6361f0ee0

@melvin-bot melvin-bot bot changed the title mWeb - BA - Buttons are not visible at first glance on Onfido terms page [$250] mWeb - BA - Buttons are not visible at first glance on Onfido terms page Apr 25, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

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

@skyweb331
Copy link
Contributor

I tried to reproduce but for me, Step 3 is not ONFIDO, how can I see it?

@samilabud
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

mWeb - BA - Buttons are not visible at first glance on Onfido terms page

What is the root cause of that problem?

We are using embedded content (OnFido) to load the terms, this comes with iframes that are out of our control, the first iframe comes with a height of 100% this is the root cause of why the content of the first iframe (the terms) is the only visible thing here, this is the cause of the double scroll bar.

What changes do you think we should make in order to solve the problem?

We should use Javascript to get the viewPortHeight when the page load, later select the first iframe (the terms content) and specify the 80% of the getted height and set this one to the mentioned iframe, this modification should be done in Onfido component inside the useEffect that already exist, like:

useEffect(() => {
        const onfidoOut = baseOnfidoRef.current?.onfidoOut;

        const observer = new MutationObserver(() => {
            const fidoRef = baseOnfidoRef.current;
            if (fidoRef) {
                // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
                const onfidoSdk = fidoRef.querySelector('#onfido-sdk > iframe') as HTMLElement | null;
                if (onfidoSdk) {
                    const viewportHeight = window.innerHeight; // Get the viewport height
                    const desiredHeight = viewportHeight * 0.8;
                    onfidoSdk.style.height = `${desiredHeight}px`;
                }
            }
        });
        if (baseOnfidoRef.current) {
            observer.observe(baseOnfidoRef.current, {attributes: false, childList: true, subtree: true});
        }

        if (!onfidoOut) {
            return;
        }

        onfidoOut.tearDown();

        // Clean up function to remove the observer when component unmounts
        return () => {
            observer.disconnect();
        };
    }, []);

Below some tests:
Android mWeb test:

Android.chrome.test.mp4

Mac Chrome test:

mac.chrome.test.mp4

@melvin-bot melvin-bot bot added the Overdue label Apr 29, 2024
@mananjadhav
Copy link
Collaborator

I think @samilabud's proposal will work. What I am not sure is should we be using querySelector this way.

🎀 👀 🎀 C+ reviewed.

@melvin-bot melvin-bot bot removed the Overdue label Apr 29, 2024
Copy link

melvin-bot bot commented Apr 29, 2024

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

@garrettmknight
Copy link
Contributor

@mananjadhav can you give this proposal a review when you get a chance?

@luacmartins
Copy link
Contributor

Solution LGTM.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 29, 2024
Copy link

melvin-bot bot commented Apr 29, 2024

📣 @samilabud 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Apr 29, 2024
@garrettmknight garrettmknight removed the Reviewing Has a PR in review label May 13, 2024
@melvin-bot melvin-bot bot added the Overdue label May 13, 2024
@garrettmknight garrettmknight added the Awaiting Payment Auto-added when associated PR is deployed to production label May 13, 2024
@garrettmknight
Copy link
Contributor

Went to prod 5 days ago, updating.

@melvin-bot melvin-bot bot removed the Overdue label May 13, 2024
@garrettmknight garrettmknight changed the title [$250] mWeb - BA - Buttons are not visible at first glance on Onfido terms page [HOLD for payment 2024-05-16] [$250] mWeb - BA - Buttons are not visible at first glance on Onfido terms page May 13, 2024
@garrettmknight
Copy link
Contributor

garrettmknight commented May 16, 2024

Payment Summary:

@mananjadhav
Copy link
Collaborator

@garrettmknight I'll be raising my request on NewDot. Just the payout summary will suffice.

@garrettmknight
Copy link
Contributor

Sounds good - can you complete the checklist before raising the request? Thanks!

@mananjadhav
Copy link
Collaborator

mananjadhav commented May 16, 2024

Sure @garrettmknight.

We don't have any offending PR for this one. This was due to onFido an external dependency and how iframe layouts work.

Assuming that this test already exists (I can see this is attached https://expensify.testrail.io/index.php?/tests/view/4507599 in the issue body), I don't think we need a regression test for this one.

@JmillsExpensify
Copy link

Based on the above, $250 approved for @mananjadhav

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 19, 2024
@garrettmknight
Copy link
Contributor

All good, closing

@github-project-automation github-project-automation bot moved this from Release 1: Spring 2024 (May) to Done in [#whatsnext] #wave-collect May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Archived in project
Development

No branches or pull requests

7 participants