[HOLD for payment 2023-05-23] [HOLD for payment 2023-05-16] [$1000] Editing workspace name to 1to3 characters adding extra line in the optional message when inviting users in Spanish

[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. Go to Settings > Preferences
2. Select Spanish as the language
3. Go back to Settings > Workspace
4. Select a workspace and change its name to 1-3 characters
5. Go to the Manage members > Invite
6. Check the last line of the personal message

Expected Result:

workspace name with 1-3 characters should not create an extra space at the last line of the personal message

Actual Result:

Changing the name of a workspace to 1-3 characters in Spanish preferences creates an extra space at the last line of the personal message

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.2.97-2
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

Recording.177.mp4

2023-04-08-22-06-39_phQPjLnl.mp4

Expensify/Expensify Issue URL:
Issue reported by: @ayazhussain79
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1680974868488639

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0108698f1d6f776495
• Upwork Job ID: 1646484899460993024
• Last Price Increase: 2023-04-20

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

@michaelhaxhiu Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~0108698f1d6f776495

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[michaelhaxhiu]

Going external, this is reproducible. Should be a very easy fix.

[tienifr]

Proposal

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

Changing the name of a workspace to 1-3 characters in Spanish preferences creates an extra space at the last line of the personal message.

What is the root cause of that problem?

The root cause is here

App/src/styles/styles.js

Line 2208 in 615a31c

minHeight: 115,

where we're setting the minHeight of the message which covers 4 lines. In the case the workspace name is short, the initial message will capture only 3 lines, leading to 1 redundant line in the bottom.

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

We need to set the message text input height according to the content of the message. We can apply the same logic that we're already doing for the Composer here for the content size calculation

App/src/components/Composer/index.android.js

Line 101 in 393efe1

onContentSizeChange={e => ComposerUtils.updateNumberOfLines(this.props, e)}

(and in other iOS/web-platform files).

If we want to limit the maximum/minimum number of lines to 3/4 for example (currently the height is always around 4.5 lines), we can also modify that logic to accept that maximum/minimum.

Since the logic is reused between 2 components, we can extract it our to a common HOC for reuse.

What alternative solutions did you explore? (Optional)

NA

[alexxxwork]

Proposal

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

When Workspace name is less then 3 characters invite text is 3 lines instead of 4

What is the root cause of that problem?

The root cause of the problem is static setting of numberOfLines={4} in the properties of the TextInput

App/src/pages/workspace/WorkspaceInvitePage.js

Lines 342 to 352 in 42bea4b

	<TextInput
	label={this.props.translate('workspace.invite.personalMessagePrompt')}
	autoCompleteType="off"
	autoCorrect={false}
	numberOfLines={4}
	textAlignVertical="top"
	multiline
	containerStyles={[styles.workspaceInviteWelcome]}
	value={this.state.welcomeNote}
	onChangeText={text => this.setState({welcomeNote: text})}
	/>

This settings works for RNTextInput component if height isn't set in style
also we have minHeight set here:

App/src/styles/styles.js

Line 2208 in 615a31c

minHeight: 115,

both of this settings affect the height of the component

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

We could just add a new line after ${workspaceName}! in spanish translation here

App/src/languages/es.js

Line 1044 in 42bea4b

welcomeNote: ({workspaceName}) => `¡Has sido invitado a ${workspaceName}! Descargue la aplicación móvil Expensify en use.expensify.com/download para comenzar a rastrear sus gastos.`,

Also I think we should make it consistent with en.js and also add a newline here:

App/src/languages/en.js

Line 1043 in 42bea4b

welcomeNote: ({workspaceName}) => `You have been invited to ${workspaceName || 'a workspace'}! Download the Expensify mobile app at use.expensify.com/download to start tracking your expenses.`,

also we could delete this style setting:

App/src/styles/styles.js

Line 2208 in 615a31c

minHeight: 115,

Result looks like this:

What alternative solutions did you explore? (Optional)

We could dynamically adjust numberOfLines with onChange handler as it's done here:

App/src/libs/ComposerUtils/index.js

Lines 11 to 15 in 6e6adb0

	function getNumberOfLines(maxLines, lineHeight, paddingTopAndBottom, scrollHeight) {
	let newNumberOfLines = Math.ceil((scrollHeight - paddingTopAndBottom) / lineHeight);
	newNumberOfLines = maxLines <= 0 ? newNumberOfLines : Math.min(newNumberOfLines, maxLines);
	return newNumberOfLines;
	}

[fedirjh]

@tienifr Thanks for the proposal. Your RCA is correct. However your solution is not clear , do you suggest that we update the height style or the number of lines ? I don't think updating number of lines will fix the problem

[fedirjh]

@alexxxwork Thanks for the proposal. I don't think your RCA is correct.

[alexxxwork]

@alexxxwork Thanks for the proposal. I don't think your RCA is correct.

@fedirjh Well, I thought the same for some time, but then I tried to reduce minHeight in style - it changes nothing.
numberOfLines works in RN component

You could see this in Expo in RN docs, but expo seems to be down right in the time
https://reactnative.dev/docs/textinput

[fedirjh]

numberOfLines works in RN component

@alexxxwork React native web supports numberOfLines it’s converted to rows

[alexxxwork]

numberOfLines works in RN component

@alexxxwork React native web supports numberOfLines it’s converted to rows

@fedirjh Well, I'm a bit inaccurate - minHeight actually set minimum height but not less then the height of lines in numberOfLines.
So RCA is numberOfLines AND minHeight

Updated proposal

[tienifr]

@tienifr Thanks for the proposal. Your RCA is correct. However your solution is not clear , do you suggest that we update the height style or the number of lines ? I don't think updating number of lines will fix the problem

@fedirjh sorry for the confusion, I meant to update the minHeight style.
The numberOfLines doesn't really have any impact when we already have the minHeight (since the height of the lines are always smaller than the minHeight, see here for context), so we can either remove the numberOfLines completely (won't cause issues) or update it along with the minHeight if we want consistency.

[fedirjh]

The numberOfLines doesn't really have any impact when we already have the minHeight (since the height of the lines are always smaller than the minHeight, see #7354 (comment) for context)

@tienifr I don't entirely agree with that. If we set numberOfLines, and the height of the input with that numberOfLines is greater than the minHeight, then the input's min height would match the height of the numberOfLines. check screen below :

---

I meant to update the minHeight style.
so we can either remove the numberOfLines completely (won't cause issues) or update it along with the minHeight if we want consistency.

@tienifr On Web, your proposed solution of dynamically changing the numberOfLines will only work if we remove the minHeight style. However, IOS doesn’t support the numberOfLines prop . How do you plan to handle these cases ? Can you please verify that your approach will not cause the regression mentioned in issue #7354 ?

Additionally, could you please update your proposal accordingly ?

---

@alexxxwork Thank you for the update. Your solution appears to be a workaround to me, as it does not address the root cause of the issue, nor it does solve the problem for wider screens.

Screen.Recording.2023-04-14.at.9.44.22.PM.mov

[alexxxwork]

Your solution appears to be a workaround to me

@fedirjh For this case I propose changing numberOfRows dynamically (alternative)

[fedirjh]

For this case I propose changing numberOfRows dynamically (alternative)

@alexxxwork That's basically what @tienifr has suggested in his proposal.

[alexxxwork]

For this case I propose changing numberOfRows dynamically (alternative)

@alexxxwork That's basically what @tienifr has suggested in his proposal.

You either make a workaround or make it automatically adaptive, not much to invent here.

It's not the same, I propose to calculate rows, not height.

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

• Fix for: Editing workspace name to 1to3 characters adding extra line in the optional message when inviting users in Spanish #17878
• Additional changes for #17202 #18611

If no regressions arise, payment will be issued on 2023-05-16. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] 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:
• [@fedirjh] 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:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] 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.
• [@michaelhaxhiu] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[parasharrajat]

There is a regression from this issue #18566 cc: @michaelhaxhiu

One more regression #18525.

[melvin-bot]

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

• Further changes for #17202 #18635

If no regressions arise, payment will be issued on 2023-05-23. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] 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:
• [@fedirjh] 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:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] 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.
• [@michaelhaxhiu] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[fedirjh]

@michaelhaxhiu As this is a new feature it does not require BZ checklist. I would suggest adding a regression test :

• Determine if we should create a regression test for this bug ✅

Regression Test Proposal

1. Go to Settings > Workspace
2. Go to the Manage members > Invite
3. Verify that the textInput expands when you add long text and it shrinks when you remove the text

Do we agree 👍 or 👎

[michaelhaxhiu]

The regression test looks simple and good to me!

I'm trying to make sense of the payments owed right now. Seems like we had a few regressions that likely disqualify the speed bonus? - but please correct me if I'm wrong as @Ollyws rose some points about the page being refactored around the same time as his PR being ready for review/merge.

I'd love a bud check from @Ollyws @fedirjh or @techievivek

bug reporter -- $250 @ayazhussain79
contributor -- $1000 @Ollyws
C+ -- $1000 @fedirjh

[Ollyws]

@michaelhaxhiu Yeah had a couple of regressions we fixed promptly. Looks good to me.

[michaelhaxhiu]

Thanks for your input. It's a bummer :( I can see ya'll worked hard on it but seems like we'll stick to the default pricing (no speed / regression free bonus this time). Next time!!

[fedirjh]

Seems like we had a few regressions that likely disqualify the speed bonus?

Yes we have two regression which, I guess, should reduce the C+ payment to 250$.

the page being refactored around the same time as his PR being ready for review/merge.

Actually @Ollyws raised that for the (- 50% penalty) as the PR got affected by other refactoring which made it surpasses the 9 days timeline penalty

So I think payment should :

• Bug reporter -> $250 @ayazhussain79
• Contributor -> $1000 @Ollyws
• C+ -> $250 @fedirjh

[Ollyws]

which made it surpasses the 9 days timeline penalty

It never surpassed the 9 days in the end, it was just a precaution.

[michaelhaxhiu]

That would mean just one 50% docking, so payments would be:

• Bug reporter -> $250 @ayazhussain79
• Contributor -> $1000 @Ollyws
• C+ -> $500 @fedirjh

👍

[michaelhaxhiu]

Old job link expired (thanks upwork... 😒 )

New job link - https://www.upwork.com/jobs/~01b18ce261bbb93719

Offered a contract to all 3 at the aforementioned price points.

[Ollyws]

@michaelhaxhiu accepted, thanks.

[ayazhussain79]

@michaelhaxhiu offer accepted

[michaelhaxhiu]

All payments issued.

[michaelhaxhiu]

All set. Closing.