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.

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

Jump to bottom

Fix room mentions in editing comments #44160

Merged
merged 2 commits into from
Jun 21, 2024
Merged

Fix room mentions in editing comments #44160

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4479b4
add `isGroupPolicyReport` to `ReportActionItemMessageEdit`
war-in Jun 21, 2024
c3b0734
Merge branch 'refs/heads/main' into war-in/fix-room-mention-styling-i…
war-in Jun 21, 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
1 change: 1 addition & 0 deletions src/pages/home/report/ReportActionItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ function ReportActionItem({
shouldDisableEmojiPicker={
(ReportUtils.chatIncludesConcierge(report) && User.isBlockedFromConcierge(blockedFromConcierge)) || ReportUtils.isArchivedRoom(report)
}
isGroupPolicyReport={!!report?.policyID && report.policyID !== CONST.POLICY.ID_FAKE}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use isReportInGroupPolicy here? See also

App/src/components/Composer/types.ts

Lines 80 to 81 in 7512346

/** Indicates whether the composer is in a group policy report. Used for disabling report mentioning style in markdown input */
isGroupPolicyReport?: boolean;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what is the personal policy, but I can use it if we don't want to enable mentions there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the prop is named ...Group... I think you're right

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's pull @rlinoz here to bring clarity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But in ReportActionCompose we set isGroupPolicyReport like I did. Which one is correct? 🤔 I think we should have the same solution in both places

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx

Line 349 in 7512346

const isGroupPolicyReport = useMemo(() => !!report?.policyID && report.policyID !== CONST.POLICY.ID_FAKE, [report]);

cc @eh2077

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I also saw it. If we go with same logic with ReportActionCompose, then I think we can consider wrapping it into an utility method.

Let's wait a little while for @rlinoz 's input

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the same solution we did for the ReportActionCompose, the problem with using the isReportInGroupPolicy is that users who are not members of the policy but got invited to the room won't have the policy object in onyx.

/>
)}
</AttachmentContext.Provider>
Expand Down
6 changes: 5 additions & 1 deletion src/pages/home/report/ReportActionItemMessageEdit.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ type ReportActionItemMessageEditProps = {

/** Whether or not the emoji picker is disabled */
shouldDisableEmojiPicker?: boolean;

/** Whether report is from group policy */
isGroupPolicyReport?: boolean;
};

// native ids
Expand All @@ -70,7 +73,7 @@ const shouldUseForcedSelectionRange = shouldUseEmojiPickerSelection();
const draftMessageVideoAttributeCache = new Map<string, string>();

function ReportActionItemMessageEdit(
{action, draftMessage, reportID, index, shouldDisableEmojiPicker = false}: ReportActionItemMessageEditProps,
{action, draftMessage, reportID, index, isGroupPolicyReport, shouldDisableEmojiPicker = false}: ReportActionItemMessageEditProps,
forwardedRef: ForwardedRef<TextInput | HTMLTextAreaElement | undefined>,
) {
const [preferredSkinTone] = useOnyx(ONYXKEYS.PREFERRED_EMOJI_SKIN_TONE, {initialValue: CONST.EMOJI_DEFAULT_SKIN_TONE});
Expand Down Expand Up @@ -434,6 +437,7 @@ function ReportActionItemMessageEdit(
}}
selection={selection}
onSelectionChange={(e) => setSelection(e.nativeEvent.selection)}
isGroupPolicyReport={isGroupPolicyReport}
/>
</View>
<View style={styles.editChatItemEmojiWrapper}>
Expand Down
Loading