[$2000] No padding below in add personal message on adding of new line on web

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open the app on web chrome or web safari
2. Open settings
3. Open workspaces
4. Open any workspace
5. Open manage member
6. Click on invite
7. Add new text below current personal message

Expected Result:

App should keep some padding below text on web chrome and web safari as it does on android app, android chrome

Actual Result:

App does not provide any padding below text on web chrome and web safari

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• [ ] Android / native
• [ ] Android / Chrome
• [ ] iOS / native
• [ ] iOS / Safari
• [x] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.3.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

https://user-images.githubusercontent.com/43996225/232320495-f1926979-5ec6-4378-8530-993961a205d8.mp4

https://user-images.githubusercontent.com/43996225/232320497-e5656c06-12dd-44f0-b14d-b935225c129e.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b88f4f183ff1d0c5
• Upwork Job ID: 1648671192667062272
• Last Price Increase: 2023-04-26

[MelvinBot]

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

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] 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
• [x] 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.
• [x] 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.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[twisterdotcom]

Asking in Slack whether this truly is a bug: https://expensify.slack.com/archives/C049HHMV9SM/p1681549363487719

[twisterdotcom]

Okay, looks like we're gonna go with Bug! Let's make the padding below this cursor consistent across platforms folks.

[MelvinBot]

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

[MelvinBot]

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

[AmjedNazzal]

Proposal

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

No padding below in add personal message on adding of new line on web

What is the root cause of that problem?

The app has a conditional style that is applied if hasLabel is false or if multiline is true, in this case multiline is indeed true so we remove the padding like this:

https://github.com/Expensify/App/blob/d19326ec58085c230eeccbe3e6f4dd9cbef38ae4/src/components/TextInput/BaseTextInput.js#L295-L298

It's what's removing the padding bottom which is the issue in the post here and as you can see in the screenshots from web and ios indeed there is no padding bottom and it applies to Close Account page as well like shown below:

Web and Ios

Same thing with Close Account page

But also if you would look at the welcomeNote initially filled into the TextInput, it looks like there is a padding but it's not, it's extra space from the minHeight applied to the container like so:

https://github.com/Expensify/App/blob/cb61143d2e164c67223ac95c234acd5f844ee8e0/src/styles/styles.js#L2606-L2608

This is why initially it looks like there is padding on the bottom but once you start typing underneath the welcomeNote, we exceed the minHeight and there is no longer extra space that looks like padding. On android however there seem to be an inconsistency between devices/Android versions where with some this minHeight initial empty space at the bottom is stretched down as you type like it's padding, the behaviour happens with some devices and some others behave normally like web and ios as I will show in the screenshots:

Samsung physical phone with Android 11:

Emulated Pixel 5 with Android 13:

Same emulated Pixel 5 with increased minHeight from 115 to 120:

Same emulated Pixel 5 but minHeight decreased to 108:

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

We can decrease the minHeight to 110 to make things more consistent and add a margin to multiline TextInputs to give some spacing underneath them

