[HOLD for payment 2024-01-17] [$500] Category & Tag - Unable to select Category/Tag with Enter key when there is less than 8 items

[kbecciv]

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: v1.4.19-1
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal team
Slack conversation:

Action Performed:

Precondition:

• User is an employee of a Collect workspace.
• The Collect workspace has less than 8 items for both Category and Tag.

1. Go to workspace chat as employee.
2. Click + > Request money > Manual.
3. Enter amount > Next > Show more.
4. Click Category.
5. Hit Enter.
6. Return to confirmation page.
7. Click Tag.
8. Hit Enter.

Expected Result:

User is able to select category/tag by hitting Enter on the selection.

Actual Result:

Nothing happens when hitting Enter on the selected category/tag.

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

Bug6328526_1703868072131.20231229_210214.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f16a13cdec4a4762
• Upwork Job ID: 1740788160854630400
• Last Price Increase: 2023-12-29
• Automatic offers:
• s77rt | Reviewer | 28073778
• FitseTLT | Contributor | 28073779

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[FitseTLT]

Proposal

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

Unable to select Category/Tag with Enter key when there is less than 8 items

What is the root cause of that problem?

When there is more than 8 items the selection via enter key will occur through onSubmitEditing of the text input in BaseOptionsSelector

App/src/components/OptionsSelector/BaseOptionsSelector.js

Line 480 in 7d1b0f8

onSubmitEditing={this.selectFocusedOption}

But for lesser items the enter key event listener here should have handled it

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 263 to 271 in 7d1b0f8

	const enterConfig = CONST.KEYBOARD_SHORTCUTS.ENTER;
	this.unsubscribeEnter = KeyboardShortcut.subscribe(
	enterConfig.shortcutKey,
	this.selectFocusedOption,
	enterConfig.descriptionKey,
	enterConfig.modifiers,
	true,
	() => !this.state.allOptions[this.state.focusedIndex],
	);

but the event gets consumed by the Request button in confirmation page below the picker page (and also the button of the participants page if you come from the main FAB flow) over the enter key listener in BaseOptionsSelector for the category/tag picker so it is not selecting the focused item.
Although the BaseOptionsSelector should have consumed the event first as it is subscribe the latest from both buttons, the event subscriber for the button re-subscribes as the activeElement changes as it subscribes using useKeyboardShortcut hook with as isActive is dependent on useActiveElement here

App/src/components/Button/index.tsx

Lines 166 to 167 in 7d1b0f8

	const activeElement = useActiveElement();
	const accessibilityRoles: string[] = Object.values(CONST.ACCESSIBILITY_ROLE);

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

1. Now the buttons that are consuming the enter key event are in pages below category picker page, which means the buttons are consuming the event although the page they are in is not in focus. However that is wrong and this will create problems everywhere we use button with pressOnEnter and we should apply the fix in Button so that it will only consume the enter key event only if it is focused so change this
   

   App/src/components/Button/index.tsx

   Lines 181 to 182 in 7d1b0f8

   	isActive: pressOnEnter && !shouldDisableEnterShortcut,
   	shouldBubble: allowBubble,

   
   to

        isActive: pressOnEnter && !shouldDisableEnterShortcut && isFocused,

App/src/components/Button/index.tsx

Lines 166 to 167 in 7d1b0f8

	const activeElement = useActiveElement();
	const accessibilityRoles: string[] = Object.values(CONST.ACCESSIBILITY_ROLE);

What alternative solutions did you explore? (Optional)

Ofcourse we can use many other alternative solutions so that the enter key event triggers the listener in BaseOptionsSelector like

1. using priority
   The enter key listener in BaseOptionsSelector should have a higher priority (N.B the priority value should be lesser to have high priority) than the Request button in confirmation page,
   So we will give the button less priority here
   

   App/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js

   Lines 533 to 539 in 7d1b0f8

   	<ButtonWithDropdownMenu
   	pressOnEnter
   	isDisabled={shouldDisableButton}
   	onPress={(_event, value) => confirm(value)}
   	options={splitOrRequestOptions}
   	buttonSize={CONST.DROPDOWN_BUTTON_SIZE.LARGE}
   	/>

   
   we will create a new priority prop for ButtonWithDropdownMenu and pass it to the Button in the component and do the same for the participant button too.
2. Or there is an already disabling enter mechanism which disables if the active element has a role different from text so we can give the list items a role that can also disable the enter key listener in the button

[s77rt]

@FitseTLT Thanks for the proposal. Your RCA is correct. The useKeyboardShortcut hook will re-run in many cases and since we can't prevent it from re-running (e.g. the keyboardShortcutCallback can change if the onPress changes) the best option here is to unsubscribe on re-run instead of subscribing again (just as you suggested). I would just add that we should remove the isFocused param from validateSubmitShortcut since it will never be false. Great solution overall 👍

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

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

[melvin-bot]

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

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

[jliexpensify]

@nkuoch does this need to be added to the Wave 6 project? Cheers!

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

• Fix pressOnEnter of button to be disabled when screen is not focused #33889

If no regressions arise, payment will be issued on 2024-01-17. 🎊

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

• @s77rt requires payment automatic offer (Reviewer)
• @FitseTLT 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:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] 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:
• [@s77rt] 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:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] 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:

[s77rt]

• The PR that introduced the bug has been identified: Migrate Button.js to function component #30349
• The offending PR has been commented on: Migrate Button.js to function component #30349 (comment)
• A discussion in #expensify-bugs has been started: Not needed
• Determine if we should create a regression test for this bug: Yes

---

Regression Test Proposal

1. Open App
2. Go to any workspace chat as an employee
3. Click + > Request money > Manual
4. Enter amount > Next > Show more
5. Click Category
6. Click anywhere so the Search input is not focused
7. Use arrow keys to select a category
8. Press Enter
9. Verify the category got selected

[jliexpensify]

Payment Summary:

• Contributor: @FitseTLT $500
• C+: @s77rt $500

Upwork job

[jliexpensify]

Whoops I am sorry - paying now!

[jliexpensify]

Paid and job closed!