Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] Workspace - The invite message can't fit into the textbox #37406

Closed
2 of 6 tasks
izarutskaya opened this issue Feb 28, 2024 · 29 comments
Closed
2 of 6 tasks

[$500] Workspace - The invite message can't fit into the textbox #37406

izarutskaya opened this issue Feb 28, 2024 · 29 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Design External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@izarutskaya
Copy link

izarutskaya commented Feb 28, 2024

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


Found when validating PR : #29991

Version Number: 1.4.44.0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal Team

Action Performed:

  1. Log in with a Gmail account
  2. Create a new workspace
  3. Tap on the "Invite members" button
  4. Select any suggested member
  5. Tap on the Invite button

Expected Result:

The invite message should fit into the textbox.

Actual Result:

The invite message can't fit into the textbox.

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

Bug6395091_1709098760283!capture

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01561b4140b01dff6c
  • Upwork Job ID: 1763716186253737984
  • Last Price Increase: 2024-03-02
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 28, 2024
Copy link

melvin-bot bot commented Feb 28, 2024

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

@izarutskaya
Copy link
Author

@mallenexpensify I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

@izarutskaya
Copy link
Author

We think that this bug might be related to #wave8-collect-admins
CC @zanyrenney

@melvin-bot melvin-bot bot added the Overdue label Mar 1, 2024
@mallenexpensify mallenexpensify added the External Added to denote the issue can be worked on by a contributor label Mar 2, 2024
@melvin-bot melvin-bot bot changed the title Workspace - The invite message can't fit into the textbox [$500] Workspace - The invite message can't fit into the textbox Mar 2, 2024
Copy link

melvin-bot bot commented Mar 2, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01561b4140b01dff6c

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 2, 2024
Copy link

melvin-bot bot commented Mar 2, 2024

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

@mallenexpensify
Copy link
Contributor

Think it can be external. @paultsimura , let me know if you disagree.
Checking with Wave 6, collect submitter for project
https://expensify.slack.com/archives/C02MW39LT9N/p1709337922878799

@paultsimura
Copy link
Contributor

Think it can be external.

Yes, it seems like a regular FE task 👍

@abzokhattab
Copy link
Contributor

abzokhattab commented Mar 2, 2024

Proposal

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

Workspace - The invite message can't fit into the textbox

What is the root cause of that problem?

we use autoGrowHeightMultilineInput in the invite textfield props which has a max height of 115 however the invite message doesn't fit into the specified max height

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

we can change the max height value to 132 or even some higher values:

POC

image

@mallenexpensify
Copy link
Contributor

Checking in Wave 8, think it's better https://expensify.slack.com/archives/C036QM0SLJK/p1709340832943519

@dukenv0307
Copy link
Contributor

dukenv0307 commented Mar 2, 2024

Proposal

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

The invite message can't fit into the textbox.

What is the root cause of that problem?

In here, we're using a fixed maxHeight for the invite message, but the default invite message could be longer than that, thus the message looks clipped as in the OP.

The correct behavior here should be that the maxHeight will always be equal to the height of the initial default welcome message (optionally plus 4, because right now the height of the initial default welcome message of normal height is 111, and the maxHeight set is 115). So that when the user lands on the invite message screen, the message will always fit 100%.

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

  1. Extract this to a reusable component AutoGrowHiddenText, which will accept all normal params and also a onSizeChange(width, height)
  2. Introduce a new prop maxHeightBasedOnDefaultValue prop to TextInput, if this has value, we'll render the AutoGrowHiddenText with the text value using maxHeightBasedOnDefaultValue, and once the height is known, set it to a state and use it here instead of the StyleSheet.flatten(containerStyles).maxHeight
  3. Set that maxHeightBasedOnDefaultValue prop to getDefaultWelcomeNote() or translate('workspace.inviteMessage.welcomeNote', {workspaceName: policy?.name ?? '',}) in here

POC branch for testing will be available upon request.

What alternative solutions did you explore? (Optional)

