[PAY TALHA] [HOLD for payment 2023-09-27] [$1000] ANDROID CHROME - Split Bill: LHN does not close if clicking on the Workspaces link

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to https://help.expensify.com/articles/playbooks/Expensify-Chat-Playbook-for-Conferences on mobile device.
2. Open LHN and click on any step.
3. Notice the LHN closes and page scrolls down to relevant section.
4. Now go to https://help.expensify.com/hubs/split-bills.
5. Open LHN and click on Workspaces.
6. Notice the LHN does not close and nothing happens.

Expected Result:

In case of https://help.expensify.com/hubs/split-bill, clicking on links in the LHN which points to some section on the same page, the LHN should close and the screen should be scrolled down to relevant section.

Actual Result:

In case of https://help.expensify.com/hubs/split-bill, clicking on links in the LHN which points to some section on the same page, the LHN did not close.

Note: This seems to occur on the Workspaces link (the other 3 links seem fine).

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.47-2
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

WhatsApp.Video.2023-07-27.at.2.05.00.PM.mp4

az_recorder_20230728_131126.1.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Talha345
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690460581911079

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b88583567474c0d1
• Upwork Job ID: 1685897339918934016
• Last Price Increase: 2023-09-11
• Automatic offers:
• abdulrahuman5196 | Reviewer | 26631441
• Talha345 | Contributor | 26631442
• Talha345 | Reporter | 26631443

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[Talha345]

Proposal

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

In help docs,the LHN does not close when clicking on some link that points to some section on the same page.This issue is only present on https://help.expensify.com/hubs/split-bills while other pages like https://help.expensify.com/articles/playbooks/Expensify-Chat-Playbook-for-Conferences do not have this issue.

What is the root cause of that problem?

The RC is that there is no implementation to take care of this issue on this page.The issue does not exist on other pages because the same page links in the LHN on other pages are populated using the following code. Here we can clearly see that toggleHeaderMenu function is called whenever a link is clicked which was populated using tocbot.

Let us consider the following 2 links:

1. https://help.expensify.com/hubs/split-bills
2. https://help.expensify.com/articles/playbooks/Expensify-Chat-Playbook-for-Conferences

If we open both of these links on a mobile device and open the sidebar we see some links under the selected link.See images below.

Now if you try to click on any of the sub links in 2nd image with the Step 1/2 etc, you will see that the sidebar closes and the page is scrolled down to the correct section whereas this behaviour is not present in the links in the 1st image specifically "Paying Friends" and "Workspaces".

The function toggleHeaderMenu is responsible for closing the sidebar on mobile devices if a link is clicked which points to some section on the same page.

The reason, the sublinks in the link 2 mentioned above work correctly is because those sublinks were populated via a JS library called TocBot which is used to create Table of Contents.This can be seen here.If you look, we are calling the toggleHeaderMenu function on the onclick event within this codeblock. This means that whenever a link in the sidebar which is populated using TocBot is clicked,the toggleHeaderMenu function is executed which closes the sidebar on mobile devices.

Now the reason why we still have this issue with sublinks on link 1 above is due to the fact that these sublinks were not populated using TocBot and the reason for this is that these sublinks are not TOC.For example, the links "Paying Friends" and "Workspaces" have href pointing to some section of the same page whereas the links "Request And Split Bills" and "The Free Plan" are links to some other pages.Since the function toggleHeaderMenu is not called on the click of "Paying Friends" and "Workspaces", this issue occurs.

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

There are 2 solutions to fix this issue:

1. We implement a generic JS in the main.js file to take care of all navigation links in the help docs LHN. That can be done as follows:

function isSamePageScrollLink(link) {
        const href = link.getAttribute("href");
        if (href.startsWith("#") && !!document.getElementById(href.slice(1))) {
            toggleHeaderMenu();
        }
    }
    
   const lhnContent = document.getElementById('lhn-content');
    lhnContent.addEventListener('click', (event) => {
        const clickedLink = event.target;
        if (clickedLink) {
            isSamePageScrollLink(clickedLink);
        }
    });

Using this we add an event listener to any link in the LHN and upon click we can check if the href attribute of a particular link points to some section of the same page using isSamePageScrollLink and if so we trigger toggleHeaderMenu.

Furthermore, this would require to remove the toggleHeaderMenu method call in the TocBot onclick event here otherwise it will be called twice for TocBot links. This way we will have a generic code that deals with all navigation links in the LHN help docs.

2. The second solution would be to just implement the sublinks on https://help.expensify.com/hubs/split-bills using Tocbot but this does not make much sense imo because we can see that there are sublinks below 'Paying Friends' and 'Workspaces' links which are not actually links to the same page and redirect to some other page and therefore are not necessarily TOC.

