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

fix(message-parser): mentions and emojis inside bold, italic and strikethrough #1035

Conversation

jayesh-jain252
Copy link
Contributor

@jayesh-jain252 jayesh-jain252 commented Apr 21, 2023

Proposed changes (including videos or screenshots)

Mentions and emojis are not parsed when they are inside bold, italic or strikethrough

After:

mentions.webm

Channel Mentions

Channel Mention

Emojis

Unicode.Emoji.webm

Unicode

Emoticons

emoticon_bold_italic_strike

Issue(s)

RocketChat/Rocket.Chat#27838
RocketChat/Rocket.Chat#27839
RocketChat/Rocket.Chat#28843

Further comments

Modifications:

  • Added tests for these cases and added Emoji | UserMention | ChannelMention to Bold, Italic, Strike definitions
  • Modified grammar.pegjs to support emojis and mentions. I added Whitespace, UserMention, ChannelMention, Emoji, Emoticon Rules.

For testing in Rocket.Chat application, modify gazzodown package (e.g. Rocket.Chat/packages/gazzodown/src/elements/BoldSpan.tsx)

// . . . 
// add imports for mentions and emojis
import EmojiElement from '../emoji/EmojiElement';
import ChannelMentionElement from '../mentions/ChannelMentionElement';
import UserMentionElement from '../mentions/UserMentionElement';

// Modify type to include emojis and mention
type BoldSpanProps = {
	children: ( MessageParser.Emoji | MessageParser.ChannelMention | MessageParser.UserMention | MessageParser.Link | MessageParser.MarkupExcluding<MessageParser.Bold>
        )[];
};

const BoldSpan = ({ children }: BoldSpanProps): ReactElement => (
	<strong>
		{children.map((block, index) => {
			switch (block.type) {
                                // . . .
				// add cases to support mentions and emojis
                                case 'MENTION_USER':
					return <UserMentionElement key={index} mention={block.value.value} />;

				case 'MENTION_CHANNEL':
					return <ChannelMentionElement key={index} mention={block.value.value} />;

				case 'EMOJI':
					return <EmojiElement key={index} {...block} />;

				default:
					return null;
			}
		})}
	</strong>
);

export default BoldSpan;

@CLAassistant
Copy link

CLAassistant commented Apr 21, 2023

CLA assistant check
All committers have signed the CLA.

@jayesh-jain252
Copy link
Contributor Author

jayesh-jain252 commented Apr 25, 2023

@hugocostadev, I'm a bit unsure about marking my pr as "Ready for review" because some of the tests are failing. Can you please advise me on what I should do in this situation?

@hugocostadev
Copy link
Contributor

Hi @jayesh-jain252 thanks a lot for your contribution 🚀

I'm taking a look here, and the tasks that are failing it's important to fix, we had to avoid generating Emphasis elements with only whitespace inside.

I'll try to figure out here, but just as a note, we need to fix those

@jayesh-jain252
Copy link
Contributor Author

jayesh-jain252 commented Apr 29, 2023

For Instant Testing:
Copy this grammar code from here: https://gist.github.com/jayesh-jain252/522356f9f375ccbd0ab60136080d5fbd
Paste this code at Peggyjs Online version - https://peggyjs.org/online.html

You can modify the grammar at Peggy online for testing

image

For this grammar I took the top part of the code from rocketchat-pegjs-benchmark by hugocostadev and rest from current message-parse grammar.

@jayesh-jain252
Copy link
Contributor Author

@hugocostadev now all the tests are passing. I've added an EmphasisWithWhitespace rule to the code. This rule checks for inputs that are of type * *, ** **, _ _.
Could you please review my changes and let me know if you have any feedback?

@jayesh-jain252 jayesh-jain252 marked this pull request as ready for review May 25, 2023 10:24
@hugocostadev
Copy link
Contributor

Awesome @jayesh-jain252 everything is working now, thanks a lot for your effort!

@hugocostadev
Copy link
Contributor

@jayesh-jain252 can you open a PR in Rocket.Chat making the changes in the Gazzodown ?

@jayesh-jain252
Copy link
Contributor Author

Thanks for the approval @hugocostadev 🚀. Opened the PR with changes in the gazzodown package RocketChat/Rocket.Chat#29391

@hugocostadev hugocostadev merged commit 036f70a into RocketChat:develop May 30, 2023
6 checks passed
@jayesh-jain252 jayesh-jain252 deleted the fix/mentions_and_emojis_inside_markup branch May 30, 2023 15:08
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.

None yet

3 participants