[HOLD for payment 2024-09-11] ][$500] Lack of maximum height for the description field for room hides other options on the page

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.44-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
Expensify/Expensify Issue URL:
Issue reported by: @JmillsExpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1709046310280799

Action Performed:

1. Create a room in the workspace with lengthy description
2. Open the room and view the details of the room

Expected Result:

All the option in the details page should be visible restricting the max height of the description field

Actual Result:

Lack of a max height for the description field obscures other options

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b55a4d576f4c3f8a
• Upwork Job ID: 1762602581669859328
• Last Price Increase: 2024-03-26
• Automatic offers:
• shubham1206agra | Reviewer | 0
• brandonhenry | Contributor | 0

Issue Owner

Current Issue Owner: @greg-schroeder

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[rayane-djouah]

Proposal

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

Lack of maximum height for the description field for room hides other options on the page.

What is the root cause of that problem?

Here:

App/src/pages/ReportDetailsPage.tsx

Lines 245 to 253 in b447c11

	<MenuItemWithTopDescription
	shouldShowRightIcon={canEditReportDescription}
	interactive={canEditReportDescription}
	title={report.description}
	shouldRenderAsHTML
	shouldCheckActionAllowedOnPress={false}
	description={translate('reportDescriptionPage.roomDescription')}
	onPress={() => Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report.reportID))}
	/>

We are rendering full description with shouldRenderAsHTML

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

we should remove shouldRenderAsHTML prop and add numberOfLinesTitle={4} prop with appropriate maximum number of lines.
We can use windowWidth to define dynamic maximum number of lines.

[ShridharGoel]

Proposal

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

For lengthy descriptions, other options go out of the screen.

What is the root cause of that problem?

There is no height limit or lines limit in the below code in MenuItem

{!!title && (shouldRenderAsHTML || (shouldParseTitle && !!html.length)) && (
                                                <View style={styles.renderHTMLTitle}>
                                                    <RenderHTML html={getProcessedTitle} />
                                                </View>
                                            )}

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

We can pass a prop from details page, which can be set as a max height limit in the above code to ensure that description height is limited.

[jeremy-croff]

It's not gonna be that easy guys :P
context #34150 (comment)
Seems we want a solve here for truncating html

[jeremy-croff]

Proposal

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

We want to limit the number of lines for html content

What is the root cause of that problem?

Our migration from #34150 (comment) is anticipating markdown support. A choice was made during PR to go with no max lines for this HTML we are rendering as we do not have this feature yet.

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

I think as we start supporting more markdown/html we need more tooling to parse over this.
We can leverage a library such as https://github.com/arendjr/text-clipper or implement our own to clip html

What alternative solutions did you explore? (Optional)

I also tried to solve it in a hackish way, setting a max height, overflowing, and adding ellipses with a pseudo element. It's ugly.

[bernhardoj]

Proposal

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

There is no maximum line for the report description.

What is the root cause of that problem?

Simply we don't limit the maximum line. The report description is rendered with a RenderHTML component and there is no straightforward way to set the number of lines props to the text.

App/src/components/MenuItem.tsx

Lines 515 to 519 in 98461f9

	{!!title && (shouldRenderAsHTML || (shouldParseTitle && !!html.length)) && (
	<View style={styles.renderHTMLTitle}>
	<RenderHTML html={getProcessedTitle} />
	</View>
	)}

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

One way is to wrap the description with a new custom tag that will have a custom renderer that can receive a number of lines attribute.

1. Create a new TextRenderer

function TextRenderer({TDefaultRenderer, textProps, ...props}) {
    return <TDefaultRenderer {...props} textProps={{...textProps, numberOfLines: props.tnode.attributes.line}} />
}

2. Register it to the custom renderer in HTMLRenderers/index.ts

text: TextRenderer,

and in BaseHTMLEngineProvider

text: HTMLElementModel.fromCustomModel({tagName: 'text', contentModel: HTMLContentModel.textual}),

3. In MenuItem, wrap the HTML with <text> tag.

return processedTitle ? `<comment><text line="${numberOfLinesTitle}">${processedTitle}</text></comment>` : '';

4. In ReportDetailsPage, pass numberOfLinesTitle with the agreed value

numberOfLinesTitle={3}

Why we can't create a custom renderer for <comment> tag? That's because comment is a block model, not textual which won't receive textProps

[dragnoir]

Proposal

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

Full description displayed.

What is the root cause of that problem?

We are using RenderHTML to display the discription. The component does not support clipping text.

App/src/components/MenuItem.tsx

Line 517 in dcf2e3e

<RenderHTML html={getProcessedTitle} />

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

Remove the option to render the description as HTML and just render as Text.

What alternative solutions did you explore? (Optional)

We can wrap the component with a Text and numberOfLines={1} prop.

+ <Text  numberOfLines={1}>
  <RenderHTML  html={getProcessedTitle}  />
+ </Text>
/>

[sarahRosannaBusch]

Proposal

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

