Start chat - App crashes when pressing keyboard up key in Start chat list

[izarutskaya]

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.63-0
Reproducible in staging?: Y
Reproducible in production?: N
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to FAB > Start chat.
3. Press keyboard down key.
4. Press keyboard up key.

Expected Result:

App does not crash.

Actual Result:

App crashes.

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

Bug6453279_1713425262908.keyboard_crash.mp4

Bug6453279_1713425262915!crash.txt

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @danieldoglas (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

Triggered auto assignment to @anmurali (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.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[izarutskaya]

@anmurali 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.

[izarutskaya]

We think this issue might be related to the #collect project.

[izarutskaya]

Production

bandicam.2024-04-18.10-55-11-411.mp4

[ShridharGoel]

Proposal

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

App crashes on cyclic traversal in big lists.

What is the root cause of that problem?

This happens only for big lists because the whole list is not loaded when the user tries to go to the bottom most item.

This started happening since #39201 because the below logic was added.

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 228 to 237 in 9b839f4

	// If `initiallyFocusedOptionKey` is not passed, we fall back to `-1`, to avoid showing the highlight on the first member
	const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({
	initialFocusedIndex: flattenedSections.allOptions.findIndex((option) => option.keyForList === initiallyFocusedOptionKey),
	maxIndex: flattenedSections.allOptions.length - 1,
	isActive: true,
	onFocusedIndexChange: (index: number) => {
	scrollToIndex(index, true);
	},
	isFocused,
	});

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

We should use cyclic traversal only when length of list <= 500 (because we show 500 items at the start, along with a "Show more" option at the bottom).

We can add disableCyclicTraversal: flattenedSections.allOptions.length > 500 in the below:

const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({
        initialFocusedIndex: flattenedSections.allOptions.findIndex((option) => option.keyForList === initiallyFocusedOptionKey),
        maxIndex: flattenedSections.allOptions.length - 1,
        isActive: true,
        disableCyclicTraversal: flattenedSections.allOptions.length > 500,
        onFocusedIndexChange: (index: number) => {
            scrollToIndex(index, true);
        },
        isFocused,
    });

This is in BaseSelectionList.

We can check for a similar issue in BaseOptionsList and update there as well.

[danieldoglas]

@ShridharGoel thanks for pointing out where the issue came from. Your proposal was great, but in cases like this (where the issue was caused by a PR that was recently deployed < 7 days ago), it is expected that the contributor who originally did the changes fixes the regressions as part of the original issue. I suggest you look for issues that have Help Wanted labels, where you would be assigned to a situation like this.

@WojtekBoman this is a regression from your PR that was merged deployed 7h ago. Please look into it as well!

cc @roryabraham

[WojtekBoman]

I'll prepare a fix for that

[WojtekBoman]

The PR with the fix for this issue is ready for review

[roryabraham]

fix is being CP'd here

[roryabraham]

confirmed this is fixed on staging, no payments due, closing this out