What alternative solutions did you explore? (Optional)

N/A

[kadiealexander]

Swapping with JLi (we have an agreement to swap android/iOS issues)

[jliexpensify]

I can verify this is a bug, but only for the Workspaces link. I would encourage more testing though, this is what I'm seeing on a Pixel 3a.

[melvin-bot]

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

[melvin-bot]

Current assignee @jliexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[Talha345]

@jliexpensify This bug exists only for Paying Friends and Workspaces links because both of these links have href attributes pointing to the same page. Also I see that you added android chrome to the task title but this issue is also on IOS safari on mobile devices because we do not have any implementation to cater this.You can check my proposal for further details.

[abdulrahuman5196]

Reviewing today

[Talha345]

@abdulrahuman5196 did you get a chance to review this?

[jliexpensify]

@Talha345 thanks for the context - just as an FYI, the GH's are original created by the QA team based off the reporter's findings. So if it's also an iOS problem, I'd encourage you to add that in your initial report so it can be recorded.

[abdulrahuman5196]

@jliexpensify Are we actually fine to report and to work on the help pages? Asking this because, I haven't personally seen issues raised on help pages. I could be wrong as well, but wanted to confirm

[Talha345]

s raised on help pages. I could b

@abdulrahuman5196 The code base for the help docs is in the same repository and I have actually worked on an issue within the help docs before too.Here is the link for that issue: #22014

[jliexpensify]

Yep @abdulrahuman5196 I'm pretty sure we can hire Contributors to work on the Help pages. I would say find a proposal that works and we'll get an Engineer assigned, and they can have the final say!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[jliexpensify]

@abdulrahuman5196 how's @Talha345 's proposal looking?

[Talha345]

Hi @abdulrahuman5196,do you have any update as this is a pretty straightforward task in my opinion?

[Talha345]

@tgolen @abdulrahuman5196 PR is ready for review #27261

[jliexpensify]

@Talha345 I always post a payments summary before paying, so you can verify the rates then!

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @Talha345 got assigned: 2023-09-12 13:36:18 Z
• when the PR got merged: 2023-09-15 17:05:08 UTC
• days elapsed: 3

On to the next one 🚀

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.71-12 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix LHN toggle in responsive view! #27261

If no regressions arise, payment will be issued on 2023-09-27. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @Talha345 requires payment offer (Contributor)
• @abdulrahuman5196 requires payment offer (Reviewer)
• @Talha345 requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR:
• [@abdulrahuman5196] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@abdulrahuman5196] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@abdulrahuman5196] Determine if we should create a regression test for this bug.
• [@abdulrahuman5196] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jliexpensify]

Payment Summary (is this right? It seems like it was merged in 3 days)

• Bug Reporter / Contributor: @Talha345 $250 + $1000 + $500 (speed bonus)
• C+: @abdulrahuman5196: $1000 + $500 (speed bonus)

Is this correct?

New Upworks job.

[Talha345]

Payment Summary (is this right? It seems like it was merged in 3 days)

• Bug Reporter / Contributor: @Talha345 $250 + $1000 + $500 (speed bonus)
• C+: @abdulrahuman5196: $1000 + $500 (speed bonus)

Is this correct?

New Upworks job.

@jliexpensify Looks correct! Do I have to apply to the new Upwork job or will you be sending an offer automatically?

[jliexpensify]

Offers have been sent!

[Talha345]

Offers have been sent!

@jliexpensify Yes the offers were sent automatically on the old Upwork job and for this new Upwork job that you just linked, I simply got an invitation to submit a proposal but no offer.

[jliexpensify]

Reminder for @abdulrahuman5196 to complete the checklist

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Not a regression. Seems to be the behaviour from beginning.

Determine if we should create a regression test for this bug.
If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

No. This is a minor case and wouldn't be beneficial to add tests.

[abdulrahuman5196]

@jliexpensify Added checklist and submitted proposal on the upwork new job

[jliexpensify]

Thanks @abdulrahuman5196 have hired you, just waiting on acceptance

[abdulrahuman5196]

@jliexpensify accepted the offer

[jliexpensify]

Paid out @abdulrahuman5196 - there was a very strange issue with your contract @Talha345 : can you confirm that you've not received any payment?

[jliexpensify]

@Talha345 I see you ended a contract today. This is a bit confusing - can I confirm if you have been paid?

[Talha345]

Paid out @abdulrahuman5196 - there was a very strange issue with your contract @Talha345 : can you confirm that you've not received any payment?

@jliexpensify I have not received any payment, however I received a latest offer from you which I have accepted but I had now 3 different contracts for this same issue.

I have therefore ended the 2 previous contracts without any payment.Now you can pay me with the open contract.

[jliexpensify]

Thanks for clarifying - paid and job closed.