Skip to content

add mention user rule#600

Closed
tienifr wants to merge 3 commits into
Expensify:mainfrom
tienifr:add-mention-user-rule
Closed

add mention user rule#600
tienifr wants to merge 3 commits into
Expensify:mainfrom
tienifr:add-mention-user-rule

Conversation

@tienifr

@tienifr tienifr commented Nov 10, 2023

Copy link
Copy Markdown
Contributor

Fixed Issues

$ Expensify/App#29978

Tests

  1. Go to staging.new.expensify.com
  2. Create a room
  3. Click on the room header > Members > Invite > Invite anyone to the room
  4. Right click on the system invitation message > Copy to clipboard
  5. Paste the message in the composer
  6. Verify the system message along with the user mention will be copied

QA

  1. Go to staging.new.expensify.com
  2. Create a room
  3. Click on the room header > Members > Invite > Invite anyone to the room
  4. Right click on the system invitation message > Copy to clipboard
  5. Paste the message in the composer
  6. Verify the system message along with the user mention will be copied

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
Screen.Recording.2023-11-10.at.15.50.29.mov
iOS: mWeb Safari
Screen.Recording.2023-11-10.at.15.34.11.mov
MacOS: Chrome / Safari
Screen.Recording.2023-11-10.at.11.56.25.mov
MacOS: Desktop
Screen.Recording.2023-11-10.at.15.42.12.mov

@tienifr

tienifr commented Nov 10, 2023

Copy link
Copy Markdown
Contributor Author

@ArekChr The PR is ready for review. For htmlToMarkdownRules, I create 2 userMention rules because we handle copy html to clipboard for Safari and other platform are different

Comment thread lib/ExpensiMark.d.ts Outdated
* @param htmlString
*/
htmlToMarkdown(htmlString: string): string;
htmlToMarkdown(htmlString: string, personalDetails: PersonalDetails): string;

@ArekChr ArekChr Nov 13, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be treated as metadata, allowing us to pass various data through the second parameter.

E.g.

const metadata = { personalDetails }
htmlToMarkdown(html, metadata)
const metadata = { reportDetails }
htmlToMarkdown(html, metadata)

same in htmlToText.
WDYT?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the second parameter be optional?

Suggested change
htmlToMarkdown(htmlString: string, personalDetails: PersonalDetails): string;
htmlToMarkdown(htmlString: string, personalDetails?: PersonalDetails): string;

@tienifr

tienifr commented Nov 15, 2023

Copy link
Copy Markdown
Contributor Author

@ArekChr Thanks for your suggestion, I've updated

@roryabraham

Copy link
Copy Markdown
Contributor

Looks like we can close this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants