[HOLD for payment 2023-08-01] [$1000] dev: Inconsistency in showing task description when the number of rows increases

[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. Create a task
2. Click description
3. Text: 1 to 7 each number each row => save
4. Observe that description only sees a part that can't be scrolled
5. Click description, clear previous description
6. Text long message => save
7. Observe that 3 dots are added to the description and can be scrolled

Expected Result:

There must be consistency in showing description when the number of rows increases

Actual Result:

How to show description is different when the number of rows increases

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
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: DEV
Reproducible in staging?: dev only
Reproducible in production?: dev only
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

Screen.Recording.2023-07-08.at.14.19.35.mov

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0116467439890d0404
• Upwork Job ID: 1679117287040221184
• Last Price Increase: 2023-07-12

[melvin-bot]

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

[melvin-bot]

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

[spcheema]

Proposed solution

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

Inconsistency in showing task description when the number of rows increases

What is the root cause of that problem?

The problem is that -webkit-line-clamp is not designed to work with numeric value alone to make a scrollable container with an ellipsis.

So whenever a numeric value is added it doesn't show ellipsis but text area is scrollable.

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

I dont think there is any need to do anything otherwise alternative solution is there

What alternative solutions did you explore? (Optional)

Option 1:
Set description container width 100% and it become full-width scrollable and shows ellipsis like text input.

Option 2
Based on this #22493 (comment)

• Remove max no. of lines from description
• Set title style to pre-wrap

            <MenuItemWithTopDescription
                description={props.translate('task.description')}
                title={props.report.description || ''}
                onPress={() => Navigation.navigate(ROUTES.getTaskReportDescriptionRoute(props.report.reportID))}
                shouldShowRightIcon={isOpen}
                disabled={!isOpen}
                titleStyle={styles.preWrap}
                wrapperStyle={[styles.pv2]}
                shouldGreyOutWhenDisabled={false}
            />

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0116467439890d0404

[melvin-bot]

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

[laurenreidexpensify]

Not overdue

[melvin-bot]

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

[mananjadhav]

Looping in @shawnborton. I don't see this as a real value issue as I don't see we'll be adding numeric list only for the description.

But one minor thing that's off is if we have a large description, we show ellipses, but the content is scrollable. I think that's intentional? But I am not sure. If that's the case, we should just close this one out.

[shawnborton]

Hmm this seems very strange. cc @jasperhuangg @thienlnam I forget where we landed with truncating the description row, but now I am wondering if we should never truncate description at all.

[namhihi237]

I think not just numbers only @mananjadhav @shawnborton

Screen.Recording.2023-07-13.at.00.35.32.mov

[namhihi237]

I my view I think we should not cut the description because in case the user wants to read the details of a task

[thienlnam]

I don't remember either, but I thought we standardized on like 3 rows or something. I agree we could not truncate so that you could view the entire description without having to push to page to edit

[namhihi237]

Proposal

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

There must be consistency in showing description, we will not truncate the description

What is the root cause of that problem?

Currently, we limit 3 rows of descriptions here:

App/src/components/ReportActionItem/TaskView.js

Lines 101 to 110 in aa53543

	<MenuItemWithTopDescription
	description={props.translate('task.description')}
	title={props.report.description || ''}
	onPress={() => Navigation.navigate(ROUTES.getTaskReportDescriptionRoute(props.report.reportID))}
	shouldShowRightIcon={isOpen}
	disabled={!isOpen}
	wrapperStyle={[styles.pv2]}
	numberOfLinesTitle={3}
	shouldGreyOutWhenDisabled={false}
	/>

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

I think we should not restrict truncated descriptions, this will make it difficult for users to see task details. They have to add 1 click to edit to see. In case the task is completed, the user cannot edit it and they will not be able to see the details of this task.
So we should remove numberOfLinesTitle={3}

What alternative solutions did you explore? (Optional)

N/A

[spcheema]

Hmm this seems very strange. cc @jasperhuangg @thienlnam I forget where we landed with truncating the description row, but now I am wondering if we should never truncate description at all.

Since we are heading to this direction i.e. not truncating the description then I'll update it in my alternative solution. Proposed solution does not require anything but close the issue :)

[spcheema]

Looping in @shawnborton. I don't see this as a real value issue as I don't see we'll be adding numeric list only for the description.

But one minor thing that's off is if we have a large description, we show ellipses, but the content is scrollable. I think that's intentional? But I am not sure. If that's the case, we should just close this one out.

Yes there is no value to fixing this issue just for the case of numbers. ellipses is intentional and along with scrollable. My only concern is that how a user will know if the content is scrollable.

[spcheema]

Hmm this seems very strange. cc @jasperhuangg @thienlnam I forget where we landed with truncating the description row, but now I am wondering if we should never truncate description at all.

I have captured different screenshot from mobile web with large task description ( honesty I am not sure what's would be the ideal length in the best)

If large description is less likely then we can remove max no. of lines condition or may be go with the upper bound something like X lines max which will show ellipsis on mobile (no scroll) and web is scrollable along with ellipsis.

Screenshots for different cases:

• Task description without any limit

• Maximum no. of lines :3

• Maximum no. of lines :5

• Maximum no. of lines : 10

[shawnborton]

I'm not really sure where the scrollable thing came from, that was certainly not intended.

Either way, let's just make it so that the read-only description field is not truncated. I don't know that we want that pattern everywhere, but let's make sure we do it for Tasks where it's important to read the whole description.

[spcheema]

@shawnborton Below are two screenshots took from mobile where one with no limit and another one is with maximum 10 lines

With no limit:

Maximum no. of lines : 10

[spcheema]

So it acceptable that C++ assume things you will do. That's strange and weird. Then why we asked to put up a proposal. You have clearly mentioned that you will make following change:

So we should remove numberOfLinesTitle={3}

It doesn't make proposal complete in my opinion. We go on assumptions basis then my proposal does say I'll fix the issue with obvious changes.

Well enjoy if this is the way.

[spcheema]

There were two changes you just mentioned removal part only. Since C++ and internal reviewer understand that you make sure things will work and assigned it. Then nothing can be done mate.

Cheers.

[namhihi237]

Hi @mananjadhav @stitesExpensify The PR is already for review. Thanks

[mananjadhav]

@spcheema Apologies for not responding sooner. For me the most important change was to remove the numberOfLines. We do work out the kinks in the PR. Based on what I saw in the history, etc. @namhihi237 pointed that out first.

So it acceptable that C++ assume things you will do. That's strange and weird.
Please test the changes locally before making a decision.

I am not keen on modifying the styles.preWrap. I should've clarified. But please have patience for the team to respond before jumping to conclusions, in case you need any clarifications

cc - @stitesExpensify

[spcheema]

@mananjadhav Please don't apologise. All good.

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

• remove limit and add max length task description #23246

If no regressions arise, payment will be issued on 2023-08-01. 🎊

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

For reference, here are some details about the assignees on this issue:

• @mananjadhav does not require payment (Eligable for Manual Requests)
• @namhihi237 requires payment

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:

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

[mananjadhav]

This is more like a feature request, hence we don't have a tagged PR.

But I think we should a regression test here, so that we don't end up adding length restriction on the description.

1. Open FAB, click on Assign Task
2. Create a task with a long description (about more than 2-3 paragraphs)
3. Open task detail and verify that can view the whole description without any ellipses.

@laurenreidexpensify Please note this PR had a regression, hence the payout will be deducted and there is no timeline bonus.

[laurenreidexpensify]

Payment Summary:

• External issue reporter @namhihi237 $250 paid in Upwork
• Contributor that fixed the issue @namhihi237 $1000 paid in Upwork
• Contributor+ that helped on the issue and/or PR $1000 Eligible for manual request @mananjadhav

[mananjadhav]

Please note this PR had a regression, hence the payout will be deducted and there is no timeline bonus.

@laurenreidexpensify There would be a deduction of 500 in this PR. The actual payout for me and @namhihi237 is $500.

[JmillsExpensify]

Yes agreed. Mind updating your summary @laurenreidexpensify?

[laurenreidexpensify]

Updated Payment Summary:

• External issue reporter @namhihi237 $250 paid in Upwork
• Contributor that fixed the issue @namhihi237 $500 paid in Upwork (Expensify has requested $500 refund for overpayment)
• Contributor+ that helped on the issue and/or PR $500 Eligible for manual request @mananjadhav

[JmillsExpensify]

Reviewed details for @mananjadhav. These details are accurate based on summary from Business Reviewer and are now approved for payment in NewDot.