-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add support for mention feature #2982
Conversation
…zure/communication-ui-library into emlyn/mention-feature-branch-html-edit
Co-authored-by: Porter Nan <jiangnanhello@live.com> Signed-off-by: Emlyn Bolton <3941071+emlynmac@users.noreply.github.com>
Failed to pass the Static HTML UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot. |
Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot. |
@@ -53,8 +62,8 @@ type InputBoxComponentProps = { | |||
inlineChildren: boolean; | |||
'data-ui-id'?: string; | |||
id?: string; | |||
textValue: string; | |||
onChange: (event: FormEvent<HTMLInputElement | HTMLTextAreaElement>, newValue?: string | undefined) => void; | |||
textValue: string; // This could be plain text or HTML. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if someone intentionally type a text like this:
Hello, today I wanna read book of <Learning C++>
Does the logic strong enough to hold this case
@@ -53,8 +62,8 @@ type InputBoxComponentProps = { | |||
inlineChildren: boolean; | |||
'data-ui-id'?: string; | |||
id?: string; | |||
textValue: string; | |||
onChange: (event: FormEvent<HTMLInputElement | HTMLTextAreaElement>, newValue?: string | undefined) => void; | |||
textValue: string; // This could be plain text or HTML. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or.. Someone just paste a piece of html code (assuming they are developers chatting using our chat UI)
<Stack | ||
className={mergeStyles( | ||
sendBoxWrapperStyles, | ||
{ overflow: 'visible' } // This is needed for the mention popup to be visible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be happening if the content of senboxWrapper has real content overflow? Like too many line of text in the box?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a huge PR and a huge feature! Thanks for addressing comments and I am going to spend some time playing it in storybook ;D
(ev: React.KeyboardEvent<HTMLInputElement | HTMLTextAreaElement>) => { | ||
// Uses KeyCode 229 and which code 229 to determine if the press of the enter key is from a composition session or not (Safari only) | ||
if (ev.nativeEvent.isComposing || ev.nativeEvent.keyCode === 229 || ev.nativeEvent.which === 229) { | ||
return; | ||
} | ||
if (ev.key === 'ArrowUp') { | ||
ev.preventDefault(); | ||
/* @conditional-compile-remove(mention) */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the key.preventDefault for ArrowUp
and ArrowDown
inherit from?
What
Adds support for @mention tagging in the chat components
Why
Customer request to add this functionality.
How Tested
Storybook; ad-hoc
Process & policy checklist
Is this a breaking change?