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

#2 - Don't render report mention markdown if it's not a policy report #48140

Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import {createContext} from 'react';

type MentionReportContextProps = {
currentReportID: string;
};

const MentionReportContext = createContext<MentionReportContextProps>({
currentReportID: '',
});

export default MentionReportContext;
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import isEmpty from 'lodash/isEmpty';
import React, {useMemo} from 'react';
import React, {useContext, useMemo} from 'react';
import type {TextStyle} from 'react-native';
import {StyleSheet} from 'react-native';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
Expand All @@ -16,6 +16,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Report} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import MentionReportContext from './MentionReportContext';

type MentionReportOnyxProps = {
/** All reports shared with the user */
Expand Down Expand Up @@ -56,10 +57,12 @@ function MentionReportRenderer({style, tnode, TDefaultRenderer, reports, ...defa
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const htmlAttributeReportID = tnode.attributes.reportid;
const {currentReportID: currentReportIDContext} = useContext(MentionReportContext);

const currentReportID = useCurrentReportID();
const currentReportIDValue = currentReportIDContext || currentReportID?.currentReportID;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous PR is reverted because of #48058. The issue is that currentReportIDContext is an empty string and previously we used ??, so currentReportID?.currentReportID is never used.

The video showing #48058 is fixed

web.mp4

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const [currentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${currentReportID?.currentReportID || -1}`);
const [currentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${currentReportIDValue || -1}`);

// When we invite someone to a room they don't have the policy object, but we still want them to be able to see and click on report mentions, so we only check if the policyID in the report is from a workspace
const isGroupPolicyReport = useMemo(() => currentReport && !isEmptyObject(currentReport) && !!currentReport.policyID && currentReport.policyID !== CONST.POLICY.ID_FAKE, [currentReport]);
Expand All @@ -71,7 +74,7 @@ function MentionReportRenderer({style, tnode, TDefaultRenderer, reports, ...defa
const {reportID, mentionDisplayText} = mentionDetails;

const navigationRoute = reportID ? ROUTES.REPORT_WITH_ID.getRoute(reportID) : undefined;
const isCurrentRoomMention = reportID === currentReportID?.currentReportID;
const isCurrentRoomMention = reportID === currentReportIDValue;

const flattenStyle = StyleSheet.flatten(style as TextStyle);
const {color, ...styleWithoutColor} = flattenStyle;
Expand Down
8 changes: 6 additions & 2 deletions src/components/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ type MenuItemBaseProps = {
/** Whether should render error text as HTML or as Text */
shouldRenderErrorAsHTML?: boolean;

/** List of markdown rules that will be ignored */
excludedMarkdownRules?: string[];

/** Should check anonymous user in onPress function */
shouldCheckActionAllowedOnPress?: boolean;

Expand Down Expand Up @@ -409,6 +412,7 @@ function MenuItem(
shouldParseHelperText = false,
shouldRenderHintAsHTML = false,
shouldRenderErrorAsHTML = false,
excludedMarkdownRules = [],
shouldCheckActionAllowedOnPress = true,
onSecondaryInteraction,
titleWithTooltips,
Expand Down Expand Up @@ -464,8 +468,8 @@ function MenuItem(
if (!title || !shouldParseTitle) {
return '';
}
return Parser.replace(title, {shouldEscapeText});
}, [title, shouldParseTitle, shouldEscapeText]);
return Parser.replace(title, {shouldEscapeText, disabledRules: excludedMarkdownRules});
}, [title, shouldParseTitle, shouldEscapeText, excludedMarkdownRules]);

const helperHtml = useMemo(() => {
if (!helperText || !shouldParseHelperText) {
Expand Down
40 changes: 23 additions & 17 deletions src/components/MoneyRequestConfirmationListFooter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import type * as OnyxTypes from '@src/types/onyx';
import type {Participant} from '@src/types/onyx/IOU';
import type {Unit} from '@src/types/onyx/Policy';
import ConfirmedRoute from './ConfirmedRoute';
import MentionReportContext from './HTMLEngineProvider/HTMLRenderers/MentionReportRenderer/MentionReportContext';
import MenuItem from './MenuItem';
import MenuItemWithTopDescription from './MenuItemWithTopDescription';
import PDFThumbnail from './PDFThumbnail';
Expand Down Expand Up @@ -266,6 +267,8 @@ function MoneyRequestConfirmationListFooter({
const resolvedThumbnail = isLocalFile ? receiptThumbnail : tryResolveUrlFromApiRoot(receiptThumbnail ?? '');
const resolvedReceiptImage = isLocalFile ? receiptImage : tryResolveUrlFromApiRoot(receiptImage ?? '');

const mentionReportContextValue = useMemo(() => ({currentReportID: reportID}), [reportID]);

// An intermediate structure that helps us classify the fields as "primary" and "supplementary".
// The primary fields are always shown to the user, while an extra action is needed to reveal the supplementary ones.
const classifiedFields = [
Expand Down Expand Up @@ -296,23 +299,26 @@ function MoneyRequestConfirmationListFooter({
},
{
item: (
<MenuItemWithTopDescription
key={translate('common.description')}
shouldShowRightIcon={!isReadOnly}
shouldParseTitle
title={iouComment}
description={translate('common.description')}
onPress={() => {
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(action, iouType, transactionID, reportID, Navigation.getActiveRouteWithoutParams(), reportActionID),
);
}}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
disabled={didConfirm}
interactive={!isReadOnly}
numberOfLinesTitle={2}
/>
<MentionReportContext.Provider value={mentionReportContextValue}>
Copy link
Member

Choose a reason for hiding this comment

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

This change caused a duplicate key warning. #49063

<MenuItemWithTopDescription
key={translate('common.description')}
shouldShowRightIcon={!isReadOnly}
shouldParseTitle
excludedMarkdownRules={!policy ? ['reportMentions'] : []}
title={iouComment}
description={translate('common.description')}
onPress={() => {
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(action, iouType, transactionID, reportID, Navigation.getActiveRouteWithoutParams(), reportActionID),
);
}}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
disabled={didConfirm}
interactive={!isReadOnly}
numberOfLinesTitle={2}
/>
</MentionReportContext.Provider>
),
shouldShow: true,
isSupplementary: false,
Expand Down
3 changes: 2 additions & 1 deletion src/components/TextInput/BaseTextInput/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ function BaseTextInput(
prefixCharacter = '',
inputID,
isMarkdownEnabled = false,
excludedMarkdownStyles = [],
shouldShowClearButton = false,
prefixContainerStyle = [],
prefixStyle = [],
Expand All @@ -74,7 +75,7 @@ function BaseTextInput(
const inputProps = {shouldSaveDraft: false, shouldUseDefaultValue: false, ...props};
const theme = useTheme();
const styles = useThemeStyles();
const markdownStyle = useMarkdownStyle();
const markdownStyle = useMarkdownStyle(undefined, excludedMarkdownStyles);
const StyleUtils = useStyleUtils();
const {translate} = useLocalize();

Expand Down
3 changes: 2 additions & 1 deletion src/components/TextInput/BaseTextInput/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function BaseTextInput(
suffixCharacter = '',
inputID,
isMarkdownEnabled = false,
excludedMarkdownStyles = [],
shouldShowClearButton = false,
prefixContainerStyle = [],
prefixStyle = [],
Expand All @@ -78,7 +79,7 @@ function BaseTextInput(

const theme = useTheme();
const styles = useThemeStyles();
const markdownStyle = useMarkdownStyle();
const markdownStyle = useMarkdownStyle(undefined, excludedMarkdownStyles);
const {hasError = false} = inputProps;
const StyleUtils = useStyleUtils();
const {translate} = useLocalize();
Expand Down
4 changes: 4 additions & 0 deletions src/components/TextInput/BaseTextInput/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type {MarkdownStyle} from '@expensify/react-native-live-markdown';
import type {GestureResponderEvent, StyleProp, TextInputProps, TextStyle, ViewStyle} from 'react-native';
import type {AnimatedTextInputRef} from '@components/RNTextInput';
import type IconAsset from '@src/types/utils/IconAsset';
Expand Down Expand Up @@ -115,6 +116,9 @@ type CustomBaseTextInputProps = {
/** Should live markdown be enabled. Changes RNTextInput component to RNMarkdownTextInput */
isMarkdownEnabled?: boolean;

/** List of markdowns that won't be styled as a markdown */
excludedMarkdownStyles?: Array<keyof MarkdownStyle>;

/** Whether the clear button should be displayed */
shouldShowClearButton?: boolean;

Expand Down
3 changes: 3 additions & 0 deletions src/pages/iou/request/step/IOURequestStepDescription.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ function IOURequestStepDescription({
const canEditSplitBill = isSplitBill && reportAction && session?.accountID === reportAction.actorAccountID && TransactionUtils.areRequiredFieldsEmpty(transaction);
// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = isEditing && (isSplitBill ? !canEditSplitBill : !ReportActionsUtils.isMoneyRequestAction(reportAction) || !ReportUtils.canEditMoneyRequest(reportAction));
const isReportInGroupPolicy = !!report?.policyID && report.policyID !== CONST.POLICY.ID_FAKE;

return (
<StepScreenWrapper
headerTitle={translate('common.description')}
Expand Down Expand Up @@ -187,6 +189,7 @@ function IOURequestStepDescription({
maxAutoGrowHeight={variables.textInputAutoGrowMaxHeight}
shouldSubmitForm
isMarkdownEnabled
excludedMarkdownStyles={!isReportInGroupPolicy ? ['mentionReport'] : []}
/>
</View>
</FormProvider>
Expand Down
Loading