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

Fixed Android - Chat - Message gets displayed from right to left #32297

Merged
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
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
9 changes: 5 additions & 4 deletions src/libs/convertToLTR/index.android.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import CONST from '@src/CONST';
import ConvertToLTR from './types';

/**
* Android only - convert RTL text to a LTR text using Unicode controls.
* https://www.w3.org/International/questions/qa-bidi-unicode-controls
*
* In React Native, when working with bidirectional text (RTL - Right-to-Left or LTR - Left-to-Right), you may encounter issues related to text rendering, especially on Android devices. These issues arise because Android's default behavior for text direction might not always align with the desired directionality of your app.
*/
import CONST from '@src/CONST';
import ConvertToLTR from './types';

const convertToLTR: ConvertToLTR = (text) => `${CONST.UNICODE.LTR}${text}`;

export default convertToLTR;
1 change: 1 addition & 0 deletions src/libs/convertToLTR/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// The Android platform has to handle switching between LTR and RTL languages a bit differently (https://developer.android.com/training/basics/supporting-devices/languages). For all other platforms, these can simply be no-op functions.
import ConvertToLTR from './types';

const convertToLTR: ConvertToLTR = (text) => text;
Expand Down
56 changes: 55 additions & 1 deletion src/libs/convertToLTRForComposer/index.android.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,62 @@
import CONST from '@src/CONST';
import ConvertToLTRForComposer from './types';

/**
* Android only - The composer can be converted to LTR if its content is the LTR character followed by an @ or space
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment should also explain why it's OK to convert it in these cases and why it shouldn't be converted. Consider adding "because..." and then fill out the rest.

*/
function canComposerBeConvertedToLTR(text: string): boolean {
// this handle cases when user type only spaces
Copy link
Contributor

Choose a reason for hiding this comment

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

Please start comments with a capital letter

Copy link
Contributor

Choose a reason for hiding this comment

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

"this handle cases" is not good grammar and should be more like "This regex handles the case when a user only types spaces into the composer.

const containOnlySpaces = /^\s*$/;
// this handle the case where someone has RTL enabled and they began typing an @mention for someone.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do comments end with a period or not? Let's be consistent

const startsWithLTRAndAt = new RegExp(`^${CONST.UNICODE.LTR}@$`);
// this handle cases could send empty messages when composer is multiline
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not quite sure what this means and the grammar is a bit confusing. Are you trying to say this?

This regex handles the case where the composer can contain multiple lines of whitespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, sorry 😣!

const startsWithLTRAndSpace = new RegExp(`${CONST.UNICODE.LTR}\\s*$`);
const emptyExpressions = [containOnlySpaces, startsWithLTRAndAt, startsWithLTRAndSpace];
return emptyExpressions.some((exp) => exp.test(text));
}

/**
* Android only - We should remove the LTR unicode when the input is empty to prevent:
* Sending an empty message;
* Mention suggestions not works if @ or \s (at or space) is the first character;
* Placeholder is not displayed if the unicode character is the only character remaining;
* force: always remove the LTR unicode, going to be used when composer is consider as empty */
Copy link
Contributor

Choose a reason for hiding this comment

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

The */ needs to go on the next line by itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the force: comment is for the force param, please format it as a legit @param doc.

const resetLTRWhenEmpty = (newComment: string, force?: boolean) => {
const result = newComment.length <= 1 || force ? newComment.replaceAll(CONST.UNICODE.LTR, '') : newComment;
return result;
};

/**
* Android only - Do not convert RTL text to a LTR text for input box using Unicode controls.
* Android does not properly support bidirectional text for mixed content for input box
*/
const convertToLTRForComposer: ConvertToLTRForComposer = (text) => text;
const convertToLTRForComposer: ConvertToLTRForComposer = (text, isComposerEmpty) => {
const shouldComposerMaintainAsLTR = canComposerBeConvertedToLTR(text);
const newText = resetLTRWhenEmpty(text, shouldComposerMaintainAsLTR);
if (shouldComposerMaintainAsLTR) {
return newText;
}
return isComposerEmpty ? `${CONST.UNICODE.LTR}${newText}` : newText;
};

/**
* This is necessary to convert the input to LTR, there is a delay that causes the cursor not to go to the end of the input line when pasting text or typing fast. The delay is caused for the time that takes the input to convert from RTL to LTR and viceversa.
*/
const moveCursorToEndOfLine = (
commentLength: number,
setSelection: (
value: React.SetStateAction<{
start: number;
end: number;
}>,
) => void,
) => {
setSelection({
start: commentLength + 1,
end: commentLength + 1,
});
};

export {moveCursorToEndOfLine};

export default convertToLTRForComposer;
5 changes: 5 additions & 0 deletions src/libs/convertToLTRForComposer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,9 @@ const convertToLTRForComposer: ConvertToLTRForComposer = (text) => {
// Add the LTR marker to the beginning of the text.
return `${CONST.UNICODE.LTR}${text}`;
};

const moveCursorToEndOfLine = (commentLength: number) => commentLength;

export {moveCursorToEndOfLine};

export default convertToLTRForComposer;
2 changes: 1 addition & 1 deletion src/libs/convertToLTRForComposer/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
type ConvertToLTRForComposer = (text: string) => string;
type ConvertToLTRForComposer = (text: string, isComposerEmpty?: boolean) => string;

export default ConvertToLTRForComposer;
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import canFocusInputOnScreenFocus from '@libs/canFocusInputOnScreenFocus';
import compose from '@libs/compose';
import * as ComposerUtils from '@libs/ComposerUtils';
import getDraftComment from '@libs/ComposerUtils/getDraftComment';
import convertToLTRForComposer from '@libs/convertToLTRForComposer';
import convertToLTRForComposer, {moveCursorToEndOfLine} from '@libs/convertToLTRForComposer';
import * as EmojiUtils from '@libs/EmojiUtils';
import focusComposerWithDelay from '@libs/focusComposerWithDelay';
import * as KeyDownListener from '@libs/KeyboardShortcut/KeyDownPressListener';
Expand Down Expand Up @@ -105,6 +105,7 @@ function ComposerWithSuggestions({
const styles = useThemeStyles();
const {preferredLocale} = useLocalize();
const isFocused = useIsFocused();
const composerIsEmpty = useRef(true);
const navigation = useNavigation();
const emojisPresentBefore = useRef([]);
const [value, setValue] = useState(() => {
Expand Down Expand Up @@ -220,41 +221,56 @@ function ComposerWithSuggestions({
debouncedUpdateFrequentlyUsedEmojis();
}
}
const newCommentConverted = convertToLTRForComposer(newComment);
const isNewCommentEmpty = !!newCommentConverted.match(/^(\s)*$/);
const isPrevCommentEmpty = !!commentRef.current.match(/^(\s)*$/);

let newCommentConvertedToLTR = newComment;
const prevComment = commentRef.current;
samilabud marked this conversation as resolved.
Show resolved Hide resolved

// This prevent the double execution of setting input value that could affect the place holder and could send an empty message or draft messages in android
if (prevComment !== newComment) {
newCommentConvertedToLTR = convertToLTRForComposer(newCommentConvertedToLTR, composerIsEmpty.current);
setValue(newCommentConvertedToLTR);
moveCursorToEndOfLine(newComment.length, setSelection);
composerIsEmpty.current = false;
}

const isNewCommentEmpty = !!newCommentConvertedToLTR.match(/^(\s)*$/);
const isPrevCommentEmpty = !!prevComment.match(/^(\s)*$/);

/** Only update isCommentEmpty state if it's different from previous one */
if (isNewCommentEmpty !== isPrevCommentEmpty) {
setIsCommentEmpty(isNewCommentEmpty);
if (isNewCommentEmpty) {
composerIsEmpty.current = true;
}
}

emojisPresentBefore.current = emojis;
setValue(newCommentConverted);

if (commentValue !== newComment) {
const position = Math.max(selection.end + (newComment.length - commentRef.current.length), cursorPosition || 0);
const position = Math.max(selection.end + (newComment.length - prevComment.length), cursorPosition || 0);
setSelection({
start: position,
end: position,
});
}

// Indicate that draft has been created.
if (commentRef.current.length === 0 && newCommentConverted.length !== 0) {
if (prevComment.length === 0 && newCommentConvertedToLTR.length !== 0) {
Report.setReportWithDraft(reportID, true);
}

// The draft has been deleted.
if (newCommentConverted.length === 0) {
if (newCommentConvertedToLTR.length === 0) {
Report.setReportWithDraft(reportID, false);
}

commentRef.current = newCommentConverted;
commentRef.current = newCommentConvertedToLTR;
if (shouldDebounceSaveComment) {
debouncedSaveReportComment(reportID, newCommentConverted);
debouncedSaveReportComment(reportID, newCommentConvertedToLTR);
} else {
Report.saveReportComment(reportID, newCommentConverted || '');
Report.saveReportComment(reportID, newCommentConvertedToLTR || '');
}
if (newCommentConverted) {
if (newCommentConvertedToLTR) {
debouncedBroadcastUserIsTyping(reportID);
}
},
Expand All @@ -268,6 +284,7 @@ function ComposerWithSuggestions({
raiseIsScrollLikelyLayoutTriggered,
debouncedSaveReportComment,
selection.end,
composerIsEmpty,
],
);

Expand Down
Loading