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

feat: append whitespace after inserting emoji #30955

Merged
merged 18 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
29 changes: 28 additions & 1 deletion src/libs/ComposerUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ function insertText(text: string, selection: Selection, textToInsert: string): s
return text.slice(0, selection.start) + textToInsert + text.slice(selection.end, text.length);
}

/**
* Insert a white space at given index of text
* @param text - text that needs whitespace to be appended to
*/

aswin-s marked this conversation as resolved.
Show resolved Hide resolved
function insertWhiteSpaceAtIndex(text: string, index: number) {
return `${text.slice(0, index)} ${text.slice(index)}`;
}

/**
* Check whether we can skip trigger hotkeys on some specific devices.
*/
Expand All @@ -23,4 +32,22 @@ function canSkipTriggerHotkeys(isSmallScreenWidth: boolean, isKeyboardShown: boo
return (isSmallScreenWidth && DeviceCapabilities.canUseTouchScreen()) || isKeyboardShown;
}

export {getNumberOfLines, updateNumberOfLines, insertText, canSkipTriggerHotkeys};
/**
* Finds the length of common suffix between two texts
*/
function findCommonSuffixLength(str1: string, str2: string, cursorPosition: number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like it belongs in expensify/common str.js, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment it is used only with composer and requires cursor position to exactly predict which character got appended when duplicate characters are present.

let commonSuffixLength = 0;
const minLength = Math.min(str1.length - cursorPosition, str2.length);

for (let i = 1; i <= minLength; i++) {
if (str1.charAt(str1.length - i) === str2.charAt(str2.length - i)) {
commonSuffixLength++;
} else {
break;
}
}

return commonSuffixLength;
}

export {getNumberOfLines, updateNumberOfLines, insertText, canSkipTriggerHotkeys, insertWhiteSpaceAtIndex, findCommonSuffixLength};
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ function ComposerWithSuggestions({
return draft;
});
const commentRef = useRef(value);
const lastTextRef = useRef(value);

const {isSmallScreenWidth} = useWindowDimensions();
const maxComposerLines = isSmallScreenWidth ? CONST.COMPOSER.MAX_LINES_SMALL_SCREEN : CONST.COMPOSER.MAX_LINES;
Expand Down Expand Up @@ -189,6 +190,47 @@ function ComposerWithSuggestions({
[],
);

/**
* Find the newly added characters between the previous text and the new text based on the selection.
*
* @param {string} prevText - The previous text.
* @param {string} newText - The new text.
* @returns {object} An object containing information about the newly added characters.
* @property {number} startIndex - The start index of the newly added characters in the new text.
* @property {number} endIndex - The end index of the newly added characters in the new text.
* @property {string} diff - The newly added characters.
*/
const findNewlyAddedChars = useCallback(
(prevText, newText) => {
let startIndex = -1;
let endIndex = -1;
let currentIndex = 0;

// Find the first character mismatch with newText
while (currentIndex < newText.length && prevText.charAt(currentIndex) === newText.charAt(currentIndex) && selection.start > currentIndex) {
currentIndex++;
}

if (currentIndex < newText.length) {
startIndex = currentIndex;
const commonSuffixLength = ComposerUtils.findCommonSuffixLength(prevText, newText, selection.end);
// if text is getting pasted over find length of common suffix and subtract it from new text length
if (commonSuffixLength > 0 || selection.end - selection.start > 0) {
endIndex = newText.length - commonSuffixLength;
} else {
endIndex = currentIndex + newText.length;
}
}

return {
startIndex,
endIndex,
diff: newText.substring(startIndex, endIndex),
};
},
[selection.start, selection.end],
);

/**
* Update the value of the comment in Onyx
*
Expand All @@ -198,7 +240,13 @@ function ComposerWithSuggestions({
const updateComment = useCallback(
(commentValue, shouldDebounceSaveComment) => {
raiseIsScrollLikelyLayoutTriggered();
const {text: newComment, emojis, cursorPosition} = EmojiUtils.replaceAndExtractEmojis(commentValue, preferredSkinTone, preferredLocale);
const {startIndex, endIndex, diff} = findNewlyAddedChars(lastTextRef.current, commentValue);
const isEmojiInserted = diff.length && endIndex > startIndex && diff.trim() === diff && EmojiUtils.containsOnlyEmojis(diff);
const {
text: newComment,
emojis,
cursorPosition,
} = EmojiUtils.replaceAndExtractEmojis(isEmojiInserted ? ComposerUtils.insertWhiteSpaceAtIndex(commentValue, endIndex) : commentValue, preferredSkinTone, preferredLocale);
Copy link
Contributor

Choose a reason for hiding this comment

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

if (!_.isEmpty(emojis)) {
const newEmojis = EmojiUtils.getAddedEmojis(emojis, emojisPresentBefore.current);
if (!_.isEmpty(newEmojis)) {
Expand Down Expand Up @@ -255,6 +303,7 @@ function ComposerWithSuggestions({
},
[
debouncedUpdateFrequentlyUsedEmojis,
findNewlyAddedChars,
preferredLocale,
preferredSkinTone,
reportID,
Expand Down Expand Up @@ -312,14 +361,8 @@ function ComposerWithSuggestions({
* @param {Boolean} shouldAddTrailSpace
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we get rid of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flodnv Didn't quite get the question. Do you mean did we get rid of @param from jsdoc? I think it has become redundant with Typescript migration.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I am asking: it seems that we got rid of the shouldAddTrailSpace variable in code, and if I'm correct it seems like we should remove this @param line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the @param line

*/
const replaceSelectionWithText = useCallback(
(text, shouldAddTrailSpace = true) => {
const updatedText = shouldAddTrailSpace ? `${text} ` : text;
const selectionSpaceLength = shouldAddTrailSpace ? CONST.SPACE_LENGTH : 0;
updateComment(ComposerUtils.insertText(commentRef.current, selection, updatedText));
setSelection((prevSelection) => ({
start: prevSelection.start + text.length + selectionSpaceLength,
end: prevSelection.start + text.length + selectionSpaceLength,
}));
(text) => {
updateComment(ComposerUtils.insertText(commentRef.current, selection, text));
},
[selection, updateComment],
);
Expand Down Expand Up @@ -524,6 +567,10 @@ function ComposerWithSuggestions({
[blur, focus, prepareCommentAndResetComposer, replaceSelectionWithText],
);

useEffect(() => {
lastTextRef.current = value;
}, [value]);

useEffect(() => {
onValueChange(value);
}, [onValueChange, value]);
Expand Down
Loading