style={[
    styles.flex1,
    styles.w100,
    this.props.inputStyle,
    (!hasLabel || this.props.multiline) && [styles.pv0, {marginBottom: 8}],

workspaceInviteWelcome: {
    minHeight: 110,
},

Result

[MelvinBot]

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

[MelvinBot]

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

[dukenv0307]

Proposal

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

No padding below in add personal message on adding of new line on web

What is the root cause of that problem?

No padding bottom of TextInput if it has multiple line. https://github.com/Expensify/App/blob/74d5290dd95500b86f8b4f384b18b9aca87364a2/src/components/TextInput/BaseTextInput.js#L293

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

Add styles.pb1 after (!hasLabel || this.props.multiline) && styles.pv0 to set padding bottom of TextInput if it has multiple line

What alternative solutions did you explore? (Optional)

NA

Result

Screencast from 17-04-2023 17:34:57.webm

[Pujan92]

Proposal

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

No space between the container border and the textInputContainer content

What is the root cause of that problem?

We are not providing any bottom padding to the textInputContainer and seems incorrect minHeight we are providing for the textinput multiline occurrences.

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

Add the bottom padding of 8px to the textinput multiline for the consistency with normal textinput. With that, correct the height/minHeight for these multiline places by considering/calculating numberOfLines, lineHeight, paddingTop, paddingBottom, borderBottom.

• For invite textInput height should be 113px instead of 115px -- (4 numberOfLines*20 + paddingTop 23 + paddingBottom 8 + bordrBottom 2) https://github.com/Expensify/App/blob/4fefc48069f58809927b02755d7c56ade0bf4021/src/styles/styles.js#L2592-L2594
• For close account textinput height is 153px which is correct -- (6 numberOfLines*20 + paddingTop 23 + paddingBottom 8 + bordrBottom 2) https://github.com/Expensify/App/blob/4fefc48069f58809927b02755d7c56ade0bf4021/src/styles/styles.js#L2856-L2858

To achieve the above we need to add the bottom padding of 8px when the textInput is multiline with this.props.multiline && styles.pb2 below.

https://github.com/Expensify/App/blob/c552dce3fbffc70bea24bbfef313c623de5db4ea/src/components/TextInput/BaseTextInput.js#L232-L242

Result

Web

https://user-images.githubusercontent.com/14358475/235296114-058584a2-c057-4ae8-9aee-3b1f8b4fda17.mp4

iOS

https://user-images.githubusercontent.com/14358475/235296238-34d16e95-ed77-4e86-b51b-7755b1171e15.mp4

Android

https://user-images.githubusercontent.com/14358475/235296513-af2fecb2-09ef-442c-8b77-8086589f21ae.mp4

[parasharrajat]

Ok, I reproduced this bug. But I think the problem is why Android has padding. The rest of the platforms do not have padding and we are also not applying any padding to multiline TextInputs.

Let me hear your thoughts on that.

[AmjedNazzal]

I've tested and the problem is present in all platforms including android but because of different ways automatic spacings go the issue is less obvious on android but when adding the additional marginBottom I could see the difference on android as well and it makes the styling looks more consistent. When it comes to multiline TextInputs not having padding, the parent wrapper of the composer for example does have padding here:

textInputComposeSpacing: {
    paddingVertical: 5,
    ...flex.flexRow,
    flex: 1,
},

Meanwhile the textInput we are working on here, the wrapper parent element does not have a padding bottom. I tried adding padding bottom to it and it started behaving similar to the composer when it comes to spacing on the bottom but the whole text input is structured differently for different purpose so I don't see adding padding on the bottom for the parent to be a good thing because it changed the way the textInput looks adding extra space underneath the autofilled welcomeNote.

[parasharrajat]

I couldn't reproduce this issue on other platforms on Android has padding bottom for me. You are saying exactly the opposite of what I said above. Can you please show us screenshots to clarify. @AmjedNazzal

This issue is neither related to Composer nor do we want it to look the same as a composer.

I am curious to know why Android has padding when we have not applied padding-bottom styling to it.

All proposals should answer that in the root cause which is the real issue.

[AmjedNazzal]

I have edited my proposal to clarify everything I managed to figure out, I apologize for the long proposal and the number of photos attached but the issue is a bit difficult to explain.

[MelvinBot]

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

[thienlnam]

This is a pretty obscure bug, going to increase for some more eyes

[MelvinBot]

Upwork job price has been updated to $2000

[parasharrajat]

Thanks, for clarification @AmjedNazzal. I think it makes sense now. So the root cause is minHeight applied to the TextInput. IMO, the reason why this issue is happening on some Android devices but not all is due to the DPI of the device. Due to DPI, px can behave differently on devices. For the devices where lines fit perfecting the text input height without showing a scrollbar has no padding. But for the other where lines overlap you will see a padding to fit the same number of lines that will show the scrollbar on them.

I feel like we can't solve this problem with minHeight adjustment as a fixed value will not fit all devices. A proper solution will be to use responsive design(I don't have much experience in this) which keeps the looks the same across devices.

Thus, I think we can close this issue @thienlnam as it is a device-specific issue.

[AmjedNazzal]

It's true that adjusting minHeight is not a solution and a better solution would be to use a more responsive design kind of approach, I must point out however that I've tested on a new empty native project using TextInput and minHeight and it seemed like Android devices I test on all behaved normally without this issue. makes me think that there is a line of code somewhere that is causing the weird behaviour of minHeight spacing on some androids, perhaps it could be the autogrow being used. also it's worth pointing out that the issue of having no padding underneath the text for multiline inputs, although it seem by design it does look inconsistent since throughout the app all TextInputs will have spacing between the text and the green border underneath. I feel like when the multiline TextInput was coded in the developers ran into an issue where setting up padding caused the welcomeNote to initially look incomplete as the TextInput has a scrollbar and a part of the welcomeNote was being covered because of that, so perhaps they decided on removing padding and adding minHeight to make sure the welcomeNote shows fully and maybe gave it a bit extra minHeight so it doesn't stick very close to the bottom border. but this is just speculation on my part. however whatever might be the reason this decision was made, the text the user type does stick very close to the bottom border for chrome and ios.

[huzaifa-99]

F

[parasharrajat]

That make sense @AmjedNazzal can you share the Snack with me so that I can also test it out?

[AmjedNazzal]

@parasharrajat Sure thing, here's the Snack: https://snack.expo.dev/@amjednazzal/multiline-test

[Pujan92]

@parasharrajat as numberOfLines for the TextInput is only supported by the android, it only takes height of (numberOfLines * lineHeight) and rest it is ignoring(which we are seeing as a padding). Currently, minHeight is incorrectly given without considering bottom padding, that's why we are seeing an issue in other platforms.

For eg. for this invite textinput we have given height 115px, considering 4 numberOfLines which will required height of (4*20 lineheight) + paddingTop(23) + bottom border(2) = 105px. Still we have 10px more which we need to provide as bottom padding to make it consistent across all platforms. Currently, this 10px left by android which you are referring(bcoz it supports numberOfLines and this extra space doesn't require for it).

To prove for android, if I give the exact required minHeight(105) as derived above you will see the same issue.

One more example where minHeight increased and numberOfLines kept 4

https://user-images.githubusercontent.com/14358475/234937666-795d9f26-09b5-49e8-a30e-346b13d8e2f4.mp4

We can clearly see how extra space isn't used in android due to numberOfLines restriction.

I updated my proposal based on these findings.

[parasharrajat]

Thanks @Pujan92 for explanation but here I am saying that we don't need padding on Android.

[Pujan92]

@parasharrajat even though if we won't provide, it is considering it as a empty space, so I think giving correct padding won't make any harm. As I explained above without padding the height should be 105px, with this case we need padding for Android also.

[twisterdotcom]

@parasharrajat and @thienlnam - what's your honest opinion on this then? It seems like this is genuinely more hassle than it's worth to solve and that was kind of what I was worried about when I asked about it internally.

@michaelhaxhiu what do you think? This seems like something that will cause zero impact to the user, and that no real user will complain about. I'd prefer we close this and focus our efforts elsewhere, especially given the sheer volume of other bugs we're seeing recently.

I love the idea of parity between platforms, but I think given the weirdly complex nature here, we should just close.

[parasharrajat]

I agree to close it @twisterdotcom as I said here

[Pujan92]

I love the idea of parity between platforms, but I think given the weirdly complex nature here, we should just close.

Sorry to comment here but I think consistency can be achieved with minimal changes(as numberOfLines is not supported except android).

[michaelhaxhiu]

Looking

@Pujan92 btw your commentary is welcome on any GH, no need to apologize :)