[HOLD for payment 2021-11-04] Don't allow attendee deselection from the bill-split confirmation page

[Julesssss]

CC @trjExpensify

Action Performed:

1. Launch the app and login
2. Click + > Split bill
3. Enter amount and click next
4. Select two attendees and click next
5. Land at the confirmation page

Expected Result:

You should not be able to deselect attendees from this flow. But this should be possible if you split from within a group chat!

Actual Result:

You are able to deselect attendees from this flow.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.0.98-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5231690_Recording__1041.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause
Slack conversation:

View all open jobs on GitHub

view this job

[MelvinBot]

Triggered auto assignment to @jboniface (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[PrashantMangukiya]

Proposed Solution

We have to pass one prop that indicates that it is group split or not. So based on that we can disable interactivity on IOUConfirmationList.js screen. I will suggest to update code as shown below.

/src/pages/iou/IOUModal.js add below code

<IOUConfirmPage
   ...
    isGroupSplit={this.steps.length === 2}     // ***  Add this code
/>

src/pages/iou/steps/IOUConfirmPage.js add below code

const propTypes = {
    ...
    /** Amount split belongs to group or not */
    isGroupSplit: PropTypes.bool.isRequired,         // *** Add this prop type 
};

<IOUConfirmationList
    ... 
    isGroupSplit={props.isGroupSplit}          // *** Add this code
/>

Within src/components/IOUConfirmationList.js add below code

const propTypes = {
    ... 
    /** Amount split belongs to group or not */
    isGroupSplit: PropTypes.bool.isRequired,         // *** Add this prop type 
    ... 
};

<OptionsList
    ...
    disableRowInteractivity={!this.props.isGroupSplit}         // Update this line
    ...
/>

Here is Screen Record - how it works after updates.

Screen.Recording.2021-09-16.at.9.44.36.PM.mov

[jboniface]

@Julesssss should this be on n6-hold?

[Julesssss]

Added.

[Santhosh-Sellavel]

Proposal

It's clear we don't want to allow deselection in the confirmation list. So here

App/src/components/IOUConfirmationList.js

Lines 331 to 332 in 50cb8d4

	onSelectRow={toggleOption}
	disableRowInteractivity={!this.props.hasMultipleParticipants}

We can straight away just pass disableRowInteractivity prop alone,
by removing this unnecessary ={!this.props.hasMultipleParticipants}

This should be enough.

Note:
Other things
onSelectRow prop can be removed & toggleOption method which is unused code, so it can be removed

[Julesssss]

Thanks for the proposals. I think @Santhosh-Sellavel's is a simpler solution as we're reusing the this.props.hasMultipleParticipants prop.

onSelectRow prop can be removed & toggleOption method which is unused code, so it can be removed

However, I'm not sure about this part. Wouldn't this still be necessary for the 'launch bill split from group chat' flow?

[PrashantMangukiya]

We need expected result as under:

You should not be able to deselect attendees from this flow. But this should be possible if you split from within a group chat!

Flow1: When flow initiated via
Click + > Split bill,
Enter amount and click next,
Select two attendees and click next,
Land at confirmation page. So in this case it should Not Allow to deselect attendees.

Flow2: Flow init from Group Chat
i.e. Click + button at left of text box in group chat,
Select Split Bill (when Group chat) (Otherwise Request Money)
Enter amount and click next,
It will directly Land at confirmation page So in this case it should Allow to deselect attendees.

My solution cover both flow. We need one props that indicates that current flow initiated via Group Chat > Split Bill OR FAB + > Split Bill, So thereafter we can control allow or not allow deselect for each flow as per need. In my screen record both flow works as expected.

[jboniface]

@Julesssss looks like we lost site of this since it's on n6-hold, do you have a response to the proposal?

[Julesssss]

@PrashantMangukiya your solution seems to work well, but my issue with it is that we don't need to set up a new isGroupSplit prop on IOUModal. Instead, we can use the preexisting this.props.hasMultipleParticipantsprop. Shown here. Could you update your original proposal to include this, please.

[PrashantMangukiya]

@Julesssss let me figure out if this.props.hasMultipleParticipants works or not. I will submit updated proposal soon.

[Santhosh-Sellavel]

@Julesssss Ideally either we should allow/restrict in both cases it causes a inconsistency in the flow. I find it confusing

[Julesssss]

I disagree about allowing both cases, the current flows are confusing as users can remove users on multiple steps. The only reason we added removal deselection in the first place is that there is no way to deselect if splitting a bill from within a group chat.

[parasharrajat]

I agree this change in this issue is necessary and well planned.

[PrashantMangukiya]

@Julesssss I did research again on this solution and I understood below things:

It looks like that we can not use hasMultipleParticipants prop from IOUModal.js to implement this solution, because hasMultipleParticipants prop comes true in both case i.e. Case1: When clicked FAB Button > Split Bill from Chats list. Other use Case2: Open Group Chat and Click + icon at left of chat text box, it shows Split Bill to initiate Split bill within a group.

Below are screenshot for Case1 and Case2

We need below expected result for this issue

You should not be able to deselect attendees from this flow. But this should be possible if you split from within a group chat!

So we need to take decision that Split Bill flow initiated via FAB button or from Group Chat. Based on that decision we can implement logic to Disable deselect attendees from FAB button Split Bill flow (case1), and allow to deselect if Split started from Group Chat (Case2).

So we can not take this decision based on hasMultipleParticipants prop from IOUModal.js because its coming true in both case. So we need some prop (e.g. isGroupSplit) that indicates from where this flow initiated.

So based on that props we have to take decision on src/components/IOUConfirmationList.js component and disable or enable deselection.

Based on my research at present I can not see other way to implement this solution. So the solution I suggested will work as expected and I can not see any other way to solve this problem. I will keep thinking and provide solution if found any other way.

@Julesssss please guide me if I missed something, I will appreciate it.

Thank you.

[Julesssss]

Oh right. Thanks for explaining that, I forgot that the prop was used to distinguish between Send Money/Split Bills!

In that case, I think we can move forward with your proposal once The N6 hold is cleared.

[PrashantMangukiya]

Thanks @Julesssss, please message me once N6 hold cleared. So thereafter I will proceed to submit pr.

[jboniface]

still on hold

[kadiealexander]

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

[jboniface]

still on hold

[PrashantMangukiya]

@jboniface thank you. I accepted Upwork offer. I will prepare and submit pr asap.

[parasharrajat]

I will be reviewing the PR and I will let you know @Julesssss when this is ready for final review and possibly for merge. See More

[mallenexpensify]

n6-hold label removed!

[jboniface]

On staging, not on production yet

[jboniface]

payment scheduled for tomorrow, not overdue

[jboniface]

paid with n6-hold bonus

[PrashantMangukiya]

Thanks