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

Context-menu repositioned #4194

Merged
merged 20 commits into from
Jul 28, 2021
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
21 changes: 21 additions & 0 deletions src/components/PopoverWithMeasuredContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class PopoverWithMeasuredContent extends Component {

this.state = {
isContentMeasured: false,
isVisible: false,
};

this.popoverWidth = 0;
Expand All @@ -63,6 +64,26 @@ class PopoverWithMeasuredContent extends Component {
this.measurePopover = this.measurePopover.bind(this);
}

/**
* When Popover becomes visible, we need to recalculate the Dimensions.
* Skip render on Popover until recalculations have done by setting isContentMeasured false as early as possible.
*
* @static
* @param {Object} props
* @param {Object} state
* @return {Object|null}
*/
static getDerivedStateFromProps(props, state) {
// When Popover is shown recalculate
if (!state.isVisible && props.isVisible) {
return {isContentMeasured: false, isVisible: true};
}
if (!props.isVisible) {
return {isVisible: false};
}
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

derived state is sort of confusing and usually anti-pattern since we don't have a single source of truth for isVisible anymore. Maybe just put this back to what it was..

Copy link
Member Author

@parasharrajat parasharrajat Jul 28, 2021

Choose a reason for hiding this comment

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

I had to use it to prevent unnecessary render & unmount after isVisible is set on the Popover.
we have a callback onModalHide which fires on the following conditions.

  1. if Modal is visible and component unmounts.
  2. Modal is set to not visible.(isVisible=false).

Previously, we are triggering the rerendering of the Modal when isvisible is set to true which first unmounts the old Popover and then remounts it with calculated dimensions.

we are triggering the rerendering of the Modal when isvisible is set to true which first unmounts the old Popover

Here onModalHide is called due to the reason that isVisible is true and unmounting is happening.

This code does not render the popover until it is measured. we use onModalHide call to reset the reportAction and reportId back to null.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, we can maybe try to improve this in a follow up I don't have a better suggestion at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to leave a code comment explaining this.

}

shouldComponentUpdate(nextProps, nextState) {
if (this.props.isVisible
&& (nextProps.windowWidth !== this.props.windowWidth
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import React from 'react';
import {View} from 'react-native';
import _ from 'underscore';
import getReportActionContextMenuStyles from '../../../../styles/getReportActionContextMenuStyles';
import ContextMenuItem from '../../../../components/ContextMenuItem';
import {
propTypes as GenericReportActionContextMenuPropTypes,
defaultProps,
} from './GenericReportActionContextMenuPropTypes';
import withLocalize, {withLocalizePropTypes} from '../../../../components/withLocalize';
import ContextMenuActions from './ContextMenuActions';

const propTypes = {
...GenericReportActionContextMenuPropTypes,
...withLocalizePropTypes,
};

class BaseReportActionContextMenu extends React.Component {
constructor(props) {
super(props);

this.wrapperStyle = getReportActionContextMenuStyles(this.props.isMini);
}

render() {
return this.props.isVisible && (
<View style={this.wrapperStyle}>
{_.map(ContextMenuActions, contextAction => contextAction.shouldShow(this.props.reportAction) && (
<ContextMenuItem
icon={contextAction.icon}
text={this.props.translate(contextAction.textTranslateKey)}
successIcon={contextAction.successIcon}
successText={contextAction.successTextTranslateKey
? this.props.translate(contextAction.successTextTranslateKey)
: undefined}
isMini={this.props.isMini}
key={contextAction.textTranslateKey}
onPress={() => contextAction.onPress(!this.props.isMini, {
reportAction: this.props.reportAction,
reportID: this.props.reportID,
draftMessage: this.props.draftMessage,
selection: this.props.selection,
})}
/>
))}
</View>
);
}
}

BaseReportActionContextMenu.propTypes = propTypes;
BaseReportActionContextMenu.defaultProps = defaultProps;

export default withLocalize(BaseReportActionContextMenu);
116 changes: 116 additions & 0 deletions src/pages/home/report/ContextMenu/ContextMenuActions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import _ from 'underscore';
import lodashGet from 'lodash/get';
import Str from 'expensify-common/lib/str';
import {
Clipboard as ClipboardIcon, LinkCopy, Mail, Pencil, Trashcan, Checkmark,
} from '../../../../components/Icon/Expensicons';
import {
setNewMarkerPosition, updateLastReadActionID, saveReportActionDraft,
} from '../../../../libs/actions/Report';
import Clipboard from '../../../../libs/Clipboard';
import {isReportMessageAttachment, canEditReportAction, canDeleteReportAction} from '../../../../libs/reportUtils';
import ReportActionComposeFocusManager from '../../../../libs/ReportActionComposeFocusManager';
import {hideContextMenu, showDeleteModal} from './ReportActionContextMenu';

/**
* Gets the HTML version of the message in an action.
* @param {Object} reportAction
* @return {String}
*/
function getActionText(reportAction) {
const message = _.last(lodashGet(reportAction, 'message', null));
return lodashGet(message, 'html', '');
}

// A list of all the context actions in this menu.
export default [
// Copy to clipboard
{
textTranslateKey: 'contextMenuItem.copyToClipboard',
icon: ClipboardIcon,
successTextTranslateKey: 'contextMenuItem.copied',
successIcon: Checkmark,
shouldShow: () => true,

// If return value is true, we switch the `text` and `icon` on
// `ContextMenuItem` with `successText` and `successIcon` which will fallback to
// the `text` and `icon`
onPress: (closePopover, {reportAction, selection}) => {
const message = _.last(lodashGet(reportAction, 'message', null));
const html = lodashGet(message, 'html', '');
const text = Str.htmlDecode(selection || lodashGet(message, 'text', ''));
const isAttachment = _.has(reportAction, 'isAttachment')
? reportAction.isAttachment
: isReportMessageAttachment(text);
if (!isAttachment) {
Clipboard.setString(text);
} else {
Clipboard.setString(html);
}
if (closePopover) {
hideContextMenu(true, ReportActionComposeFocusManager.focus);
}
},
},

{
textTranslateKey: 'reportActionContextMenu.copyLink',
icon: LinkCopy,
shouldShow: () => false,
onPress: () => {},
},

{
textTranslateKey: 'reportActionContextMenu.markAsUnread',
icon: Mail,
successIcon: Checkmark,
shouldShow: () => true,
onPress: (closePopover, {reportAction, reportID}) => {
updateLastReadActionID(reportID, reportAction.sequenceNumber);
setNewMarkerPosition(reportID, reportAction.sequenceNumber);
if (closePopover) {
hideContextMenu(true, ReportActionComposeFocusManager.focus);
}
},
},

{
textTranslateKey: 'reportActionContextMenu.editComment',
icon: Pencil,
shouldShow: reportAction => canEditReportAction(reportAction),
onPress: (closePopover, {reportID, reportAction, draftMessage}) => {
const editAction = () => saveReportActionDraft(
reportID,
reportAction.reportActionID,
_.isEmpty(draftMessage) ? getActionText(reportAction) : '',
);

if (closePopover) {
// Hide popover, then call editAction
hideContextMenu(false, editAction);
return;
}

// No popover to hide, call editAction immediately
editAction();
},
},
{
textTranslateKey: 'reportActionContextMenu.deleteComment',
icon: Trashcan,
shouldShow: reportAction => canDeleteReportAction(reportAction),
onPress: (closePopover, {reportID, reportAction}) => {
if (closePopover) {
// Hide popover, then call showDeleteConfirmModal
hideContextMenu(
false,
() => showDeleteModal(reportID, reportAction),
);
return;
}

// No popover to hide, call showDeleteConfirmModal immediately
showDeleteModal(reportID, reportAction);
},
},
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import PropTypes from 'prop-types';
import ReportActionPropTypes from '../ReportActionPropTypes';

const propTypes = {
/** The ID of the report this report action is attached to. */
reportID: PropTypes.number.isRequired,

/** The report action this context menu is attached to. */
reportAction: PropTypes.shape(ReportActionPropTypes).isRequired,

/** If true, this component will be a small, row-oriented menu that displays icons but not text.
If false, this component will be a larger, column-oriented menu that displays icons alongside text in each row. */
isMini: PropTypes.bool,

/** Controls the visibility of this component. */
isVisible: PropTypes.bool,

/** The copy selection of text. */
selection: PropTypes.string,

/** Draft message - if this is set the comment is in 'edit' mode */
draftMessage: PropTypes.string,
};

const defaultProps = {
isMini: false,
isVisible: false,
selection: '',
draftMessage: '',
};

export {propTypes, defaultProps};
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import _ from 'underscore';
import React from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import {
propTypes as GenericReportActionContextMenuPropTypes,
defaultProps as GenericReportActionContextMenuDefaultProps,
} from '../GenericReportActionContextMenuPropTypes';
import {getMiniReportActionContextMenuWrapperStyle} from '../../../../../styles/getReportActionItemStyles';
import BaseReportActionContextMenu from '../BaseReportActionContextMenu';

const propTypes = {
..._.omit(GenericReportActionContextMenuPropTypes, ['isMini']),

/** Should the reportAction this menu is attached to have the appearance of being
* grouped with the previous reportAction? */
displayAsGroup: PropTypes.bool,
};

const defaultProps = {
..._.omit(GenericReportActionContextMenuDefaultProps, ['isMini']),
displayAsGroup: false,
};

const MiniReportActionContextMenu = props => (
<View style={getMiniReportActionContextMenuWrapperStyle(props.displayAsGroup)}>
{/* eslint-disable-next-line react/jsx-props-no-spreading */}
<BaseReportActionContextMenu isMini {...props} />
</View>
);

MiniReportActionContextMenu.propTypes = propTypes;
MiniReportActionContextMenu.defaultProps = defaultProps;
MiniReportActionContextMenu.displayName = 'MiniReportActionContextMenu';

export default MiniReportActionContextMenu;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => null;
Loading