Use another method to get the height from the default welcome text like in here, to pass as maxHeight of containerStyles

Customizing the fixed max height in here (Note: autoGrowHeightMultilineInput should not be touched because it will break other parts of the app) to a higher value could also work but it will break easily if we change the invite message format in the future, also it will regress the behavior of maxHeight for shorter invite message.

Result

Screen.Recording.2024-03-02.at.1.49.17.PM.mov

@ghost
Copy link

ghost commented Mar 2, 2024

Proposal

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

Workspace - The invite message can't fit into the textbox

What is the root cause of that problem?

Here,

containerStyles={[styles.autoGrowHeightMultilineInput]}

we are passing autoGrowHeightMultilineInput, which has a max height of maxHeight: 115which is basically clipping the text and because of this the invite message does not fit inside the textbox.

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

To solve this issue, I can see that the app behaves by having a scrollbar in the textbox on all the other platforms i.e.

  • Android: Native
  • iOS: Native
  • MacOS: Chrome / Safari
  • MacOS: Desktop

So, the best solution is that we can add overflow: scroll here :

App/src/styles/index.ts

Lines 3124 to 3126 in a509234

autoGrowHeightMultilineInput: {
maxHeight: 115,
},

What alternative solutions did you explore? (Optional)

Other, than that we can do one more thing is we can make the textbox, height dynamic i.e. it changes according to the text input.

Result

Screen.Recording.2024-03-02.at.3.16.40.PM.mp4

@paultsimura
Copy link
Contributor

paultsimura commented Mar 2, 2024

After taking a closer look – I'm not sure it's something we need to fix.
The issue is that the welcome message is taller than the defined max height, and the message can be of any height, so there's no sense to just increase the max height to some other fixed value.

However, the field is scrollable. Do we want to change this behavior? And if yes – what would be the expected outcome?

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-03-02.at.12.43.51.mp4

cc @mallenexpensify @Expensify/design

@ghost
Copy link

ghost commented Mar 2, 2024

After taking a closer look – I'm not sure it's something we need to fix. The issue is that the welcome message is taller than the defined max height, and the message can be of any height, so there's no sense to just increase the max height to some other fixed value.

However, the field is scrollable. Do we want to change this behavior? And if yes – what would be the expected outcome?

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-03-02.at.12.43.51.mp4
cc @mallenexpensify @Expensify/design

@paultsimura, As stated above, I can understand that and see that. Also, my solution revolves more around making it scrollable as I have also stated that this kind of behaviour is experienced in the app on other platforms like Web, Native and Desktop etc. Please check out my proposal if possible for you

Copy link

melvin-bot bot commented Mar 3, 2024

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

@mallenexpensify
Copy link
Contributor

@shawnborton looping in design for 👀.
Also, I wasn't able to reproduce on iOS Safari

IMG_1208

One possible discussion point, if we end up having too much text and the user needs to scroll, when the user first sees the message, do we want to show them the top of the text or the bottom, assuming they're able to scroll down.

@paultsimura
Copy link
Contributor

Also, I wasn't able to reproduce on iOS Safari

Just duplicate your invite message to make it 2x as long

@dukenv0307
Copy link
Contributor

dukenv0307 commented Mar 4, 2024

The issue is that the welcome message is taller than the defined max height, and the message can be of any height, so there's no sense to just increase the max height to some other fixed value.

@paultsimura I think the problem here is that the "default invite message" is clipped when it's shown to the user, it's supposed to always be showing in full when the user opens that page (if not the user will have to scroll to read the full text). The current max height was set based on that assumption too.

Just duplicate your invite message to make it 2x as long

If the user edits the invite message and it becomes longer than the text box, it's not a problem because at the time the user already reads the full text of the "default invite message"

@ghost
Copy link

ghost commented Mar 4, 2024

The issue is that the welcome message is taller than the defined max height, and the message can be of any height, so there's no sense to just increase the max height to some other fixed value.

