-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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-05-22] [$2000] Update ExpensiMark to recognize mentions #17882
Comments
Current assignee @puneetlath is eligible for the NewFeature assigner, not assigning anyone new. |
Job added to Upwork: https://www.upwork.com/jobs/~01227cf510add7dd82 |
Current assignee @puneetlath is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav ( |
Current assignee @puneetlath is eligible for the External assigner, not assigning anyone new. |
ProposalPlease re-state the problem that we are trying to solve in this issue.Make ExpensiMark recognize mentions to replace to required mention web components. What is the root cause of that problem?N/A this is kind of a new feature. What changes do you think we should make in order to solve the problem?We can use two separate mention regexes and add them accordingly, to replace the mentions with required html with the content. the following regex handles the cases for the mention examples provided above. const mentionUserRegex = /(^|\s)@([\w.@+-]+(?:\.[\w@+-]+)+)\b/g;
const mentionHereRegex = /(^|\s)@here\b/g; What alternative solutions did you explore? (Optional)N/A |
Thanks for the proposal @getusha. I think your proposal works, I need to verify it once. I'll update here once I am done. Meanwhile,
|
Yes, this is just the ExpensiMark part of the solution. The actual rendering of |
For the mention-user regex, we may want to consider leveraging the email regex we already have here. |
@mananjadhav We can put the mention rules above every other rules by doing that we will insure that it will only treats it as mention before other replacement. or just below the |
Going to up the bounty on this since this is a high-priority roadmap issue. Given the timeline, I'm going to go ahead and assign @getusha as well. I think the proposal seems generally good and we're close enough to an agreement that we can work out the kinks in the PR. @getusha will you be able to raise a PR asap? |
📣 @getusha You have been assigned to this job by @puneetlath! |
Upwork job price has been updated to $2000 |
I will raise the PR in short ASAP. thanks! |
We're actively working through the PR. Good progress. |
PR is in good shape, but still on hold until we deploy the custom renderers. |
User renderer got merged. Just need to wait for the Here renderer now and then this will be able to go off hold. |
@puneetlath, @mananjadhav, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
PR is still on hold. Will likely be able to merge it tomorrow. |
@getusha I commented on the PR, but we need to get your commits signed. If you can do that today then we can merge the PR asap. Thanks! |
This is live on staging. |
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.13-5 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-05-22. 🎊 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.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
Upwork offers sent. |
@puneetlath There won't be any checklist for this one as this a feature request. what do you think about regression test? do we add that for new features? I mean are they added during the design phase or during the PR? |
Yes, exactly. We add them as part of the design doc itself, so no need to worry about it. |
All paid. Thanks everyone! |
This is part of the mentions project and is one of the features that we'd like to ship before EC3
ExpensiMark is the system that converts markdown in chat messages into html for storage (e.g.
*bold text*
to<strong>bold text</strong>
).We need to update ExpensiMark to:
@email@domain.com
in chat messages and replace it with<mention-user>@email@domain.com</mention-user>
@here
in chat messages and replace it with<mention-here>@here</mention-here>
We should only replace mentions if the
@
symbol comes at the beginning of a word.Examples that should match for user mentions:
Examples that should not match for user mentions:
Examples that should match for here mentions:
Examples that should not match for here mentions:
When something is treated as a mention it should not get treated as an email address (meaning we don't make it a mailto link).
Mentions should be allowed within:
Mentions should not be allowed within:
We will want to wait to merge this PR until the custom renderers for mention-user (#17885) and mention-here (#17886) have been added.
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: