Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.03k stars 2.54k forks source link

[$250] mWeb -Room - Inconsistency while pasting highlighted mention in room and expense description #41919

Open lanitochka17 opened 3 weeks ago

lanitochka17 commented 3 weeks ago

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.72 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

1.Go to https://staging.new.expensify.com/

  1. Tap FAB > Start a Chat > create a new room
  2. Copy the following text in chat

@applausetester++0411km @jaihanumanblog@gmail.com (notice: first mention selected from list, second mention is highlighted)

  1. Tap header

  2. Paste text in room description

  3. Note second mention is not highlighted

  4. Navigate to LHN

  5. Tap fab -- track expense

  6. Enter amount and tap next

  7. Paste the same above text in track expense description

  8. Note now second mention is highlighted and displayed

Expected Result:

There should not be inconsistency while pasting highlighted mention in room and expense description

Actual Result:

Inconsistency while pasting highlighted mention in room and expense description

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/019b0b29-175f-4a31-bcb2-8f1a1fc95d48

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c4a08b3d6f351b65
  • Upwork Job ID: 1790309855227322368
  • Last Price Increase: 2024-05-14
  • Automatic offers:
    • shubham1206agra | Reviewer | 0
    • tienifr | Contributor | 0
    • bernhardoj | Contributor | 0
Issue OwnerCurrent Issue Owner: @shubham1206agra
melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

lanitochka17 commented 3 weeks ago

@Christinadobrzyn FYI 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

lanitochka17 commented 3 weeks ago

We think that this bug might be related to #vip-vsp

bernhardoj commented 3 weeks ago

Proposal

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

When pasting a mention to room description, it's not rendered as a mention but still as a plain text. But if we paste it to money request description, the mention is rendered as mention markdown.

What is the root cause of that problem?

In money request description page, we enable the live markdown preview https://github.com/Expensify/App/blob/807c945ef8edabe86cc62f16c866aa754db8750b/src/pages/iou/request/step/IOURequestStepDescription.tsx#L182

but we don't enable it in the room description. https://github.com/Expensify/App/blob/807c945ef8edabe86cc62f16c866aa754db8750b/src/pages/RoomDescriptionPage.tsx#L81-L100

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

Enable the live markdown in room description by passing isMarkdownEnabled to the input.

we can do the same for other input that support markdown too

Christinadobrzyn commented 3 weeks ago

I don't think this is a bug,

From what I can test, the only time the @jaihanumanblog@gmail.com isn't highlighted is after it's copied and pasted but before it's 'saved' in the room/expense/description. Once saved it turns into a mention and is highlighted.

@bernhardoj let me know if you feel otherwise but I'm going to close this as an edge case, small incident.

bernhardoj commented 3 weeks ago

From what I can test, the only time the @jaihanumanblog@gmail.com isn't highlighted is after it's copied and pasted but before it's 'saved' in the room/expense/description. Once saved it turns into a mention and is highlighted.

@Christinadobrzyn That's true. To simplify it, the room description page currently doesn't enable the live markdown preview, while the expense/task description enables it. So, if we want to enable the live markdown preview for room description too, then we can reopen this.

Btw, there is a previous discussion here to enable the live markdown preview on some inputs, but looks like the room description page is missing from the list.

bernhardoj commented 2 weeks ago

@Christinadobrzyn bump

Christinadobrzyn commented 2 weeks ago

Ah sorry @bernhardoj this GH was closed so I didn't see this. Ah yes, we should work on this based on this convo - https://expensify.slack.com/archives/C01GTK53T8Q/p1711641837108169

Although, I'm wondering if these markdowns are on hold based on this Slack convo - https://expensify.slack.com/archives/C01GTK53T8Q/p1711641837108169 I'll ask in that chat: Answer - no need to hold- let's work on these now.

melvin-bot[bot] commented 2 weeks ago

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

Christinadobrzyn commented 2 weeks ago

I assume this can be external?

melvin-bot[bot] commented 2 weeks ago

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

tienifr commented 2 weeks ago

Proposal

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

  1. Inconsistency while pasting highlighted mention in room and expense description
  2. Short mentions do not show with highlight

What is the root cause of that problem?

  1. We don't have isMarkdownEnabled in the room description input here and here
  2. Let's say my private email domain is expensify.com and there's a personal detail john@expensify.com, then @john is a valid mention (try sending that and it will be highlighted as a mention, will tooltips). This is a valid case of short mentions

But currently in expensify-common, we don't have logic to parse short mention, so in the live markdown input it was not recognized as a mention and does not have the highlight.

We can see this problem in the @ApplauseTester++0411km in the OP video, it's not highlighted although it's a valid mention.

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

  1. Pass isMarkdownEnabled to the room description input here and here
  2. We need to add logic to parse short mention to expensify-common.
    • Since short mentions are basically static mention (similar to @here), we know in advance the list of personal details short hand we'll be supporting, we can pass a list of those to replace of expensify-common
    • Then in here, we can add a rule for short mentions, this rule will be quite similar to the hereMention rule here, but instead of matching here, we should match any of the static mention in the list passed in earlier.
    • If we want to parse those short mentions into emails, we need to append the private email domain of the current user.
    • Then we'll pass those short mentions list into the markdown input, which will use it to pass to replace of expensify-common
    • The short mentions list can easily be retrieved by getting all personal details logins that share the same private domain, and get the before-@ part of it.

If any other input has the same problem, the fix will also be the same.

What alternative solutions did you explore? (Optional)

NA

Christinadobrzyn commented 2 weeks ago

Hi @shubham1206agra can you check out these proposals when you have a moment?

shubham1206agra commented 2 weeks ago

We can go with @tienifr's proposal. This will also fix problem with short mentions.

πŸŽ€ πŸ‘€ πŸŽ€ C+ Reviewed

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @francoisl, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot[bot] commented 2 weeks ago

πŸ“£ @shubham1206agra πŸŽ‰ An offer has been automatically sent to your Upwork account for the Reviewer role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 2 weeks ago

πŸ“£ @tienifr πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

bernhardoj commented 2 weeks ago

I think we are handling the short mention live markdown preview here.

shubham1206agra commented 2 weeks ago

I think we are handling the short mention live markdown preview here.

@bernhardoj I am really sorry this mixup happened from my side. Considering this, I am willing to change to your proposal if @francoisl agrees with me.

tienifr commented 2 weeks ago

@shubham1206agra The PR for the main change is mostly ready, after assignment I also spent a large amount of time trying to implement the short mention live markdown preview, but I guess we can let that be handled in https://github.com/Expensify/App/issues/38025.

I think it'd be fair to split the bounty between me and @bernhardoj, and we can move forward with the PR so there's no wasted effort.

cc @francoisl

shubham1206agra commented 1 week ago

@francoisl Can you finalise here please?

francoisl commented 1 week ago

Ok yeah, considering that @tienifr already put some time into this, splitting the bounty and using @bernhardoj's proposal sounds good to me.

francoisl commented 1 week ago

(Co-assigning @bernhardoj for the payment later.)

melvin-bot[bot] commented 1 week ago

πŸ“£ @bernhardoj πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

Christinadobrzyn commented 5 days ago

monitoring PR - https://github.com/Expensify/App/pull/42361