@paultsimura I think the problem here is that the "default invite message" is clipped when it's shown to the user, it's supposed to always be showing in full when the user opens that page (if not the user will have to scroll to read the full text). The current max height was set based on that assumption too.

Just duplicate your invite message to make it 2x as long

If the user edits the invite message and it becomes longer than the text box, it's not a problem because at the time the user already reads the full text of the "default invite message"

But I think making it scrollable is a better alternative as in all the other platforms we are just making the invite message as scrollable. After that we can add as much text in the invite message as possible and it won't effect anything and also it would maintain consistency throughout the app on all the platforms.

@paultsimura
Copy link
Contributor

I think making it scrollable is a better alternative as in all the other platforms we are just making the invite message as scrollable

From what I see – we have this message scrollable on all platforms. Could you please share the recordings on what platforms the message is not scrollable?

@paultsimura
Copy link
Contributor

If the user edits the invite message and it becomes longer than the text box, it's not a problem because at the time the user already reads the full text of the "default invite message"

If I modify the message, leave the page, and go back to it – the new custom message will be "default" for all future invitations. So this is not necessarily related to the default invitation message set by Expensify.

Screen.Recording.2024-03-04.at.12.50.31.mov

@ghost
Copy link

ghost commented Mar 4, 2024

I think making it scrollable is a better alternative as in all the other platforms we are just making the invite message as scrollable

From what I see – we have this message scrollable on all platforms. Could you please share the recordings on what platforms the message is not scrollable?

Yes yes I know that and that's what I am saying that we should make the mweb Chrome : Android and mweb Safari : IOS scrollable for invite message textbox

@paultsimura
Copy link
Contributor

Yes yes I know that and that's what I am saying that we should make the mweb Chrome : Android and mweb Safari : IOS scrollable for invite message textbox

@godofoutcasts94 please try to keep the communication productive. From what I see, all the platforms (including both mWeb) already have this field scrollable. If you have a different experience - please provide the screen recording.

@ghost
Copy link

ghost commented Mar 4, 2024

Yes yes I know that and that's what I am saying that we should make the mweb Chrome : Android and mweb Safari : IOS scrollable for invite message textbox

@godofoutcasts94 please try to keep the communication productive. From what I see, all the platforms (including both mWeb) already have this field scrollable. If you have a different experience - please provide the screen recording.

oops my bad! I am sorry. I had some misunderstanding. @paultsimura I am extremely sorry.

@paultsimura
Copy link
Contributor

@shawnborton just so you don't have to scroll through all the messages.

TD;DR:
The issue is that the welcome message is taller than the defined max height, and the message can be of any height, so there's no sense to just increase the max height to some other fixed value.

However, the field is scrollable. Do we want to change this behavior? And if yes – what would be the expected outcome?

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-03-02.at.12.43.51.mp4

#37406 (comment)
#37406 (comment)

@dannymcclain
Copy link
Contributor

I'm down to wait for the other designers to chime in, but I think this is expected behavior and isn't something we need to do anything about. Looks to be working like a normal textarea input, which I think is a good and fine thing.

One possible discussion point, if we end up having too much text and the user needs to scroll, when the user first sees the message, do we want to show them the top of the text or the bottom, assuming they're able to scroll down.

@mallenexpensify I think it kinda depends on whether the field is focused or not. If the field is focused, we should continue to default to the end/bottom, because that's where we place your cursor in the input (I assume). If the field is not focused, then I could see an argument for defaulting to the top of the message, but honestly think either is fine.

@shawnborton
Copy link
Contributor

Yeah I agree with Danny here - this seems to be standard textarea behavior.

Did we recently change the max height or something?

@paultsimura
Copy link
Contributor

Did we recently change the max height or something?

Nope. I think the issue reporter just had a relatively small screen and a relatively long workspace name, so the default message overflowed the pre-defined textarea height.

So would you suggest to close this issue? @shawnborton @dannymcclain

@shawnborton
Copy link
Contributor

I would be good with that, yes.

@mallenexpensify
Copy link
Contributor

Thanks all, closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Design External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

7 participants