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.

Sign up for GitHub

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

Reset emoji suggestion pop-up automatically after an emoji has been inserted #27711

Merged
merged 5 commits into from
Sep 20, 2023
+6 −1
Merged

Reset emoji suggestion pop-up automatically after an emoji has been inserted #27711

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
baa788a
fix issue 27156
ahmedGaber93 Sep 18, 2023
124ec28
fix issue 27156
ahmedGaber93 Sep 18, 2023
a66f27c
lint and remove selection compare
ahmedGaber93 Sep 18, 2023
ff989cc
prettier
ahmedGaber93 Sep 18, 2023
aa5f44f
improve comments
ahmedGaber93 Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ function ComposerWithSuggestions({
setIsCommentEmpty(!!newComment.match(/^(\s)*$/));
setValue(newComment);
if (commentValue !== newComment) {
// Ensure emoji suggestions are hidden even when the selection is not changed (so calculateEmojiSuggestion would not be called).
if (suggestionsRef.current) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should also have !_.isEmpty(emojis) as a condition here? I guess it's possible that in future newComment might not only be changed by emoji replacements, so might be worth future-proofing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it's possible that in future newComment might not only be changed by emoji replacements

@jjcoffee
I agree about that, but I think !_.isEmpty(emojis) will not be useful in this case because emojis it returns all emoji in the text not only the current replaced emojis.
For example: text with emoji 😄👁️ then many lines of text then cursor at the end writing normal letters not emoji will return emojis = [😄, 👁️]

newComment now in the code refer to comment after replace emoji.
So I think If new replacement applied in the future, in this time the fix should be if (commentValueWithEmoji !== commentValueWithoutEmoji)

Copy link
Contributor

Choose a reason for hiding this comment

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

@ahmedGaber93 Ah yes I see what you mean, that makes sense. Let's leave it for now then!

suggestionsRef.current.resetSuggestions();
}

const remainder = ComposerUtils.getCommonSuffixLength(commentRef.current, newComment);
setSelection({
start: newComment.length - remainder,
Expand Down Expand Up @@ -212,7 +217,7 @@ function ComposerWithSuggestions({
debouncedBroadcastUserIsTyping(reportID);
}
},
[debouncedUpdateFrequentlyUsedEmojis, preferredLocale, preferredSkinTone, reportID, setIsCommentEmpty],
[debouncedUpdateFrequentlyUsedEmojis, preferredLocale, preferredSkinTone, reportID, setIsCommentEmpty, suggestionsRef],
);

/**
Expand Down