Skip to content

Add support for inline code with space only characters #49038

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

Merged
merged 6 commits into from
Sep 26, 2024
+28 −9
Merged

Add support for inline code with space only characters #49038

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3c7bfed
Add support for inline code with space only characters
tsa321 Sep 12, 2024
102455a
Modify comment
tsa321 Sep 12, 2024
3c12da6
Merge main and resolve conflict
tsa321 Sep 18, 2024
d05cfc7
fix reportactionsutils lint errors
tsa321 Sep 20, 2024
6885f11
Merge branch 'main' into supportEmptyInlineCode
tsa321 Sep 20, 2024
81c3fbc
fix lints
tsa321 Sep 20, 2024
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
8 changes: 4 additions & 4 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"date-fns-tz": "^2.0.0",
"dom-serializer": "^0.2.2",
"domhandler": "^4.3.0",
"expensify-common": "2.0.84",
"expensify-common": "2.0.88",
Copy link
Contributor

Choose a reason for hiding this comment

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

Just double-check, are you available to check if there are no breaking changes when we bump from 2.0.84 to 2.0.88?

Copy link
Contributor Author

@tsa321 tsa321 Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I believe there are no breaking changes. However, I have noticed another issue that also wants updating the expensify-common package (to version 2.0.87): #46117. Should we wait for a PR to be raised for that issue? or should we proceed with this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should. @deetergp what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've asked them in that PR if they want us to wait or not.

"expo": "51.0.17",
"expo-av": "14.0.6",
"expo-image": "1.12.12",
Expand Down
1 change: 1 addition & 0 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,7 @@ const CONST = {
MAX_PREVIEW_AVATARS: 4,
MAX_ROOM_NAME_LENGTH: 99,
LAST_MESSAGE_TEXT_MAX_LENGTH: 200,
MIN_LENGTH_LAST_MESSAGE_WITH_ELLIPSIS: 20,
OWNER_EMAIL_FAKE: '__FAKE__',
OWNER_ACCOUNT_ID_FAKE: 0,
DEFAULT_REPORT_NAME: 'Chat Report',
Expand Down
15 changes: 14 additions & 1 deletion src/libs/ReportActionsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,18 @@ function getLastVisibleAction(reportID: string, actionsToMerge: Record<string, N
return sortedReportActions[0];
}

function formatLastMessageText(lastMessageText: string) {
const trimmedMessage = String(lastMessageText).trim();

// Add support for inline code containing only space characters
// The message will appear as a blank space in the LHN
if ((trimmedMessage === '' && lastMessageText.length > 0) || (trimmedMessage === '?\u2026' && lastMessageText.length > CONST.REPORT.MIN_LENGTH_LAST_MESSAGE_WITH_ELLIPSIS)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you share what are contexts that the second condition would be used?

Copy link
Contributor Author

@tsa321 tsa321 Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

To test: try sending an inline code with many spaces inside it, for example:

The lastMessageText of the report from the back end is a lot of spaces, with an ellipsis at the end.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation.

return ' ';
}

return StringUtils.lineBreaksToSpaces(trimmedMessage).substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH).trim();
}

function getLastVisibleMessage(
reportID: string,
actionsToMerge: Record<string, NullishDeep<ReportAction> | null> = {},
Expand All @@ -763,7 +775,7 @@ function getLastVisibleMessage(

let messageText = getReportActionMessageText(lastVisibleAction) ?? '';
if (messageText) {
messageText = StringUtils.lineBreaksToSpaces(String(messageText)).substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH).trim();
messageText = formatLastMessageText(messageText);
}
return {
lastMessageText: messageText,
Expand Down Expand Up @@ -1667,6 +1679,7 @@ function getRemovedFromApprovalChainMessage(reportAction: OnyxEntry<ReportAction
export {
doesReportHaveVisibleActions,
extractLinksFromMessageHtml,
formatLastMessageText,
getActionableMentionWhisperMessage,
getAllReportActions,
getCombinedReportActions,
Expand Down
4 changes: 2 additions & 2 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ import * as PolicyUtils from './PolicyUtils';
import type {LastVisibleMessage} from './ReportActionsUtils';
import * as ReportActionsUtils from './ReportActionsUtils';
import * as ReportConnection from './ReportConnection';
import StringUtils from './StringUtils';
import * as TransactionUtils from './TransactionUtils';
import * as Url from './Url';
import type {AvatarSource} from './UserUtils';
Expand Down Expand Up @@ -1823,7 +1822,8 @@ function formatReportLastMessageText(lastMessageText: string, isModifiedExpenseM
if (isModifiedExpenseMessage) {
return String(lastMessageText).trim().replace(CONST.REGEX.LINE_BREAK, '').trim();
}
return StringUtils.lineBreaksToSpaces(String(lastMessageText).trim()).substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH).trim();

return ReportActionsUtils.formatLastMessageText(lastMessageText);
}

/**
Expand Down
Loading