[HOLD for payment 2022-05-02] [$250] Close account - Unnecessary extra vertical margin above the pop up message when tapping close account

[kavimuru]

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. Launch the app
2. Log in with account kbecciv+2002@gmail.com/Feya86Katya
   on New dot.
3. Go to Settings - Security - Close account.
4. Enter the message in input box.
5. Enter email which you are going to close.
6. Click close account.

Expected Result:

Normal space between border and message in popup after tap close account

Actual Result:

Too much space in popup message after tap close account

Workaround:

Visual

Platform:

Where is this issue occurring?

• iOS
• Android

Version Number: 1.1.49.0

Reproducible in staging?: Y

Reproducible in production?: Y

Email or phone of affected tester (no customers): kbecciv+2002@gmail.com / Feya86Katya

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

Notes/Photos/Videos:

Expensify/Expensify Issue URL:

Issue reported by: Applause @Tushu17

Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1648072472591519

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @marcaaron (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[Tushu17]

Hi @kavimuru I had reported this on slack, is this eligible for the reporting bonus.

[sobitneupane]

Proposal:
Reason: The Confirm Modal in CloseAccountPage has space as title so extra space appears at the top.
There are two ways to deal with this issue.

1. Add title to Confirm Modal in CloseAccountPage. All Confirm Modal except in CloseAccountPage has title so we might be tempted to add title to this confirm modal as well. We can use "Unable to Close Your Account" or similar title.
   

   App/src/pages/settings/Security/CloseAccountPage.js

   Lines 113 to 115 in 50496a0

   	<ConfirmModal
   	title=""
   	success

2. Show the header section only if title is present.
   

   App/src/components/ConfirmContent.js

   Lines 58 to 60 in 50496a0

   	<View style={[styles.flexRow, styles.mb4]}>
   	<Header title={props.title} />
   	</View>

        {props.title.length > 0 && (
            <View style={[styles.flexRow, styles.mb4]}>
                <Header title={props.title} />
            </View>
        )}

[Puneet-here]

Proposal

We aren't using any title for this modal and we have some margin at bottom for the title that's why there's extra space.

App/src/pages/settings/Security/CloseAccountPage.js

Lines 113 to 114 in 50496a0

	<ConfirmModal
	title=""

if we use this code there won't be any extra space when there's no title.

 {!_.isEmpty(props.title) && (
        <View style={[styles.flexRow, styles.mb4]}>
            <Header title={props.title} />
        </View>
        )}

Also the title is required here -

App/src/components/ConfirmContent.js

Lines 11 to 13 in 50496a0

	const propTypes = {
	/** Title of the modal */
	title: PropTypes.string.isRequired,

There can be other modals in future that won't need a title so if we want we can make it optional by removing isRequired and we can add title: '' in default props we also need to make the title optional ConfirmModal as it passes the title to ConfirmContent

And also if we want we can add a title in this modal -

[melvin-bot]

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

[marcaaron]

LGTM. I updated the title so the issue would be more clear. I thought it was referring to a white space character and not extra vertical margin.

[mallenexpensify]

Hi @kavimuru I had reported this on slack, is this eligible for the reporting bonus.

@kavimuru , can you confirm this is the same ·(or... the same-enough) as #8457? If so, I'll close that issue and add reported by @Tushu17 to the OP here.

[isagoico]

Looks like the same issue but in a different modal in my opinion - @mallenexpensify I think you can go ahead with closing the other issue and editing this one

[melvin-bot]

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

[melvin-bot]

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

[mallenexpensify]

ah... interesting, since I'm the CM assigned and also on the marketing team, I stay assigned for waiting for copy (not surprised but haven't had it happen before).

I like "Unable to close account" and think it will fit. Without involving our design team, is there an easy way to find out?

[stitesExpensify]

Yeah it should be trivial to toss that in there 😄 Let's go ahead and hire @sobitneupane and they can take care of it

[melvin-bot]

📣 @sobitneupane You have been assigned to this job by @stitesExpensify!
Please apply to this job in Upwork 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 📖

[rushatgabhane]

I like "Unable to close account"

@mallenexpensify Thanks, let's go with it then.

Without involving our design team, is there an easy way to find out?

If you access to the design (figma) files. You could find out if it fits.
Anyway, I gave the proposal a try and "Unable to close account" fits comfortably for small devices.

[stitesExpensify]

Spanish translation: No se pudo cerrar tu cuenta

[sobitneupane]

Job posted https://www.upwork.com/jobs/~01ac2d33f4be5cc3d3

Applied for the job.

[mallenexpensify]

Everyone is hired in Upwork!

[stitesExpensify]

Not overdue

[stitesExpensify]

PR is now merged

[melvin-bot]

@mallenexpensify, @sobitneupane, @rushatgabhane, @stitesExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.56-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Add title in unable to close account modal #8584

If no regressions arise, payment will be issued on 2022-05-02. 🎊

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[mallenexpensify]

I'm out Mon/Tues so I paid $250 ea today to @Tushu17 , @sobitneupane and @rushatgabhane
Thanks!