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

Disable Copy link context menu for attachments and url. #9955

Merged
merged 6 commits into from
Aug 9, 2022
Merged

Disable Copy link context menu for attachments and url. #9955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9ec59c1
Update ContextMenuActions.js
Harshdeepjoshi Jul 18, 2022
dd51641
Disable Copy link context menu for attachment suggestion 1
Harshdeepjoshi Jul 25, 2022
93f10a2
Disable Copy link context menu for attachments and url suggestion 2
Harshdeepjoshi Jul 25, 2022
ebb51ff
disable copy link only on image target.
Harshdeepjoshi Aug 2, 2022
1a84291
Merge branch 'main' of github.com:Expensify/App
Harshdeepjoshi Aug 3, 2022
1b1879c
Coments added and remove extra code
Harshdeepjoshi Aug 3, 2022
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
6 changes: 5 additions & 1 deletion src/pages/home/report/ContextMenu/BaseReportActionContextMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@ import ONYXKEYS from '../../../../ONYXKEYS';
const propTypes = {
/** String representing the context menu type [LINK, REPORT_ACTION] which controls context menu choices */
type: PropTypes.string,

/** Target node which is the target of ContentMenu */
anchor: PropTypes.node,
...genericReportActionContextMenuPropTypes,
...withLocalizePropTypes,
...windowDimensionsPropTypes,
};

const defaultProps = {
type: CONTEXT_MENU_TYPES.REPORT_ACTION,
anchor: null,
...GenericReportActionContextMenuDefaultProps,
};
class BaseReportActionContextMenu extends React.Component {
Expand All @@ -34,7 +38,7 @@ class BaseReportActionContextMenu extends React.Component {
}

render() {
const shouldShowFilter = contextAction => contextAction.shouldShow(this.props.type, this.props.reportAction, this.props.betas);
const shouldShowFilter = contextAction => contextAction.shouldShow(this.props.type, this.props.reportAction, this.props.betas, this.props.anchor);

return this.props.isVisible && (
<View style={this.wrapperStyle}>
Expand Down
10 changes: 8 additions & 2 deletions src/pages/home/report/ContextMenu/ContextMenuActions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export default [
successIcon: Expensicons.Checkmark,
shouldShow: (type, reportAction) => (type === CONTEXT_MENU_TYPES.REPORT_ACTION
&& reportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.IOU
&& !ReportUtils.isReportMessageAttachment(lodashGet(reportAction, ['message', 0, 'text'], ''))),
&& !ReportUtils.isReportMessageAttachment(_.last(lodashGet(reportAction, ['message'], [{}])))),

// If return value is true, we switch the `text` and `icon` on
// `ContextMenuItem` with `successText` and `successIcon` which will fallback to
Expand Down Expand Up @@ -125,7 +125,13 @@ export default [
{
textTranslateKey: 'reportActionContextMenu.copyLink',
icon: Expensicons.LinkCopy,
shouldShow: (type, reportAction, betas) => Permissions.canUseCommentLinking(betas),
shouldShow: (type, reportAction, betas, menuTarget) => {
const isAttachment = ReportUtils.isReportMessageAttachment(_.last(lodashGet(reportAction, ['message'], [{}])));

// Only hide the copylink menu item when context menu is opened over img element.
const isAttachmentTarget = lodashGet(menuTarget, 'tagName') === 'IMG' && isAttachment;
Copy link
Contributor

Choose a reason for hiding this comment

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

@parasharrajat was there reason to disable copy link for only img tag? What issues you had if isAttachment but not img tag? i.e. doc file

Copy link
Contributor

Choose a reason for hiding this comment

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

@parasharrajat - Reminder for reply.

Copy link
Contributor

Choose a reason for hiding this comment

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

@parasharrajat - Any idea ?

Copy link
Member

@parasharrajat parasharrajat Aug 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

More details on #9660 (comment) and the next comments after it. Let me know if you need more clarification.

return Permissions.canUseCommentLinking(betas) && type === CONTEXT_MENU_TYPES.REPORT_ACTION && !isAttachmentTarget;
},
onPress: (closePopover, {reportAction, reportID}) => {
Environment.getEnvironmentURL()
.then((environmentURL) => {
Expand Down
3 changes: 3 additions & 0 deletions src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ class PopoverReportActionContextMenu extends React.Component {
) {
const nativeEvent = event.nativeEvent || {};
this.contextMenuAnchor = contextMenuAnchor;
this.contextMenuTargetNode = nativeEvent.target;

// Singleton behaviour of ContextMenu creates race conditions when user requests multiple contextMenus.
// But it is possible that every new request registers new callbacks thus instanceID is used to corelate those callbacks
Expand Down Expand Up @@ -222,6 +223,7 @@ class PopoverReportActionContextMenu extends React.Component {
selection={this.state.selection}
reportID={this.state.reportID}
reportAction={this.state.reportAction}
anchor={this.contextMenuTargetNode}
/>
);
}
Expand Down Expand Up @@ -293,6 +295,7 @@ class PopoverReportActionContextMenu extends React.Component {
reportID={this.state.reportID}
reportAction={this.state.reportAction}
draftMessage={this.state.reportActionDraftMessage}
anchor={this.contextMenuTargetNode}
/>
</PopoverWithMeasuredContent>
<ConfirmModal
Expand Down