[$500] Split Bill - Amount turns to grayed out when User press Ctrl+Enter in Group conversation IOU

[lanitochka17]

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

---

Action Performed:

1. Navigate to staging.new.expensify.com
2. Click the '+' button in a Group conversation
3. Select Split Bill option
4. Enter an amount and press Next
5. Deselect all the participants of the list by using the arrow keys and ENTER
6. Press Ctrl+Enter

Expected Result:

Nothing should happens when user press Ctrl+Enter

Actual Result:

Amount field turns to grayed out

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• Windows / Chrome
• MacOS / Desktop

Version Number: 1.3.66-1

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6193153_Recording__936.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0138924214c461038e
• Upwork Job ID: 1701240648855633920
• Last Price Increase: 2023-09-18

[melvin-bot]

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

[bernhardoj]

Proposal

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

Pressing CTRL+ENTER in the split bill confirmation list will disable the amount and description.

What is the root cause of that problem?

The money request confirmation list page uses OptionsSelector to show the list. On the page, we subscribe to CTRL+ENTER keyboard shortcut that will trigger onConfirmSelection.

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 171 to 177 in b11bddc

	this.unsubscribeCTRLEnter = KeyboardShortcut.subscribe(
	CTRLEnterConfig.shortcutKey,
	() => {
	if (this.props.canSelectMultipleOptions) {
	this.props.onConfirmSelection();
	return;
	}

onConfirmSelection will call confirm that sets didConfirm state to true.

App/src/components/MoneyRequestConfirmationList.js

Line 396 in b11bddc

onConfirmSelection={confirm}

App/src/components/MoneyRequestConfirmationList.js

Lines 333 to 353 in b11bddc

	const confirm = useCallback(
	(paymentMethod) => {
	setDidConfirm(true);
	if (_.isEmpty(selectedParticipants)) {
	return;
	}
	if (props.iouType === CONST.IOU.MONEY_REQUEST_TYPE.SEND) {
	if (!paymentMethod) {
	return;
	}
	Log.info(`[IOU] Sending money via: ${paymentMethod}`);
	onSendMoney(paymentMethod);
	} else {
	onConfirm(selectedParticipants);
	}
	},
	[selectedParticipants, onSendMoney, onConfirm, props.iouType],
	);

If didConfirm is true, the amount and description will be disabled.

App/src/components/MoneyRequestConfirmationList.js

Line 437 in b11bddc

disabled={didConfirm || props.isReadOnly}

However, didConfirm is still set to true even though there is no participant selected.

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

Set the didConfirm state to true if there is some selected participant.

if (_.isEmpty(selectedParticipants)) return;
setDidConfirm(true);

We can take a safer approach by setting the didConfirm state to true before onSendMoney and onConfirm, but I didn't see any case where paymentMethod is empty.

What alternative solutions did you explore? (Optional)

1. Pass a new props to OptionsSelector to disable onConfirmSelection. It will be disabled if selectedParticipants is empty.
2. Pass nothing to onConfirmSelection if selectedParticipants is empty.

onConfirmSelection={_.isEmpty(selectedParticipants) ? undefined : onConfirm}

[melvin-bot]

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

[melvin-bot]

Current assignee @maddylewis is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[maddylewis]

adding external label to review proposal

[ArekChr]

It looks like I can't even select and unselect participants by mouse

Nagranie.z.ekranu.2023-09-13.o.11.41.11.mov

[ArekChr]

Set the didConfirm state to true if there is some selected participant.

@bernhardoj Where exactly do you suggest this change?

[bernhardoj]

@ArekChr In confirm function

App/src/components/MoneyRequestConfirmationList.js

Lines 333 to 339 in b11bddc

	const confirm = useCallback(
	(paymentMethod) => {
	setDidConfirm(true);
	if (_.isEmpty(selectedParticipants)) {
	return;
	}

We will switch the position of setDidConfirm and the if

[bernhardoj]

It looks like I can't even select and unselect participants by mouse

I can't find any report yet, but looks like this is a recent regression.

EDIT: regression from #26501

App/src/components/OptionsList/BaseOptionsList.js

Line 180 in 9675a77

isSelected={Boolean(_.find(selectedOptions, (option) => option.accountID === item.accountID || option.name === item.searchText))}

The PR add || option.name === item.searchText condition. On the money request page, both option.name and item.searchText is undefined, so it's always true.

[ArekChr]

I think we should put this issue on hold until regression with selecting participants will be fixed

[maddylewis]

@ArekChr - will you link the fix that you're suggesting we hold for?

[bernhardoj]

Here is the PR that fixes it and it's already merged. I tested on the main and it is fixed.

[melvin-bot]

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

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[maddylewis]

@ArekChr will you confirm -- was issue #27043 totally fixed by a separate issue (#27350)?

are we good to close this one out without any further action?

[bernhardoj]

@maddylewis #27350 is to fix this issue #27043 (comment).

[ArekChr]

@maddylewis Confirming that issue has been fixed

[melvin-bot]

@ArekChr @maddylewis this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

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

[maddylewis]

thank you for confirming!

[bernhardoj]

@maddylewis this GH issue is still happening. The issue that we are talking about is this one.

[bernhardoj]

@maddylewis please check the above comment.

[bernhardoj]

@maddylewis bump

[bernhardoj]

@maddylewis could you recheck this one?