[HOLD for payment 2023-09-25] [$500] Web - Select all checkbox is not aligned with member checkboxes

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open the app
2. Open settings->workspaces->any workspace->members
3. Observe that 'Select all' checkbox has 2px padding which is not present on all member checkboxes making it misalign

Expected Result:

Select all checkbox should be aligned with member checkboxes

Actual Result:

Select all checkbox has 2px padding which is not present on all member checkboxes making it misalign

Workaround:

Unknown

Platforms:

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

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

Version Number: 1.3.61.0
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
Notes/Photos/Videos: Any additional supporting documentation

select.all.checkbox.not.aligned.mp4

Recording.4160.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693067880629009

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016d76b0b3b26c4f09
• Upwork Job ID: 1697720858917765120
• Last Price Increase: 2023-09-08
• Automatic offers:
• Ollyws | Reviewer | 26620742
• Pujan92 | Contributor | 26620744
• dhanashree-sawant | Reporter | 26620745

[c3024]

Proposal

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

Check box of select all has an extra padding of 2px on the left making the checkboxes appearing not in line

What is the root cause of that problem?

The Select All checkbox here has this styles.checkboxPressable which adds a padding of 2px.

App/src/components/Checkbox.js

Line 94 in c029dc5

style={[props.style, styles.checkboxPressable]}

App/src/styles/styles.js

Line 2585 in c029dc5

padding: 2,

This padding is not there for the CheckboxListItem.

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

We should change the padding to paddingVertical: 2 in the checkboxPressable of styles so that the vertical padding remains and this checkbox used at other places will appear fine. The horizontal padding 2px does not make a difference in other pages where this is used.

What alternative solutions did you explore? (Optional)

Result

[melvin-bot]

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

[Pujan92]

Proposal

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

Checkboxes are not aligned correctly for selection list

What is the root cause of that problem?

We provided the padding of 2px and some other styles for the normal CheckBox here which isn't provided for the selection list options checkboxes and that is the reason for this issue.

App/src/components/Checkbox.js

Line 94 in 0775f2f

style={[props.style, styles.checkboxPressable]}

App/src/styles/styles.js

Lines 2596 to 2601 in 0775f2f

	checkboxPressable: {
	borderRadius: 6,
	padding: 2,
	justifyContent: 'center',
	alignItems: 'center',
	},

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

