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

[TS migration] Migrate Taskpreview.js component to TypeScript #33852

Merged
merged 20 commits into from
Jan 11, 2024
Merged
Changes from 16 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
@@ -1,164 +1,155 @@
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import Str from 'expensify-common/lib/str';
import React from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import {OnyxEntry, withOnyx} from 'react-native-onyx';

Check failure on line 4 in src/components/ReportActionItem/TaskPreview.tsx

View workflow job for this annotation

GitHub Actions / lint

Import "OnyxEntry" is only used as types
import Checkbox from '@components/Checkbox';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import {usePersonalDetails} from '@components/OnyxProvider';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
import refPropTypes from '@components/refPropTypes';
import RenderHTML from '@components/RenderHTML';
import {showContextMenuForReport} from '@components/ShowContextMenuContext';
import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsDefaultProps, withCurrentUserPersonalDetailsPropTypes} from '@components/withCurrentUserPersonalDetails';
import withLocalize, {withLocalizePropTypes} from '@components/withLocalize';
import withCurrentUserPersonalDetails, {WithCurrentUserPersonalDetailsProps} from '@components/withCurrentUserPersonalDetails';

Check failure on line 12 in src/components/ReportActionItem/TaskPreview.tsx

View workflow job for this annotation

GitHub Actions / lint

Import "WithCurrentUserPersonalDetailsProps" is only used as types
import useLocalize from '@hooks/useLocalize';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import compose from '@libs/compose';
import ControlSelection from '@libs/ControlSelection';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import getButtonState from '@libs/getButtonState';
import * as LocalePhoneNumber from '@libs/LocalePhoneNumber';
import Navigation from '@libs/Navigation/Navigation';
import * as ReportUtils from '@libs/ReportUtils';
import * as TaskUtils from '@libs/TaskUtils';
import reportActionPropTypes from '@pages/home/report/reportActionPropTypes';
import * as Session from '@userActions/Session';
import * as Task from '@userActions/Task';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Policy, Report, ReportAction} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

