[HOLD for payment 2024-04-09] [$500] Search/Send money/Request money - Last option is not selected when pressing up arrow key

[lanitochka17]

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

---

Version Number: 1.4.53-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: https://expensify.testrail.io/index.php?/tests/view/4428031
Issue reported by: Applause - Internal Team

Action Performed:

1. Log in to New Expensify with Expensifail account
2. Click on search
3. Press up arrow key on keyboard

Expected Result:

Last option in the list should be selected and the list should scroll down to the bottom as it is the case in Start chat list

Actual Result:

List does not move at all
Reproducible in Search, Send money and Request money lists

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

Bug6415407_1710535393957.2024-03-15_21-39-36.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0121c95e267ea04cc2
• Upwork Job ID: 1768768968790626304
• Last Price Increase: 2024-03-15
• Automatic offers:
• mollfpr | Reviewer | 0
• abzokhattab | Contributor | 0

[melvin-bot]

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

[lanitochka17]

@bfitzexpensify 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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0121c95e267ea04cc2

[melvin-bot]

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

[abzokhattab]

Proposal

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

Last option is not selected when pressing up arrow key

What is the root cause of that problem?

The root cause of this issue lies in the calculation of maxIndex within the SelectionList. The maxIndex is set to the total length of flattenedSections.allOptions, representing all available options. This approach does not account for the actual number of options displayed to the user, which is limited to the first 500 until the user opts to click on show more.

App/src/components/SelectionList/BaseSelectionList.tsx

Line 466 in 3d51305

maxIndex={flattenedSections.allOptions.length - 1}

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

Adjust maxIndex to reflect the count of currently displayed options. This change ensures keyboard navigation correctly accounts for the visible options limit: we can do that by:

• flattening the slicedSections array and get the length of the data inside each section of the slicedSections:

            maxIndex={slicedSections.flatMap((section) => section.data).length - 1}

• or alternatively use the flattenedSections.allOptions and get only the visible items

            maxIndex={flattenedSections.allOptions.slice(0, CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * currentPage).length - 1}

• or we can create a new property in the flattenedSections function as slicedOptions and it will resolve to the value above in case we will need it in other parts in the file.

...

Additionally, the current behavior of slicing options is buggy, as it improperly applies the 500-option limit per section rather than cumulatively. We need to amend this by iterating through sections and slicing options until reaching the global limit (500):

    const [slicedSections, ShowMoreButtonInstance] = useMemo(() => {
        let maxRemainingOptions = CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * currentPage;
        const processedSections = sections.map((section) => {
            const data = !isEmpty(section.data) && maxRemainingOptions > 0 ? section.data.slice(0, maxRemainingOptions) : [];
            maxRemainingOptions -= data.length;

            return {
                ...section,
                data,
            };
        });

        // Determine if the "Show More" button should be displayed.
        const shouldShowMoreButton = flattenedSections.allOptions.length > CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * currentPage;
        const showMoreButton = shouldShowMoreButton ? (
            <ShowMoreButton
                containerStyle={[styles.mt2, styles.mb5]}
                currentCount={CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * currentPage}
                totalCount={flattenedSections.allOptions.length}
                onPress={incrementPage}
            />
        ) : null;
        return [processedSections, showMoreButton];
        // we don't need to add styles here as they change
        // we don't need to add flattendedSections here as they will change along with sections
        // eslint-disable-next-line react-hooks/exhaustive-deps
    }, [sections, currentPage]);

[allgandalf]

I am not able to reproduce this bug, are you able to reproduce @abzokhattab ?

[abzokhattab]

@GandalfGwaihir Yeah but you need to have more than 500 result in the search list:

• Here is the issue:

Screen.Recording.2024-03-16.at.1.14.03.AM.mov

• Here is the behavior after using maxIndex={flattenedSections.allOptions.slice(0, CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * currentPage).length - 1} :

Screen.Recording.2024-03-16.at.1.25.00.AM.mov

• and here is the behaviour after modifying the slicing function:

Screen.Recording.2024-03-16.at.1.30.19.AM.mov

[bfitzexpensify]

Proposals ready for review @mollfpr - thank you!

[mollfpr]

I can't reproduce it on staging v1.4.54-1 and production v1.4.53-2.

Screen.Recording.2024-03-19.at.23.31.20.mp4

[abzokhattab]

Still reproducible on staging and production on my account:

Screen.Recording.2024-03-19.at.6.37.02.PM.mov

Screen.Recording.2024-03-19.at.6.37.56.PM.mov

[mollfpr]

I guess 700+ reports don't reproduce the issue.

@abzokhattab How do you get the account with 4k report?

nvm I can reproduce it in my main account

[abzokhattab]

@abzokhattab How do you get the account with 4k report?

I don't know but this is the main account 😂

I guess 700+ reports don't reproduce the issue.

seems like it, but the code has a bug, I believe we should fix it.

[mollfpr]

@abzokhattab I tried your solution It works on my account with 4k reports but it is not with 700+ reports.

Screen.Recording.2024-03-20.at.23.24.14.mp4

[abzokhattab]

could you try again please, there was a small issue in the slicedSection function. please check the updated function

[mollfpr]

Thanks @abzokhattab it's working great now!

The proposal from @abzokhattab looks good to me!

🎀 👀 🎀 C+ reviewed!

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @abzokhattab 🎉 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 📖

[abzokhattab]

The PR is ready! please check it out and let me know if you have any comments.

[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.4.58-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix/last option is not selected when pressing up arrow key #38886

If no regressions arise, payment will be issued on 2024-04-09. 🎊

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

• @mollfpr requires payment automatic offer (Reviewer)
• @abzokhattab requires payment automatic offer (Contributor)

[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:

• [@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
• [@mollfpr] 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:
• [@mollfpr] 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:
• [@mollfpr] Determine if we should create a regression test for this bug.
• [@mollfpr] 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.
• [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bfitzexpensify]

Payment summary:

$500 to @abzokhattab for contributor work - paid via Upwork ✅
$500 to @mollfpr for C+ work - @mollfpr, can you please accept the Upwork job offer, and also complete the BZ checklist? Thanks!

[mollfpr]

Sorry for the delay @bfitzexpensify! I'll do a manual request in NewDot, thank you!

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] 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:

I couldn't find the offending PR.

[@mollfpr] 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:

The regression step should be good.

[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] 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.

1. Open App
2. Go to the search page
3. Press up arrow key on the keyboard
4. Ensure that the last item in the list is selected
5. 👍 or 👎

[JmillsExpensify]

$500 approved for @mollfpr

[bfitzexpensify]

Thanks @mollfpr - agree with those regression steps, proposed they be added via https://github.com/Expensify/Expensify/issues/387069. Closing this out - thanks everyone!