We cannot remove this padding considering the outline change added in this PR and comment.
I think we need to create a wrapper View for checkbox inside CheckboxListItem and provide the same style styles.checkboxPressable or only padding padding: 2(as the checbox in CheckboxListItem isn't focusable).

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016d76b0b3b26c4f09

[melvin-bot]

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

[melvin-bot]

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

[muradkhan101]

Proposal

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

The "Select all" checkbox is not aligned with the member checkboxes

What is the root cause of that problem?

This PressableWithFeedback that wraps the "Select all" checkbox has a constant "paddingHorizontal" of 20 in all use cases, however, in the Members page, it should have a slightly smaller padding so that it is aligned with the other checkboxes.

App/src/components/SelectionList/BaseSelectionList.js

Lines 342 to 349 in 9701fbe

	<PressableWithFeedback
	style={[styles.peopleRow, styles.userSelectNone, styles.ph5, styles.pb3]}
	onPress={onSelectAll}
	accessibilityLabel={translate('workspace.people.selectAll')}
	accessibilityRole="button"
	accessibilityState={{checked: flattenedSections.allSelected}}
	disabled={flattenedSections.allOptions.length === flattenedSections.disabledOptionsIndexes.length}
	>

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

We need to add a new compactPadding, or similar name, prop to BaseSelectionList and if that prop is set, we render the smaller padding. Then on the Members page, we pass this new prop so we only affect the padding on this page and don't cause regressions else where.

App/src/pages/workspace/WorkspaceMembersPage.js

Lines 385 to 398 in 9701fbe

	<SelectionList
	canSelectMultiple
	sections={[{data, indexOffset: 0, isDisabled: false}]}
	textInputLabel={props.translate('optionsSelector.findMember')}
	textInputValue={searchValue}
	onChangeText={setSearchValue}
	headerMessage={headerMessage}
	onSelectRow={(item) => toggleUser(item.keyForList)}
	onSelectAll={() => toggleAllUsers(data)}
	onDismissError={dismissError}
	showLoadingPlaceholder={!OptionsListUtils.isPersonalDetailsReady(props.personalDetails) || _.isEmpty(props.policyMembers)}
	shouldDelayFocus
	showScrollIndicator
	/>

What alternative solutions did you explore? (Optional)

Directly override the paddingHorizontal in BaseSelectionList but this would affect other pages in the app too

[melvin-bot]

📣 @muradkhan101! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[alifhaider]

Proposal

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

Select all checkbox and other member list checkboxes aren't aligned properly

What is the root cause of that problem?

Here we are adding 2px of padding to visualize the focus state of that checkbox.

App/src/components/Checkbox.js

Line 94 in 7e3e760

style={[props.style, styles.checkboxPressable]}

But in the other checkboxes of the Members list, this padding is not present.

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

We can add a margin to all other checkboxes in SectionList. This can be done in two ways.

Option 1:
Either we add a wrapper <View> with padding: 2, before this view

App/src/components/SelectionList/CheckboxListItem.js

Line 38 in 7e3e760

<View

<View style={{padding: 2}}>
                <View
                    style={[
                        StyleUtils.getCheckboxContainerStyle(20, 4),
                        item.isSelected && styles.checkedContainer,
                        item.isSelected && styles.borderColorFocus,
                        item.isDisabled && styles.cursorDisabled,
                        item.isDisabled && styles.buttonOpacityDisabled,
                    ]}
                >

Option 2:
Or we add another utility class with existing classes for a margin

App/src/components/SelectionList/CheckboxListItem.js

Lines 39 to 44 in 7e3e760

	style={[
	StyleUtils.getCheckboxContainerStyle(20, 4),
	item.isSelected && styles.checkedContainer,
	item.isSelected && styles.borderColorFocus,
	item.isDisabled && styles.cursorDisabled,
	item.isDisabled && styles.buttonOpacityDisabled,

<View
                    style={[
                        StyleUtils.getCheckboxContainerStyle(20, 4),
                        item.isSelected && styles.checkedContainer,
                        item.isSelected && styles.borderColorFocus,
                        item.isDisabled && styles.cursorDisabled,
                        item.isDisabled && styles.buttonOpacityDisabled,
                        styles.checkboxDefaultMargin, // margin: 2px here
                    ]}
                >

What alternative solutions did you explore? (Optional)

N/A

[Ollyws]

Will review this asap.

[stephanieelliott]

Hey @Ollyws bumping this, have you had a chance to review the proposals?

[depak379mandal]

I came here for same thing, You gotta remove padding: 2 Or add it in selectItem.
Same as above Developers did here.

[melvin-bot]

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

[Ollyws]

Given we can't remove the padding on the select-all checkbox for reasons mentioned by @Pujan92, I think the best solution is to make sure all checkboxes have the same padding as outlined in @Pujan92's proposal.
🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

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

[melvin-bot]

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[techievivek]

Not overdue, we have selected a proposal now #26494 (comment)

[Pujan92]

@Ollyws PR is ready for review!

[melvin-bot]

🎯 ⚡️ Woah @Ollyws / @Pujan92, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @Pujan92 got assigned: 2023-09-11 23:04:27 Z
• when the PR got merged: 2023-09-14 10:16:11 UTC

On to the next one 🚀

[stephanieelliott]

This was deployed to production on 9/18! Not sure why this issue didn't update, will update the title manuall to apply 7 day hold.

[stephanieelliott]

Summarizing payment on this issue:

• Issue reporter: @dhanashree-sawant $50, paid via Upwork
• Contributor: @Pujan92 $500, paid via Upwork
• Contributor+: @Ollyws $500, offer extended in Upwork -- awaiting acceptance

Upwork job is here, no bonus 50% bonus on final payout

[Pujan92]

@stephanieelliott it is eligible for bonus, plz check this

[stephanieelliott]

Oops, thanks @Pujan92! Fixed the summary and issued $250 bonus to you. Also paid out $750 total to @Ollyws