Skip to content

Commit

Permalink
Merge pull request #8691 from Expensify/cmartins-longComments
Browse files Browse the repository at this point in the history
Fix app hang when sending long comments
  • Loading branch information
luacmartins authored Apr 25, 2022
2 parents ad637d3 + 2517aca commit c308f6b
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 37 deletions.
3 changes: 3 additions & 0 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,9 @@ const CONST = {
this.EMAIL.ADMIN,
];
},

// There's a limit of 60k characters in Auth - https://github.com/Expensify/Auth/blob/198d59547f71fdee8121325e8bc9241fc9c3236a/auth/lib/Request.h#L28
MAX_COMMENT_LENGTH: 60_000,
};

export default CONST;
5 changes: 3 additions & 2 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -1081,9 +1081,10 @@ function fetchAllReports(
* @param {File} [file]
*/
function addAction(reportID, text, file) {
// Convert the comment from MD into HTML because that's how it is stored in the database
// For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
// For longer comments, skip parsing and display plaintext for performance reasons. It takes over 40s to parse a 100k long string!!
const parser = new ExpensiMark();
const commentText = parser.replace(text);
const commentText = text.length < 10000 ? parser.replace(text) : text;
const isAttachment = _.isEmpty(text) && file !== undefined;
const attachmentInfo = isAttachment ? file : {};

Expand Down
52 changes: 31 additions & 21 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,8 @@ class ReportActionCompose extends React.Component {

const trimmedComment = this.comment.trim();

// Don't submit empty comments
if (!trimmedComment) {
// Don't submit empty comments or comments that exceed the character limit
if (!trimmedComment || trimmedComment.length > CONST.MAX_COMMENT_LENGTH) {
return;
}

Expand Down Expand Up @@ -401,6 +401,8 @@ class ReportActionCompose extends React.Component {
const isComposeDisabled = this.props.isDrawerOpen && this.props.isSmallScreenWidth;
const isBlockedFromConcierge = ReportUtils.chatIncludesConcierge(this.props.report) && User.isBlockedFromConcierge(this.props.blockedFromConcierge);
const inputPlaceholder = this.getInputPlaceholder();
const hasExceededMaxCommentLength = this.comment.length > CONST.MAX_COMMENT_LENGTH;

return (
<View style={[shouldShowReportRecipientLocalTime && styles.chatItemComposeWithFirstRow]}>
{shouldShowReportRecipientLocalTime
Expand All @@ -411,6 +413,7 @@ class ReportActionCompose extends React.Component {
: styles.chatItemComposeBoxColor,
styles.chatItemComposeBox,
styles.flexRow,
hasExceededMaxCommentLength && styles.borderColorDanger,
]}
>
<AttachmentModal
Expand Down Expand Up @@ -565,15 +568,15 @@ class ReportActionCompose extends React.Component {
<TouchableOpacity
style={[
styles.chatItemSubmitButton,
this.state.isCommentEmpty ? styles.buttonDisable : styles.buttonSuccess,
(this.state.isCommentEmpty || hasExceededMaxCommentLength) ? styles.buttonDisable : styles.buttonSuccess,
]}
onPress={this.submitForm}
underlayColor={themeColors.componentBG}

// Keep focus on the composer when Send message is clicked.
// eslint-disable-next-line react/jsx-props-no-multi-spaces
onMouseDown={e => e.preventDefault()}
disabled={this.state.isCommentEmpty || isBlockedFromConcierge}
disabled={this.state.isCommentEmpty || isBlockedFromConcierge || hasExceededMaxCommentLength}
hitSlop={{
top: 3, right: 3, bottom: 3, left: 3,
}}
Expand All @@ -583,24 +586,31 @@ class ReportActionCompose extends React.Component {
</Tooltip>
</View>
</View>
{this.props.network.isOffline ? (
<View style={[styles.chatItemComposeSecondaryRow]}>
<View style={[
styles.chatItemComposeSecondaryRowOffset,
styles.flexRow,
styles.alignItemsCenter]}
>
<Icon
src={Expensicons.Offline}
width={variables.iconSizeExtraSmall}
height={variables.iconSizeExtraSmall}
/>
<Text style={[styles.ml2, styles.chatItemComposeSecondaryRowSubText]}>
{this.props.translate('reportActionCompose.youAppearToBeOffline')}
</Text>
</View>
<View style={[styles.chatItemComposeSecondaryRow, styles.flexRow, styles.justifyContentBetween]}>
<View>
{this.props.network.isOffline ? (
<View style={[
styles.chatItemComposeSecondaryRowOffset,
styles.flexRow,
styles.alignItemsCenter]}
>
<Icon
src={Expensicons.Offline}
width={variables.iconSizeExtraSmall}
height={variables.iconSizeExtraSmall}
/>
<Text style={[styles.ml2, styles.chatItemComposeSecondaryRowSubText]}>
{this.props.translate('reportActionCompose.youAppearToBeOffline')}
</Text>
</View>
) : <ReportTypingIndicator reportID={this.props.reportID} />}
</View>
) : <ReportTypingIndicator reportID={this.props.reportID} />}
{hasExceededMaxCommentLength && (
<Text style={[styles.textMicro, styles.textDanger, styles.chatItemComposeSecondaryRow]}>
{`${this.comment.length}/${CONST.MAX_COMMENT_LENGTH}`}
</Text>
)}
</View>
</View>
);
}
Expand Down
25 changes: 11 additions & 14 deletions src/pages/home/report/ReportTypingIndicator.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import _ from 'underscore';
import {withOnyx} from 'react-native-onyx';
Expand Down Expand Up @@ -50,7 +49,7 @@ class ReportTypingIndicator extends React.Component {
// Decide on the Text element that will hold the display based on the number of users that are typing.
switch (numUsersTyping) {
case 0:
return <View style={[styles.chatItemComposeSecondaryRow]} />;
return <></>;
case 1:
return (
<TextWithEllipsis
Expand All @@ -63,18 +62,16 @@ class ReportTypingIndicator extends React.Component {
);
default:
return (
<View style={[styles.chatItemComposeSecondaryRow]}>
<Text
style={[
styles.chatItemComposeSecondaryRowSubText,
styles.chatItemComposeSecondaryRowOffset,
]}
numberOfLines={1}
>
{this.props.translate('reportTypingIndicator.multipleUsers')}
{` ${this.props.translate('reportTypingIndicator.areTyping')}`}
</Text>
</View>
<Text
style={[
styles.chatItemComposeSecondaryRowSubText,
styles.chatItemComposeSecondaryRowOffset,
]}
numberOfLines={1}
>
{this.props.translate('reportTypingIndicator.multipleUsers')}
{` ${this.props.translate('reportTypingIndicator.areTyping')}`}
</Text>
);
}
}
Expand Down

0 comments on commit c308f6b

Please sign in to comment.