const propTypes = {
/** The ID of the associated taskReport */
taskReportID: PropTypes.string.isRequired,

/** Whether the task preview is hovered so we can modify its style */
isHovered: PropTypes.bool,

/** The linked reportAction */
action: PropTypes.shape(reportActionPropTypes).isRequired,
type PolicyRole = {
/** The role of current user */
role: string;
Copy link
Contributor

@tgolen tgolen Jan 5, 2024

Choose a reason for hiding this comment

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

It looks like some comments are getting dropped. I find the comments very helpful, is it a standard process to remove them? I'd like to keep them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tgolen We preserve all comments, and sometimes we even add new ones for better clarity :)

};

type TaskPreviewOnyxProps = {
/* Onyx Props */

taskReport: PropTypes.shape({
/** Title of the task */
reportName: PropTypes.string,

/** AccountID of the manager in this iou report */
managerID: PropTypes.number,

/** AccountID of the creator of this iou report */
ownerAccountID: PropTypes.number,
}),
/* current report of TaskPreview */
taskReport: OnyxEntry<Report>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment for this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.


/** The policy of root parent report */
rootParentReportpolicy: PropTypes.shape({
/** The role of current user */
role: PropTypes.string,
}),

/** The chat report associated with taskReport */
chatReportID: PropTypes.string.isRequired,

/** Popover context menu anchor, used for showing context menu */
contextMenuAnchor: refPropTypes,

/** Callback for updating context menu active state, used for showing context menu */
checkIfContextMenuActive: PropTypes.func,

/* Onyx Props */
...withLocalizePropTypes,

...withCurrentUserPersonalDetailsPropTypes,
};

const defaultProps = {
...withCurrentUserPersonalDetailsDefaultProps,
taskReport: {},
rootParentReportpolicy: {},
isHovered: false,
rootParentReportpolicy: OnyxEntry<PolicyRole>;
};

function TaskPreview(props) {
type TaskPreviewProps = WithCurrentUserPersonalDetailsProps &
TaskPreviewOnyxProps & {
/** The ID of the associated policy */
// eslint-disable-next-line react/no-unused-prop-types
policyID: string;
/** The ID of the associated taskReport */
taskReportID: string;

/** Whether the task preview is hovered so we can modify its style */
isHovered: boolean;

/** The linked reportAction */
action: OnyxEntry<ReportAction>;

/** The chat report associated with taskReport */
chatReportID: string;

/** Popover context menu anchor, used for showing context menu */
contextMenuAnchor: Element;

/** Callback for updating context menu active state, used for showing context menu */
checkIfContextMenuActive: () => void;
};

function TaskPreview({
taskReport,
taskReportID,
action,
Copy link
Contributor

Choose a reason for hiding this comment

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

@blazejkustra Is there a general rule about when to use destructuring with defaults and when not to? It feels to me like it would make the code easier to follow if we used defaults in the destructuring. The only case I think it won't work is when you need to know if the value was undefined or null, but a simple fasly check like action ?? {} should be fine, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so may I update the code to use default value in destructuring if possible?

Copy link
Contributor

Choose a reason for hiding this comment

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

The rule of thumb is to avoid unnecessary default values such as undefined, null, {}, [], '' and () => {} - when we started migration we left some of these, but we can clean them as a follow up once whole TS migration is finished.

Two examples for useful default values:

function Avatar({
    size = CONST.AVATAR_SIZE.DEFAULT,
    fallbackIcon = Expensicons.FallbackAvatar,
    type = CONST.ICON_TYPE_AVATAR,
}: AvatarProps) {}

function GenericPressable(
    {
        shouldUseHapticsOnLongPress = false,
        shouldUseHapticsOnPress = false,
        shouldUseAutoHitSlop = false,
        enableInScreenReaderStates = CONST.SCREEN_READER_STATES.ALL,
        accessible = true,
        ...rest
    }: PressableProps) {}

contextMenuAnchor,
chatReportID,
checkIfContextMenuActive,
currentUserPersonalDetails,
rootParentReportpolicy,
isHovered = false,
}: TaskPreviewProps) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const personalDetails = usePersonalDetails() || CONST.EMPTY_OBJECT;
const {translate} = useLocalize();
// The reportAction might not contain details regarding the taskReport
// Only the direct parent reportAction will contain details about the taskReport
// Other linked reportActions will only contain the taskReportID and we will grab the details from there
const isTaskCompleted = !_.isEmpty(props.taskReport)
? props.taskReport.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.taskReport.statusNum === CONST.REPORT.STATUS.APPROVED
: props.action.childStateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.action.childStatusNum === CONST.REPORT.STATUS.APPROVED;
const taskTitle = _.escape(TaskUtils.getTaskTitle(props.taskReportID, props.action.childReportName));
const taskAssigneeAccountID = Task.getTaskAssigneeAccountID(props.taskReport) || props.action.childManagerAccountID;
const assigneeLogin = lodashGet(personalDetails, [taskAssigneeAccountID, 'login'], '');
const assigneeDisplayName = lodashGet(personalDetails, [taskAssigneeAccountID, 'displayName'], '');
const isTaskCompleted = !isEmptyObject(taskReport)
? taskReport?.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && taskReport.statusNum === CONST.REPORT.STATUS.APPROVED
: action?.childStateNum === CONST.REPORT.STATE_NUM.SUBMITTED && action?.childStatusNum === CONST.REPORT.STATUS.APPROVED;
const taskTitle = Str.htmlEncode(TaskUtils.getTaskTitle(taskReportID, action?.childReportName ?? ''));
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure these implementations give the same result:

    const taskTitle = _.escape(TaskUtils.getTaskTitle(props.taskReportID, props.action.childReportName));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

image
Same result.

const taskAssigneeAccountID = Task.getTaskAssigneeAccountID(taskReport ?? {}) ?? action?.childManagerAccountID ?? '';
const assigneeLogin = personalDetails[taskAssigneeAccountID]?.login ?? '';
const assigneeDisplayName = personalDetails[taskAssigneeAccountID]?.displayName ?? '';
const taskAssignee = assigneeDisplayName || LocalePhoneNumber.formatPhoneNumber(assigneeLogin);
const htmlForTaskPreview =
taskAssignee && taskAssigneeAccountID !== 0
? `<comment><mention-user accountid="${taskAssigneeAccountID}">@${taskAssignee}</mention-user> ${taskTitle}</comment>`
: `<comment>${taskTitle}</comment>`;
const isDeletedParentAction = ReportUtils.isCanceledTaskReport(props.taskReport, props.action);
const isDeletedParentAction = ReportUtils.isCanceledTaskReport(taskReport, action);

if (isDeletedParentAction) {
return <RenderHTML html={`<comment>${props.translate('parentReportAction.deletedTask')}</comment>`} />;
return <RenderHTML html={`<comment>${translate('parentReportAction.deletedTask')}</comment>`} />;
}

return (
<View style={[styles.chatItemMessage]}>
<PressableWithoutFeedback
onPress={() => Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(props.taskReportID))}
onPress={() => Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(taskReportID))}
onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
onLongPress={(event) => showContextMenuForReport(event, props.contextMenuAnchor, props.chatReportID, props.action, props.checkIfContextMenuActive)}
onLongPress={(event) => showContextMenuForReport(event, contextMenuAnchor, chatReportID, action ?? {}, checkIfContextMenuActive)}
style={[styles.flexRow, styles.justifyContentBetween]}
role={CONST.ROLE.BUTTON}
accessibilityLabel={props.translate('task.task')}
accessibilityLabel={translate('task.task')}
>
<View style={[styles.flex1, styles.flexRow, styles.alignItemsStart]}>
<Checkbox
style={[styles.mr2]}
containerStyle={[styles.taskCheckbox]}
isChecked={isTaskCompleted}
disabled={!Task.canModifyTask(props.taskReport, props.currentUserPersonalDetails.accountID, lodashGet(props.rootParentReportpolicy, 'role', ''))}
disabled={!Task.canModifyTask(taskReport ?? {}, currentUserPersonalDetails.accountID, rootParentReportpolicy?.role ?? '')}
onPress={Session.checkIfActionIsAllowed(() => {
if (isTaskCompleted) {
Task.reopenTask(props.taskReport);
Task.reopenTask(taskReport ?? {});
} else {
Task.completeTask(props.taskReport);
Task.completeTask(taskReport ?? {});
}
})}
accessibilityLabel={props.translate('task.task')}
accessibilityLabel={translate('task.task')}
/>
<RenderHTML html={htmlForTaskPreview} />
</View>
<Icon
src={Expensicons.ArrowRight}
fill={StyleUtils.getIconFillColor(getButtonState(props.isHovered))}
fill={StyleUtils.getIconFillColor(getButtonState(isHovered))}
/>
</PressableWithoutFeedback>
</View>
);
}

TaskPreview.propTypes = propTypes;
TaskPreview.defaultProps = defaultProps;
TaskPreview.displayName = 'TaskPreview';

export default compose(
withLocalize,
withCurrentUserPersonalDetails,
withOnyx({
export default withCurrentUserPersonalDetails(
withOnyx<TaskPreviewProps, TaskPreviewOnyxProps>({
taskReport: {
key: ({taskReportID}) => `${ONYXKEYS.COLLECTION.REPORT}${taskReportID}`,
initialValue: {},
},
rootParentReportpolicy: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID || '0'}`,
selector: (policy) => _.pick(policy, ['role']),
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID ?? '0'}`,
selector: (policy: Policy | null) => ({role: policy?.role ?? ''}),
},
}),
)(TaskPreview);
})(TaskPreview),
);
Loading