-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix: ComposeBox - When you click on the mention again, extra characters appear. #61362
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
Changes from 2 commits
40d3c4b
4f63be9
2f8b903
0e6be1c
ef5c749
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -92,7 +92,7 @@ function SuggestionMention( | |||
// eslint-disable-next-line react-compiler/react-compiler | ||||
suggestionValuesRef.current = suggestionValues; | ||||
|
||||
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT); | ||||
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {canBeMissing: false}); | ||||
|
||||
const currentUserPersonalDetails = useCurrentUserPersonalDetails(); | ||||
const isMentionSuggestionsMenuVisible = !!suggestionValues.suggestedMentions.length && suggestionValues.shouldShowSuggestionMenu; | ||||
|
@@ -190,6 +190,12 @@ function SuggestionMention( | |||
[formatLoginPrivateDomain], | ||||
); | ||||
|
||||
function getOriginalMentionText(inputValue: string, atSignIndex: number) { | ||||
const rest = inputValue.slice(atSignIndex); | ||||
const mentionMatch = rest.match(/^[@\w.\-+]+/); // capture till space or special characters not in emails | ||||
return mentionMatch?.[0] ?? ''; | ||||
} | ||||
|
||||
/** | ||||
* Replace the code of mention and update selection | ||||
*/ | ||||
|
@@ -201,8 +207,8 @@ function SuggestionMention( | |||
return; | ||||
} | ||||
const mentionCode = getMentionCode(mentionObject, suggestionValues.prefixType); | ||||
const commentAfterMention = value.slice(suggestionValues.atSignIndex + suggestionValues.mentionPrefix.length + 1); | ||||
|
||||
const originalMention = getOriginalMentionText(value, suggestionValues.atSignIndex); | ||||
const commentAfterMention = value.slice(suggestionValues.atSignIndex + originalMention.length); | ||||
updateComment(`${commentBeforeAtSign}${mentionCode} ${trimLeadingSpace(commentAfterMention)}`, true); | ||||
const selectionPosition = suggestionValues.atSignIndex + mentionCode.length + CONST.SPACE_LENGTH; | ||||
setSelection({ | ||||
|
@@ -216,16 +222,7 @@ function SuggestionMention( | |||
shouldShowSuggestionMenu: false, | ||||
})); | ||||
}, | ||||
[ | ||||
value, | ||||
suggestionValues.atSignIndex, | ||||
suggestionValues.suggestedMentions, | ||||
suggestionValues.prefixType, | ||||
suggestionValues.mentionPrefix.length, | ||||
getMentionCode, | ||||
updateComment, | ||||
setSelection, | ||||
], | ||||
[value, suggestionValues.atSignIndex, suggestionValues.suggestedMentions, suggestionValues.prefixType, getMentionCode, updateComment, setSelection], | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it have a lint issue if we use previous style? I prefer spread in multiple line as previously, it makes code cleaner There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so, because Line 7 in 81b1b49
|
||||
); | ||||
|
||||
/** | ||||
|
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.
@Krishna2323 won't we use MENTION_BREAKER as you mentioned in your proposal? If not can you put this new regrex into a const?
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.
Sorry, my mistake — I've updated it to use MENTION_BREAKER.
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.
Thanks @Krishna2323. Can you elaborate on why we need to use both
.search
and.match
for MENTION_BREAKER? It look likeMENTION_BREAKER
is not used anywhereThere 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.
@hoangzinh, I got this solution from AI, and after prompting again, I received the following clarification and updated the code accordingly. I’ve tested the solution as well: