[HOLD for payment 2024-01-24] [$500] Split bill - No bottom margin for "Show more" button on IOU details page

[kbecciv]

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: v1.4.19-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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Create a workspace and invite 10 members to it
2. Go to the announce room of the workspace created on step 1
3. Click on the + icon inside the compose box
4. Click on split bill
5. Enter an amount and split the bill with all the members
6. Go in announce room and click on the IOU preview component
7. Scroll down until you see the "Show more" button

Expected Result:

There should be a bottom margin for the button

Actual Result:

There is no bottom margin for the "Show more" button

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

Screen_Recording_20240106_193217_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0177cfaec73e6a7668
• Upwork Job ID: 1740747833980624896
• Last Price Increase: 2023-12-29
• Automatic offers:
• c3024 | Reviewer | 28088096
• esh-g | Contributor | 28088097

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0177cfaec73e6a7668

[melvin-bot]

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

[melvin-bot]

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

[esh-g]

Proposal

Please re-state the problem?

Split bill - No bottom margin for "Show more" button on IOU details page

What is root cause?

There is no bottom margin here:

App/src/components/MoneyRequestConfirmationList.js

Lines 644 to 647 in 939632e

	{!shouldShowAllFields && (
	<ShowMoreButton
	containerStyle={styles.mt1}
	onPress={toggleShouldExpandFields}

What changes should be made?

Add bottom margin by possibly replacing the styles.mt1 with styles.mv1 (or just add styles.mb2 if additional margin needed) the exact amount can be decided in the PR

[neonbhai]

Proposal

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

Split bill - No bottom margin for "Show more" button on IOU details page

What is the root cause of that problem?

This happens as we don’t apply enough margin in this view:




App/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js

Line 696 in 939632e

<View style={[styles.flexRow, styles.justifyContentBetween, styles.mh3, styles.alignItemsCenter, styles.mb2, styles.mt1]}>

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

We should increase the margin from mb2 to mb3

Alternatively

We can increase the buttons margin here:

App/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js

Line 705 in 939632e

style={styles.mh0}

[bernhardoj]

Proposal

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

There is no bottom margin for Show more button in split bill details page.

What is the root cause of that problem?

Split bill details page uses MoneyRequestConfirmationList and the show more button doesn't have any margin bottom (only top) applied to it.

App/src/components/MoneyRequestConfirmationList.js

Lines 644 to 649 in 939632e

	{!shouldShowAllFields && (
	<ShowMoreButton
	containerStyle={styles.mt1}
	onPress={toggleShouldExpandFields}
	/>
	)}

But the money request flow confirmation page has some margin applied to it.

App/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js

Lines 695 to 709 in 939632e

	{!shouldShowAllFields && (
	<View style={[styles.flexRow, styles.justifyContentBetween, styles.mh3, styles.alignItemsCenter, styles.mb2, styles.mt1]}>
	<View style={[styles.shortTermsHorizontalRule, styles.flex1, styles.mr0]} />
	<Button
	small
	onPress={toggleShouldExpandFields}
	text={translate('common.showMore')}
	shouldShowRightIcon
	iconRight={Expensicons.DownArrow}
	iconFill={theme.icon}
	style={styles.mh0}
	/>
	<View style={[styles.shortTermsHorizontalRule, styles.flex1, styles.ml0]} />
	</View>
	)}

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

I guess we should follow the margin from the new confirmation page and apply it to the old MoneyRequestConfirmationList

What alternative solutions did you explore? (Optional)

As MoneyRequestConfirmationList is the old component, maybe we want to replace MoneyRequestConfirmationList with the new page (MoneyTemporaryForRefactorRequestConfirmationList) in split bill details page (maybe this is being worked on already?).

[c3024]

It seems there is some other bug in the flow of split bill in announce room in the latest main. After creating the split bill clicking on the preview again shows the split bill button.

(Please ignore the initial few seconds of the video.)
https://github.com/Expensify/App/assets/102477862/4d4eba8c-8c75-4c19-bdfe-780e38233c04

Please help me if I am missing something.

Could you post a video with the repo steps? @kbecciv

[dragnoir]

Proposal

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

Split bill - No bottom margin for "Show more" button on IOU details page

What is the root cause of that problem?

Margin button missing for the "Show more"

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

As we are using the right margin for the "Show more" button on BaseOptionSelector

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 539 to 540 in 320ff54

	<ShowMoreButton
	containerStyle={{...this.props.themeStyles.mt2, ...this.props.themeStyles.mb5}}

We need to add same margin too, styles.mb5 to ShowMoreButton here

Adding styles.mb5 give consistency to the application and make all the pages looks similar in term of margins. mt2 also is better to make the app UI consistent.

[melvin-bot]

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

[bfitzexpensify]

Seems like we're generally addressing the broader issue of problems creating a split bill in the #announce channel via #33802 & #33786

So here we can just focus on the margin issue @c3024

[c3024]

@esh-g 's proposal here looks good to me.

🎀 👀 🎀 C+ Reviewed

[melvin-bot]

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

[kbecciv]

@c3024 Updated with a video. Thanks

[melvin-bot]

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

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

[bfitzexpensify]

Not overdue, we've picked a proposal.

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

• Add margin to show more button in money request #34103

If no regressions arise, payment will be issued on 2024-01-24. 🎊

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

• @esh-g requires payment automatic offer (Contributor)
• @c3024 requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@c3024] The PR that introduced the bug has been identified. Link to the PR:
• [@c3024] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@c3024] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@c3024] Determine if we should create a regression test for this bug.
• [@c3024] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[c3024]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@c3024] The PR that introduced the bug has been identified. Link to the PR: Limit tag list to 500 items #31447
• [@c3024] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/31447/files#r1469994259
• [@c3024] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: It is a case of missing margin when migrating to a component. As such, a Slack discussion does not appear to be useful.
• [@c3024] Determine if we should create a regression test for this bug. No, this is a simple margin style change.
• [@c3024] If we decide to create a regression test for the bug, please propose the [regression test] (https://github.com/Expensify/App/blob/main/contributingGuides/REGRESSION_TEST_BEST_PRACTICES.md) steps to ensure the same bug will not reach production again. NA

[c3024]

Bump @bfitzexpensify. Melvin did not add Daily label here.

[bfitzexpensify]

Thanks for the tag @c3024 - contracts have all been paid. Thanks for the work here everyone!