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

[HOLD for payment 2023-09-27] [$1000] Feature request: Hyperlink any URLs that appear in request descriptions #23949

Closed
JmillsExpensify opened this issue Jul 31, 2023 · 60 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@JmillsExpensify
Copy link

JmillsExpensify commented Jul 31, 2023

While we hyperlink URLs that appear in chat messages, we don't do the same for URLs that appear in the description field of a request. Let's close that gap, because it's frustrating to have the hyperlink in one place but not in the other.

Screenshot 2023-07-31 at 13 11 46

Assigning myself as part of wave3 in the product roadmap.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e1f1d0e52e445d5f
  • Upwork Job ID: 1689124762759274496
  • Last Price Increase: 2023-09-06
  • Automatic offers:
    • jeet-dhandha | Contributor | 26670169
@JmillsExpensify JmillsExpensify added Weekly KSv2 NewFeature Something to build that is a new item. labels Jul 31, 2023
@JmillsExpensify JmillsExpensify self-assigned this Jul 31, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 31, 2023

Current assignee @JmillsExpensify is eligible for the NewFeature assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Jul 31, 2023

Triggered auto assignment to Design team member for new feature review - @shawnborton (NewFeature)

@jeet-dhandha
Copy link
Contributor

Proposal

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

  • Feature Request to allow URL in Description field of Tasks

What is the root cause of that problem?

  • Currently no implementation is done to handle URL in Description field of Tasks.

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

  1. We will use ExpensiMark as parser and parse the description to convert to HTML.
  2. Then we will pass html to description field's MenuItemWithTopDescription component.
  3. We will update MenuItemWithTopDescription component to render html instead of title.
  4. We will update MenuItem component to render html instead of title if html is passed.
{props.html ? <RenderHTML html={`<comment>${props.html}</comment>`} /> : ...title code}
After changes Screenshot 2023-08-01 at 1 29 45 AM

What alternative solutions did you explore? (Optional)

  • N/A

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 31, 2023
@JmillsExpensify
Copy link
Author

@shawnborton removing you as I think we're on the same page .

@melvin-bot
Copy link

melvin-bot bot commented Aug 4, 2023

@JmillsExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot
Copy link

melvin-bot bot commented Aug 8, 2023

@JmillsExpensify Still overdue 6 days?! Let's take care of this!

@JmillsExpensify JmillsExpensify added the External Added to denote the issue can be worked on by a contributor label Aug 9, 2023
@melvin-bot melvin-bot bot changed the title Feature request: Hyperlink any URLs that appear in request descriptions [$1000] Feature request: Hyperlink any URLs that appear in request descriptions Aug 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 9, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 9, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 9, 2023

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

@JmillsExpensify
Copy link
Author

Whoops, I just realized that I should have put an external label on this issue. We have one proposal above, so at least that is ready for review.

@melvin-bot melvin-bot bot removed the Overdue label Aug 9, 2023
@eh2077
Copy link
Contributor

eh2077 commented Aug 9, 2023

@JmillsExpensify Do we just only want to add support for hyperlink in task description or support all Markdown styling features that chat supports?

@WikusKriek
Copy link
Contributor

WikusKriek commented Aug 9, 2023

Proposal

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

We want to hyperlink any URLs that appear in request descriptions

What is the root cause of that problem?

We do not handle any markup in the description section of the request.

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

We should make use of ReportUtils and write a function that uses ExpensiMark parser to parse markdown link and autolink styling. We can add that function to the report utils.

function getParsedLink(text) {
        const parser = new ExpensiMark();
        const autolinkFilter = {filterRules: _.filter(_.pluck(parser.rules, 'name'), (name) => name === 'autolink' || name === 'link')};
        return text.length < CONST.MAX_MARKUP_LENGTH ? parser.replace(text, autolinkFilter) : _.escape(text);
    }
const formattedTransactionDescription = getParsedLink(transactionDescription);

Then we pass that formated description to the title of MenuItemWithTopDescription in the code below.

<MenuItemWithTopDescription
description={translate('common.description')}
title={transactionDescription}
disabled={isSettled}
// shouldShowRightIcon={!isSettled}
// onPress={() => Navigation.navigate(ROUTES.getEditRequestRoute(props.report.reportID, CONST.EDIT_REQUEST_FIELD.DESCRIPTION))}
/>

