Skip to content

Commit

Permalink
Merge pull request #26176 from koko57/fix/17365-nostrikethrough-solution
Browse files Browse the repository at this point in the history
Fix/17365 nostrikethrough solution
  • Loading branch information
AndrewGable committed Sep 19, 2023
2 parents 015cfb2 + 4cb0abc commit 22018f8
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 54 deletions.
21 changes: 11 additions & 10 deletions src/components/HTMLEngineProvider/HTMLRenderers/EditedRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,23 @@ function EditedRenderer(props) {
const defaultRendererProps = _.omit(props, ['TDefaultRenderer', 'style', 'tnode']);
const isPendingDelete = Boolean(props.tnode.attributes.deleted !== undefined);
return (
<Text
// eslint-disable-next-line react/jsx-props-no-spreading
{...defaultRendererProps}
fontSize={variables.fontSizeSmall}
color={themeColors.textSupporting}
style={[editedLabelStyles, isPendingDelete && styles.offlineFeedback.deleted]}
>
{/* Native devices do not support margin between nested text */}
<Text>
<Text
selectable={false}
style={[styles.w1, styles.userSelectNone]}
style={styles.userSelectNone}
dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
>
{' '}
</Text>
{props.translate('reportActionCompose.edited')}
<Text
// eslint-disable-next-line react/jsx-props-no-spreading
{...defaultRendererProps}
fontSize={variables.fontSizeSmall}
color={themeColors.textSupporting}
style={[editedLabelStyles, isPendingDelete && styles.offlineFeedback.deleted]}
>
{props.translate('reportActionCompose.edited')}
</Text>
</Text>
);
}
Expand Down
5 changes: 0 additions & 5 deletions src/components/HTMLEngineProvider/applyStrikethrough/index.js

This file was deleted.

This file was deleted.

32 changes: 18 additions & 14 deletions src/components/OfflineWithFeedback.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ import _ from 'underscore';
import React from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import compose from '../libs/compose';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import {withNetwork} from './OnyxProvider';
import CONST from '../CONST';
import networkPropTypes from './networkPropTypes';
import stylePropTypes from '../styles/stylePropTypes';
import styles from '../styles/styles';
import Tooltip from './Tooltip';
Expand All @@ -16,6 +12,8 @@ import * as StyleUtils from '../styles/StyleUtils';
import DotIndicatorMessage from './DotIndicatorMessage';
import shouldRenderOffscreen from '../libs/shouldRenderOffscreen';
import PressableWithoutFeedback from './Pressable/PressableWithoutFeedback';
import useLocalize from '../hooks/useLocalize';
import useNetwork from '../hooks/useNetwork';

/**
* This component should be used when we are using the offline pattern B (offline with feedback).
Expand Down Expand Up @@ -46,9 +44,6 @@ const propTypes = {
/** The content that needs offline feedback */
children: PropTypes.node.isRequired,

/** Information about the network */
network: networkPropTypes.isRequired,

/** Additional styles to add after local styles. Applied to the parent container */
style: stylePropTypes,

Expand All @@ -58,7 +53,11 @@ const propTypes = {
/** Additional style object for the error row */
errorRowStyles: stylePropTypes,

...withLocalizePropTypes,
/** Whether applying strikethrough to the children should be disabled */
shouldDisableStrikeThrough: PropTypes.bool,

/** Whether to apply needsOffscreenAlphaCompositing prop to the children */
needsOffscreenAlphaCompositing: PropTypes.bool,
};

