IOU - Keyboard arrow keys does not work inside description field

[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.71-0
Reproducible in staging?: Y
Reproducible in production?: Y
Found when executing PR : #41291
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Go to staging.new.expensify.com
2. Click on FAB > Submit Expense > Enter an amount > Click next
3. Open the description field
4. Create multiple new lines
5. Press keyboard up arrow key to go up the lines

Expected Result:

Cursor jumps to previous line when pressing up arrow

Actual Result:

When user presses on the up arrow key nothing happens

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

Bug6473883_1715095823382.bandicam_2024-05-07_18-25-25-017.mp4

View all open jobs on GitHub

[melvin-bot]

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

[izarutskaya]

We think this issue might be related to the #vip-vsb.

[bernhardoj]

Proposal

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

The arrow up/down key doesn't work in money request description page.

What is the root cause of that problem?

On the confirmation page, we use OptionsSelector which uses ArrowKeyFocusManager. There is currently no logic to unsubscribe the shortcut when the screen is out of focus. So, the arrow up/down key is consumed by the shortcut.

Why did it work before? That's because we are excluding a TEXTAREA element.

App/src/components/ArrowKeyFocusManager.js

Line 60 in 55c56bf

[this.props.shouldExcludeTextAreaNodes && 'TEXTAREA'],

However, the description field now is a live markdown input which is a DIV. But instead of excluding specific tag, we should disable the shortcut when the screen is not focused anymore. I think we need a new way to exclude the live markdown input, but I guess that's not the focus of this issue.

OptionsSelector component itself is deprecated and is in progress to be replaced and ArrowKeyFocusManager will also probably be removed, however, this issue also happens in SelectionList. The shortcut is always enabled.

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 230 to 234 in 55c56bf

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

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

In ArrowKeyFocusManager, we can unsub the shortcut when isFocused is false and subscribe it back when isFocused is true.

App/src/components/ArrowKeyFocusManager.js

Lines 64 to 71 in 55c56bf

	componentDidUpdate(prevProps) {
	if (prevProps.maxIndex === this.props.maxIndex) {
	return;
	}
	if (this.props.focusedIndex > this.props.maxIndex && this.props.shouldResetIndexOnEndReached) {
	this.onArrowDownKey();
	}
	}

if (prevIsFocused && !isFocused) unsubscribe
else if (!prevIsFocused && isFocused) subscribe

Or we can simply return the children here when isFocused is false

App/src/components/ArrowKeyFocusManager.js

Lines 126 to 136 in 55c56bf

	function ArrowKeyFocusManager(props) {
	const isFocused = useIsFocused();
	return (
	<BaseArrowKeyFocusManager
	// eslint-disable-next-line react/jsx-props-no-spreading
	{...props}
	isFocused={isFocused}
	/>
	);
	}

if (!isFocused) return props.children;

For the selection list fix, we can set the isActive to isFocused

App/src/components/SelectionList/BaseSelectionList.tsx

Line 234 in 55c56bf

isActive: true,

or to !disableKeyboardShortcuts && isFocused just like we did with ctrl+enter here

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 498 to 513 in 55c56bf

	useKeyboardShortcut(
	CONST.KEYBOARD_SHORTCUTS.CTRL_ENTER,
	(e) => {
	const focusedOption = flattenedSections.allOptions[focusedIndex];
	if (onConfirm) {
	onConfirm(e, focusedOption);
	return;
	}
	selectFocusedOption();
	},
	{
	captureOnInputs: true,
	shouldBubble: !flattenedSections.allOptions[focusedIndex],
	isActive: !disableKeyboardShortcuts && isFocused,
	},
	);

If we want to improve the excludedNodes so it can exclude the live markdown element, we can refactor the excludedNodes code here so it becomes a function and returns a boolean.

App/src/libs/KeyboardShortcut/bindHandlerToKeydownEvent/index.ts

Lines 23 to 30 in 55c56bf

	const textArea = event.target as HTMLTextAreaElement;
	const contentEditable = textArea?.contentEditable;
	const nodeName = textArea?.nodeName;
	// Early return for excludedNodes
	if (callback.excludedNodes.includes(nodeName)) {
	return true;
	}

if (callback.excludedNodes(textArea)) {
    return true;
}

excludedNodes will return true if it's a live markdown. We can check if textArea is a div with contenteditable and aria-multiline is true. Or maybe we can set an ID to the component and check it's ID instead.

[stephanieelliott]

This is pretty minor and only affects web, I don't think we should prioritize this right now.

[melvin-bot]

@stephanieelliott Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@stephanieelliott Still overdue 6 days?! Let's take care of this!

[stephanieelliott]

Oops, had meant to close as not a priority