When a room description is long, it pushes menu items off the screen so the user has to scroll down to see them. This happens on all platforms where the screen/window is too short to display everything in the details panel.

What is the root cause of that problem?

The room description's height is variable because it is determined by the length of the user input.

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

Move the room description below the menu items in ReportDetailsPage.tsx. This ensures the menu items are always visible and the layout stays consistent. The bottom of long descriptions will still be off the screen, but it will be visually obvious that the user can scroll down to see the full text.

This solution is very simple to implement and doesn't require any adding any tools, functionality, or conditions, so it won't cause regressions. However, if there are design docs or user manuals that show this screen, they will need to be updated.

What alternative solutions did you explore? (Optional)

• limit number of lines being displayed; this would unnecessarily hide data on screens that are large enough to display the full description
• make the description view height flexible to fit in the available space; this text area would still either need to be scrollable or collapsible for long descriptions, which looks messy and is problematic because it is already acting as a button to open the description editor
• make CONST.REPORT_DESCRIPTION.MAX_LENGTH smaller; this would be restrictive for users who want a long description, and wouldn't reliably fix the issue since users can increase their font sizes for accessiblity

[melvin-bot]

📣 @sarahRosannaBusch! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[sarahRosannaBusch]

Contributor details
Your Expensify account email: annarose0113@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/sarahrosannabusch

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[ishpaul777]

#37357 (comment) - @dragnoir sorry but i dont find you solution working as we expect i.e. render html as it is now just have a max height/ limit for number of lines

#37357 (comment) - @bernhardoj do you mind sharing a test branch(doesn't need to polished just a working POC)

#37357 (comment) - @ShridharGoel can you please elaborate where do you want to use the maxheight prop in codebase please give minimum details so it can be tested easily

#37357 (comment) - @rayane-djouah As per require we want to render the text as html not normal text

#37357 (comment) - @jeremy-croff i dont see a solution in your proposal but a library suggestion without any detail on how it can be integrated also if you want to propose a new library please follow this template for your proposal

[bernhardoj]

@ishpaul777 sure, here is the test branch

[ishpaul777]

Thanks for sharing the branch @bernhardoj I'll evaluate and test branch later today 🙇‍♂️

[melvin-bot]

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

[melvin-bot]

@greg-schroeder, @ishpaul777 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ishpaul777]

testing changes from @bernhardoj branch, will update asap

[brandonhenry]

@shubham1206agra @greg-schroeder @puneetlath looks like this one is blocking and I see the issue

shouldTruncateTitle is always set to true, but we should only truncate when title is > 200 characters. I can open a new PR to fix this

[puneetlath]

Got it. Yes, please open a PR, thanks!

[brandonhenry]

@puneetlath i tried to create a PR and it said I wasn't a collaborator? Did that connection break at some point?

[puneetlath]

@brandonhenry hmm I'm not sure, but actually let's hold off for a sec. I'm not sure that's the best solution. Let me think about it a bit without being rushed for the deploy.

[brandonhenry]

@puneetlath okay I got the PR all setup here: #48098

[puneetlath]

@brandonhenry I'm going to let @nkdengineer raise the PR since they found the proper root cause here. I don't think your solution will work in the scenario where it's longer than 200 characters, but doesn't contain any HTML.

[shubham1206agra]

@Expensify/design @puneetlath Can I do the enhancement in the room description? Right now, clicking on the image does nothing. I am proposing attachment modal similar to what we have in private notes.

[puneetlath]

We support images in room descriptions?

[shubham1206agra]

We support images in room descriptions?

Yes, we support all types of markdown

[puneetlath]

Ahh I see. I say let's live with it for a bit to see how we like it before adding more enhancements.

[shubham1206agra]

@puneetlath This is ready for payment. Can you add the labels?

[greg-schroeder]

Hmm - was this not picked up by automation or something? @puneetlath do you know?

[mallenexpensify]

@greg-schroeder automation has been failing often lately (Rory is working on a fix!)
The PR was deployed to production, automation failed to post it though, the way to check is to look at the deploy checklist, if the checklist is closed, it's been deployed to production. Labels, title and issue owner updated

[greg-schroeder]

Payment summary:

Contributor: @brandonhenry - $500 - Offer sent via Upwork
C+: @shubham1206agra - $500 - Offer sent via Upwork

[puneetlath]

@greg-schroeder I discussed with both Brandon and Shubham and re-checked the PRs. We had multiple scope increases, so I think we should update the payout to be $1250.

[brandonhenry]

@greg-schroeder @puneetlath thanks, let me know if I need to do anything here

[greg-schroeder]

Oh, word. All right - I didn't realize as the issue title still shows $500. I can update the offers.

[shubham1206agra]

@greg-schroeder Bump here

[greg-schroeder]

Hmm - I updated it - did it not go through? I can try again

[greg-schroeder]

Tried again. Modified offer for @shubham1206agra - added bonus payment for @brandonhenry

[shubham1206agra]

@greg-schroeder I accepted the offer.