const defaultProps = {
Expand All @@ -71,6 +70,8 @@ const defaultProps = {
style: [],
contentContainerStyle: [],
errorRowStyles: [],
shouldDisableStrikeThrough: false,
needsOffscreenAlphaCompositing: false,
};

/**
Expand All @@ -92,17 +93,20 @@ function applyStrikeThrough(children) {
}

function OfflineWithFeedback(props) {
const {translate} = useLocalize();
const {isOffline} = useNetwork();

const hasErrors = !_.isEmpty(props.errors);

// Some errors have a null message. This is used to apply opacity only and to avoid showing redundant messages.
const errorMessages = _.omit(props.errors, (e) => e === null);
const hasErrorMessages = !_.isEmpty(errorMessages);
const isOfflinePendingAction = props.network.isOffline && props.pendingAction;
const isOfflinePendingAction = isOffline && props.pendingAction;
const isUpdateOrDeleteError = hasErrors && (props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE);
const isAddError = hasErrors && props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD;
const needsOpacity = !props.shouldDisableOpacity && ((isOfflinePendingAction && !isUpdateOrDeleteError) || isAddError);
const needsStrikeThrough = props.network.isOffline && props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
const hideChildren = props.shouldHideOnDelete && !props.network.isOffline && props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && !hasErrors;
const needsStrikeThrough = !props.shouldDisableStrikeThrough && isOffline && props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
const hideChildren = props.shouldHideOnDelete && !isOffline && props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && !hasErrors;
let children = props.children;

// Apply strikethrough to children if needed, but skip it if we are not going to render them
Expand All @@ -126,12 +130,12 @@ function OfflineWithFeedback(props) {
messages={errorMessages}
type="error"
/>
<Tooltip text={props.translate('common.close')}>
<Tooltip text={translate('common.close')}>
<PressableWithoutFeedback
onPress={props.onClose}
style={[styles.touchableButtonImage]}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
accessibilityLabel={props.translate('common.close')}
accessibilityLabel={translate('common.close')}
>
<Icon src={Expensicons.Close} />
</PressableWithoutFeedback>
Expand All @@ -146,4 +150,4 @@ OfflineWithFeedback.propTypes = propTypes;
OfflineWithFeedback.defaultProps = defaultProps;
OfflineWithFeedback.displayName = 'OfflineWithFeedback';

export default compose(withLocalize, withNetwork())(OfflineWithFeedback);
export default OfflineWithFeedback;
1 change: 1 addition & 0 deletions src/pages/home/report/ReportActionItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ function ReportActionItem(props) {
errors={props.action.errors}
errorRowStyles={[styles.ml10, styles.mr2]}
needsOffscreenAlphaCompositing={ReportActionsUtils.isMoneyRequestAction(props.action)}
shouldDisableStrikeThrough
>
{isWhisper && (
<View style={[styles.flexRow, styles.pl5, styles.pt2, styles.pr3]}>
Expand Down
39 changes: 22 additions & 17 deletions src/pages/home/report/ReportActionItemFragment.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import compose from '../../../libs/compose';
import convertToLTR from '../../../libs/convertToLTR';
import {withNetwork} from '../../../components/OnyxProvider';
import CONST from '../../../CONST';
import applyStrikethrough from '../../../components/HTMLEngineProvider/applyStrikethrough';
import editedLabelStyles from '../../../styles/editedLabelStyles';
import UserDetailsTooltip from '../../../components/UserDetailsTooltip';
import avatarPropTypes from '../../../components/avatarPropTypes';
Expand Down Expand Up @@ -112,6 +111,7 @@ function ReportActionItemFragment(props) {
);
}
const {html, text} = props.fragment;
const isPendingDelete = props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && props.network.isOffline;

// Threaded messages display "[Deleted message]" instead of being hidden altogether.
// While offline we display the previous message with a strikethrough style. Once online we want to
Expand All @@ -128,35 +128,40 @@ function ReportActionItemFragment(props) {

// Only render HTML if we have html in the fragment
if (!differByLineBreaksOnly) {
const isPendingDelete = props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && props.network.isOffline;
const editedTag = props.fragment.isEdited ? `<edited ${isPendingDelete ? 'deleted' : ''}></edited>` : '';
const htmlContent = applyStrikethrough(html + editedTag, isPendingDelete);
const htmlContent = isPendingDelete ? `<del>${html}</del>` : html;

return <RenderHTML html={props.source === 'email' ? `<email-comment>${htmlContent}</email-comment>` : `<comment>${htmlContent}</comment>`} />;
const htmlWithTag = editedTag ? `${htmlContent}${editedTag}` : htmlContent;

return <RenderHTML html={props.source === 'email' ? `<email-comment>${htmlWithTag}</email-comment>` : `<comment>${htmlWithTag}</comment>`} />;
}
const containsOnlyEmojis = EmojiUtils.containsOnlyEmojis(text);

return (
<Text
selectable={!DeviceCapabilities.canUseTouchScreen() || !props.isSmallScreenWidth}
style={[containsOnlyEmojis ? styles.onlyEmojisText : undefined, styles.ltr, ...props.style]}
>
{convertToLTR(props.iouMessage || text)}
<Text style={[containsOnlyEmojis ? styles.onlyEmojisText : undefined, styles.ltr, ...props.style]}>
<Text
selectable={!DeviceCapabilities.canUseTouchScreen() || !props.isSmallScreenWidth}
style={[containsOnlyEmojis ? styles.onlyEmojisText : undefined, styles.ltr, ...props.style, isPendingDelete ? styles.offlineFeedback.deleted : undefined]}
>
{convertToLTR(props.iouMessage || text)}
</Text>
{Boolean(props.fragment.isEdited) && (
<Text
fontSize={variables.fontSizeSmall}
color={themeColors.textSupporting}
style={[editedLabelStyles, ...props.style]}
>
<>
<Text
selectable={false}
style={[containsOnlyEmojis ? styles.onlyEmojisTextLineHeight : undefined, styles.w1, styles.userSelectNone]}
style={[containsOnlyEmojis ? styles.onlyEmojisTextLineHeight : undefined, styles.userSelectNone]}
dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
>
{' '}
</Text>
{props.translate('reportActionCompose.edited')}
</Text>
<Text
fontSize={variables.fontSizeSmall}
color={themeColors.textSupporting}
style={[editedLabelStyles, isPendingDelete ? styles.offlineFeedback.deleted : undefined, ...props.style]}
>
{props.translate('reportActionCompose.edited')}
</Text>
</>
)}
</Text>
);
Expand Down
1 change: 1 addition & 0 deletions src/styles/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const webViewStyles = (theme) => ({
del: {
textDecorationLine: 'line-through',
textDecorationStyle: 'solid',
flex: 1,
},

strong: {
Expand Down
6 changes: 6 additions & 0 deletions tests/perf-test/SelectionList.perf-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ jest.mock('../../src/components/withLocalize', () => (Component) => (props) => (
/>
));

jest.mock('../../src/hooks/useNetwork', () =>
jest.fn(() => ({
isOffline: false,
})),
);

jest.mock('../../src/components/withKeyboardState', () => (Component) => (props) => (
<Component
// eslint-disable-next-line react/jsx-props-no-spreading
Expand Down

0 comments on commit 22018f8

Please sign in to comment.