[HOLD for payment 2025-01-18] [$250] Categories - Category list moves with up and down key when RHP Is opened

[vincdargento]

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: v9.0.78-5
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Categories.
3. Open any category RHP.
4. Press keyboard up or down key.

Expected Result:

Category list will not move with up and down key when RHP is opened (as in PR #39327)

Actual Result:

Category list moves with up and down key when RHP is opened.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021874222072625755779
• Upwork Job ID: 1874222072625755779
• Last Price Increase: 2024-12-31
• Automatic offers:
• QichenZhu | Contributor | 105551234

Issue Owner

Current Issue Owner: @anmurali

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

[QichenZhu]

Proposal

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

The categories list is scrollable with the arrow keys when the RHP is open.

What is the root cause of that problem?

In my testing, when using the arrow keys, the browser scrolls the last element the user interacted with. It's not well documented, but I found a Stack Overflow answer https://stackoverflow.com/a/77580408 mentioning this browser behavior.

When the RHP is not open, useArrowKeyFocusManager prevents the default behavior and controls the scrolling programmatically.

App/src/hooks/useArrowKeyFocusManager.ts

Line 110 in e94df0f

useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_UP, arrowUpCallback, arrowConfig);

App/src/hooks/useKeyboardShortcut.ts

Line 34 in e94df0f

shouldPreventDefault = true,

When the RHP is open, useArrowKeyFocusManager is inactive, so the default behavior resumes.

App/src/components/SelectionList/BaseSelectionList.tsx

Line 323 in e94df0f

isActive: isFocused,

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

Always keep useArrowKeyFocusManager active. It will prevent the default behavior but do nothing else when the list is not focused.

isActive: true,

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

This issue is caused by the browser's default behavior, so I suggest manual testing in real browsers.

Also verity this change doesn't reintroduce #41906.

What alternative solutions did you explore? (Optional)

[linhvovan29546]

Edited by proposal-police: This proposal was edited at 2024-12-27 04:38:39 UTC.

Proposal

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

Categories - Category list moves with up and down key when RHP Is opened

What is the root cause of that problem?

This issue stems from default web behavior. When a user clicks on content within a scrollable view, the browser allows scrolling via the keyboard's arrow keys.
The reason this issue does not appear on other pages as PR #39327, is that those screens use SectionList with useArrowKeyFocusManager to handle keydown events, and when the Category list is not focused this hook is not active

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 319 to 323 in e94df0f

	const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({
	initialFocusedIndex: flattenedSections.allOptions.findIndex((option) => option.keyForList === initiallyFocusedOptionKey),
	maxIndex: Math.min(flattenedSections.allOptions.length - 1, CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage - 1),
	disabledIndexes: disabledArrowKeyIndexes,
	isActive: isFocused,

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

Disable scrolling when the Category list is not focused

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 791 to 793 in e94df0f

	<SectionList
	removeClippedSubviews={removeClippedSubviews}
	ref={listRef}

scrollEnabled={isFocused}

POC

Screen.Recording.2024-12-27.at.11.18.13.AM.mov

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

What alternative solutions did you explore? (Optional)

Override arrow key behavior when RHP(CategorySettingsPage) is opened, we can add useArrowKeyFocusManager hook to CategorySettingsPage page

App/src/pages/workspace/categories/CategorySettingsPage.tsx

Lines 38 to 43 in e94df0f

	function CategorySettingsPage({
	route: {
	params: {backTo, policyID, categoryName},
	},
	navigation,
	}: CategorySettingsPageProps) {

useArrowKeyFocusManager({
    maxIndex: -1, 
});

[melvin-bot]

@anmurali Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[melvin-bot]

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

[jayeshmangwani]

@QichenZhu I tested your proposal of keeping isActive always set to true, and it does resolve this bug. However, I'm unsure why having isActive set to true when isFocused is true is causing this issue. I suspect the root cause might be within the useArrowKeyFocusManager implementation.

[jayeshmangwani]

The reason this issue does not appear on other pages as PR #39327, is that those screens use SectionList with useArrowKeyFocusManager to handle keydown events, and when the Category list is not focused this hook is not active

@linhvovan29546 I'm not entirely clear on your root cause here. In that PR, we were using the ArrowKeyFocusManager HOC instead of the useArrowKeyFocusManager hook, and there might be slight differences in how they work.

[linhvovan29546]

The reason this issue does not appear on other pages as PR #39327, is that those screens use SectionList with useArrowKeyFocusManager to handle keydown events, and when the Category list is not focused this hook is not active

No, it works the same. Both files listen to the KeyboardShortcut event to handle arrow down/up.
In this OP, the issue occurs when the RHP is opened aka CategorySettingsPage. On this page, we do not use useArrowKeyFocusManager or to override the keyboard behavior, which causes the issue to appear

[QichenZhu]

However, I'm unsure why having isActive set to true when isFocused is true is causing this issue. I suspect the root cause might be within the useArrowKeyFocusManager implementation.

@jayeshmangwani Thanks for your comment!

When useArrowKeyFocusManager is inactive, the browser's default behavior causes the list to scroll. When active, it prevents this default behavior.

Setting isActive: isFocused is not ideal because focus and scrollability are two separate things. An element not focused doesn’t mean it’s not scrollable with arrow keys.

As mentioned in the Stack Overflow answer, the browser has its strategy to decide which element to scroll, and

this is not exposed and not directly controllable

So we need to keep useArrowKeyFocusManager active to prevent the default behavior, even when the list is not focused.

[jayeshmangwani]

Thanks for the details here!

@QichenZhu's Proposal of setting isActive: true works fine. I’ve verified that adding isActive: true does not reintroduce this issue #41906 we can move forward with this.

@QichenZhu 's Proposal LGTM 🚀

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @QichenZhu 🎉 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]

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

[melvin-bot]

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

• Prevent default scrolling behavior in selection list #54797

If no regressions arise, payment will be issued on 2025-01-18. 🎊

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

• @jayeshmangwani requires payment through NewDot Manual Requests
• @QichenZhu requires payment automatic offer (Contributor)

[melvin-bot]

@jayeshmangwani @anmurali @jayeshmangwani The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[jayeshmangwani]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: Fix: navigate to invite list by arrow keys #42713 (comment)

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• We already have the regression test for this PR: User can access the background list using the arrow keys while the RHP is open #39327 so there’s no need to create the same one.

[melvin-bot]

@iwiznia, @anmurali, @jayeshmangwani, @QichenZhu Whoops! This issue is 2 days overdue. Let's get this updated quick!

[anmurali]

Payment Summary

• @jayeshmangwani approved $250 via NewDot Manual Requests
• @QichenZhu $250 approved via automatic offer (Contributor)

[garrettmknight]

$250 approved for @jayeshmangwani