[HOLD for payment 2024-04-03] [$500] Workspace - Member and Role can be clicked to select all the members in Members list

[lanitochka17]

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.44-0
Reproducible in staging?: Y
Reproducible in production?: N
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:

Issue found when executing PR #37074

Action Performed:

1. Go to staging.new.expensify.com
2. Go to Workspace settings
3. Select a workspace
4. Go to Members
5. Click on Member and Role

Expected Result:

Member and Role should not be clickable, Only the checkbox is clickable

Actual Result:

Member and Role are clickable, which will select all members when clicked

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

Bug6393862_1709032651523.bandicam_2024-02-27_10-20-02-315.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cbcba3e1d6d489b9
• Upwork Job ID: 1762481814362009600
• Last Price Increase: 2024-02-27
• Automatic offers:
• ntdiary | Reviewer | 0
• rayane-djouah | Contributor | 0
• aimane-chnaif | Contributor | 0

[melvin-bot]

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

[melvin-bot]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @nkuoch (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[lanitochka17]

We think that this bug might be related to #wave5
CC @dylanexpensify

[neonbhai]

Proposal

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

Member and Role can be clicked to select all the members in Members list

What is the root cause of that problem?

We wrap both the Listheader and Checkbox with Pressable here

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

We move move the customlistheader out of pressable in BaseSelectionList

Move this code out of the Pressable wrapper here

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 448 to 452 in cfa0ae3

	{customListHeader ?? (
	<View style={[styles.flex1]}>
	<Text style={[styles.textStrong, styles.ph3]}>{translate('workspace.people.selectAll')}</Text>
	</View>
	)}

[rayane-djouah]

Proposal

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

Member and Role can be clicked to select all the members in Members list.

What is the root cause of that problem?

We are rendering customListHeader inside PressableWithFeedback Here:

App/src/components/SelectionList/BaseSelectionList.tsx

Line 448 in 174a032

{customListHeader ?? (

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

• We should wrap the list header with a View and move the header styles to it.
• we should add a condition to display the workspace.people.selectAll Text when there is not a customListHeader
• we should add styles.flexRow, styles.alignItemsCenter to the PressableWithFeedback which will contain the Checkbox and the select all Text only.
• we should move customListHeader outside of PressableWithFeedback

// Wrap with a View
<View
// move the styles into here
    style={[styles.peopleRow, styles.ph4, styles.pb3, listHeaderWrapperStyle]}
>
    <PressableWithFeedback
        style={[styles.userSelectNone, /* add this styles : */ styles.flexRow, styles.alignItemsCenter]}
        onPress={selectAllRow}
        accessibilityLabel={translate('workspace.people.selectAll')}
        role="button"
        accessibilityState={{checked: flattenedSections.allSelected}}
        disabled={flattenedSections.allOptions.length === flattenedSections.disabledOptionsIndexes.length}
        dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
        onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined}
    >
        <Checkbox
            accessibilityLabel={translate('workspace.people.selectAll')}
            isChecked={flattenedSections.allSelected}
            onPress={selectAllRow}
            disabled={flattenedSections.allOptions.length === flattenedSections.disabledOptionsIndexes.length}
        />
        {/* display "Select all" only if there is no customListHeader*/}
        {!customListHeader ? (
            <View style={[styles.flex1]}>
                <Text style={[styles.textStrong, styles.ph3]}>{translate('workspace.people.selectAll')}</Text>
            </View>
        ) : null
        }  
    </PressableWithFeedback>
    {/* display customListHeader outside PressableWithFeedback*/}
        {customListHeader}
</View>

[suneox]

Proposal

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

Workspace - Member and Role can be clicked to select all the members in Members list

What is the root cause of that problem?

The pressable at this line take full width, so when we click on empty space also trigger toggle all

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

We have to wrap the pressable with a view and move all style of PressableWithFeedback to view wrapper and then move customListHeader outside PressableWithFeedback
Only allow click on the checkbox

+     <View style={[styles.peopleRow, styles.userSelectNone, styles.ph4, styles.pb3, listHeaderWrapperStyle]}>
          <PressableWithFeedback
-             // style={[styles.peopleRow, styles.userSelectNone, styles.ph4, styles.pb3, listHeaderWrapperStyle]}
              onPress={selectAllRow}
              accessibilityLabel={translate('workspace.people.selectAll')}
              role="button"
              accessibilityState={{checked: flattenedSections.allSelected}}
              disabled={flattenedSections.allOptions.length === flattenedSections.disabledOptionsIndexes.length}
              dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
              onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined}
          >
              <Checkbox
                  accessibilityLabel={translate('workspace.people.selectAll')}
                  isChecked={flattenedSections.allSelected}
                  onPress={selectAllRow}
                  disabled={flattenedSections.allOptions.length === flattenedSections.disabledOptionsIndexes.length}
              />
-
          </PressableWithFeedback>
+         {customListHeader ?? (
+             <View style={[styles.flex1]}>
+                 <Text style={[styles.textStrong, styles.ph3]}>{translate('workspace.people.selectAll')}</Text>
+             </View>
+         )}
+     </View>

[bi-kash]

Proposal

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

The current issue involves the clickability of the "Member" and "Role" text. Not only that in my observation entire row is clickable. Clicking on these elements selects all members, which is unintended behavior.

What is the root cause of that problem?

The root cause lies in the fact that the PressableWithFeedback is not diabled causing entire row to be pressable. There is no reason for enabling it. Code for it is in

App/src/components/SelectionList/BaseSelectionList.tsx

Line 438 in 174a032

disabled={flattenedSections.allOptions.length === flattenedSections.disabledOptionsIndexes.length}

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

To address this issue, it is suggested to explicitly disable the PressableWithFeedback by setting the disabled prop to true. Since the checkbox is already pressable on its own, disabling the entire row's pressable functionality will prevent unintended selections.

disabled={true}

tested:
test.webm

What alternative solutions did you explore? (Optional)

NA

[aimane-chnaif]

@suneox I think your proposal is same as #37295 (comment). Any meaningful difference to consider?

[aimane-chnaif]

Taking this over as C+ (discussion)

[aimane-chnaif]

@rayane-djouah's proposal looks good to me.
🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @nkuoch is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[suneox]

@suneox I think your proposal is same as #37295 (comment). Any meaningful difference to consider?

We have a difference on my primary solution is only click on check box

[aimane-chnaif]

@suneox I think your proposal is same as #37295 (comment). Any meaningful difference to consider?

We have a difference on my primary solution is only click on check box

As production behavior is to click full item row, I think it's fine to keep that.
Will still get approval from design team in PR

[suneox]

As production behavior is to click full item row

Expected Result:
Member and Role should not be clickable, Only the checkbox is clickable

@aimane-chnaif the behavior conflicts with the Expected Result, as I mean the expected behavior is only clickable on the checkbox for the header

[aimane-chnaif]

As production behavior is to click full item row

Expected Result: Member and Role should not be clickable, Only the checkbox is clickable

@aimane-chnaif the behavior conflicts with the Expected Result, as I mean the expected behavior is only clickable on check at the header

I know. I meant the case when customListHeader doesn't exist.

[rayane-djouah]

Thank you for the review @aimane-chnaif !
I'm waiting for @nkuoch to assign me before raising a PR

[ntdiary]

@nkuoch, un-assign myself, please reassign it to @aimane-chnaif. 😄

[melvin-bot]

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

[aimane-chnaif]

@rayane-djouah bump

[rayane-djouah]

Will get the PR ready today

[luacmartins]

Let's add this issue to the scope here - #37511

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

• Fixes for selection list #37521

If no regressions arise, payment will be issued on 2024-04-03. 🎊

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

• @rayane-djouah requires payment automatic offer (Contributor)
• @aimane-chnaif requires payment automatic offer (Contributor)

[melvin-bot]

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[sonialiap]

@rayane-djouah $500 - paid ✔️
@aimane-chnaif $500 - paid ✔️