We can then add another view here to handle the rendered html specifically based on a prop we pass through:

<View style={[styles.flexRow, styles.alignItemsCenter]}>
{Boolean(props.title) && (
<Text
style={titleTextStyle}
numberOfLines={props.numberOfLinesTitle || undefined}
>
{convertToLTR(props.title)}
</Text>
)}
{Boolean(props.shouldShowTitleIcon) && (
<View style={[styles.ml2]}>
<Icon
src={props.titleIcon}
fill={themeColors.iconSuccessFill}
/>
</View>
)}
</View>

or replace the Title Text section in line 207 with:

<RenderHTML html={`<comment>${props.title}</comment>`} />

What alternative solutions did you explore? (Optional)

function getParsedLink(text) {
        const parser = new ExpensiMark();
        const autolinkFilter = ['autolink', 'link']
        return text.length < CONST.MAX_MARKUP_LENGTH ? parser.replace(text, autolinkFilter) : _.escape(text);
    }

@JmillsExpensify
Copy link
Author

Do we just only want to add support for hyperlink in task description or support all Markdown styling features that chat supports?

Great question. I think we want to keep it simple for now, and just focus on supporting hyperlinks. cc @shawnborton @trjExpensify in case ya'll are thinking about it differently.

@WikusKriek
Copy link
Contributor

Proposal

Updated

@trjExpensify
Copy link
Contributor

Yeah, I think it's cool to tackle hyperlinks as a first step and not bloat this issue.

@melvin-bot melvin-bot bot added the Overdue label Aug 14, 2023
@JmillsExpensify
Copy link
Author

@thesahindia Mind commenting on the proposals thus far?

@melvin-bot
Copy link

melvin-bot bot commented Sep 19, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot
Copy link

melvin-bot bot commented Sep 19, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Sep 20, 2023
@melvin-bot melvin-bot bot changed the title [$1000] Feature request: Hyperlink any URLs that appear in request descriptions [HOLD for payment 2023-09-27] [$1000] Feature request: Hyperlink any URLs that appear in request descriptions Sep 20, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Sep 20, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 20, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot
Copy link

melvin-bot bot commented Sep 20, 2023

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

If no regressions arise, payment will be issued on 2023-09-27. 🎊

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:

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 melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Sep 26, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 29, 2023

@JmillsExpensify, @narefyev91, @thienlnam, @jeet-dhandha Whoops! This issue is 2 days overdue. Let's get this updated quick!

@jeet-dhandha
Copy link
Contributor

Hold for payment

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 29, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

@JmillsExpensify, @narefyev91, @thienlnam, @jeet-dhandha Eep! 4 days overdue now. Issues have feelings too...

@thienlnam
Copy link
Contributor

This is ready for payment cc @JmillsExpensify

@melvin-bot melvin-bot bot removed the Overdue label Oct 5, 2023
@jeet-dhandha
Copy link
Contributor

jeet-dhandha commented Oct 6, 2023

@JmillsExpensify Let's cancel my payment as its been too long for payment and also Regressions were happening due to less testing and discussion on both of our ends.

Let's make this a learning to not implement any feature until all the cases are covered.

@JmillsExpensify
Copy link
Author

To clarify, you don't wish to be paid for this issue/PR?

@jeet-dhandha
Copy link
Contributor

Nothing just handle the payment as usual.

@JmillsExpensify
Copy link
Author

Payment summary:

@JmillsExpensify
Copy link
Author

Offer has been sent via Upwork.

@jeet-dhandha
Copy link
Contributor

@JmillsExpensify I got payment from service account for $1000 i need just extra $500 payment.
Screenshot 2023-10-08 at 7 10 13 PM

@jeet-dhandha
Copy link
Contributor

@JmillsExpensify Can you update the price of new contract to $500 ?
Screenshot 2023-10-08 at 7 12 00 PM

@JmillsExpensify
Copy link
Author

Will do! Updating now.

@JmillsExpensify
Copy link
Author

Should be all set here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests

10 participants