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

Refactor task preview for hidden case #35027

Merged
merged 7 commits into from
Feb 6, 2024
+2 −10
Merged

Refactor task preview for hidden case #35027

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8c802ed
refactor task preview for hidden case
dukenv0307 Jan 24, 2024
bba68ba
resolve conflict
dukenv0307 Jan 28, 2024
357ef0c
fix lint
dukenv0307 Jan 28, 2024
caaf8c1
fix lint
dukenv0307 Jan 29, 2024
852c38d
Merge branch 'main' into fix/34052
dukenv0307 Feb 5, 2024
069e83e
revert change
dukenv0307 Feb 5, 2024
03743f7
fix lint
dukenv0307 Feb 6, 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
2 changes: 1 addition & 1 deletion src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function MentionUserRenderer(props) {
if (!_.isEmpty(htmlAttribAccountID)) {
const user = lodashGet(personalDetails, htmlAttribAccountID);
accountID = parseInt(htmlAttribAccountID, 10);
displayNameOrLogin = LocalePhoneNumber.formatPhoneNumber(lodashGet(user, 'login', '')) || lodashGet(user, 'displayName', '') || translate('common.hidden');
displayNameOrLogin = lodashGet(user, 'displayName', '') || LocalePhoneNumber.formatPhoneNumber(lodashGet(user, 'login', '')) || translate('common.hidden');
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we can replace this line by ReportUtils.getDisplayNameForParticipant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looked at getDisplayNameForParticipant function and yeah we can use this as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

TLDR; we tried to use getDisplayNameForParticipant but we found a bug here, so we decided to keep it as it is for now.
#35027 (comment)

navigationRoute = ROUTES.PROFILE.getRoute(htmlAttribAccountID);
} else if (!_.isEmpty(props.tnode.data)) {
// We need to remove the LTR unicode and leading @ from data as it is not part of the login
Expand Down
10 changes: 1 addition & 9 deletions src/components/ReportActionItem/TaskPreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type {OnyxEntry} from 'react-native-onyx';
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 RenderHTML from '@components/RenderHTML';
import {showContextMenuForReport} from '@components/ShowContextMenuContext';
Expand All @@ -20,7 +19,6 @@ import useThemeStyles from '@hooks/useThemeStyles';
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';
Expand Down Expand Up @@ -84,7 +82,6 @@ function TaskPreview({
}: 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
Expand All @@ -95,13 +92,8 @@ function TaskPreview({
: action?.childStateNum === CONST.REPORT.STATE_NUM.APPROVED && action?.childStatusNum === CONST.REPORT.STATUS_NUM.APPROVED;
const taskTitle = Str.htmlEncode(TaskUtils.getTaskTitle(taskReportID, action?.childReportName ?? ''));
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>`;
taskAssigneeAccountID !== 0 ? `<comment><mention-user accountid="${taskAssigneeAccountID}"></mention-user> ${taskTitle}</comment>` : `<comment>${taskTitle}</comment>`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any linter warning if we keep 3 lines as before instead of 1 line of code, it's hard to read code for me atm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's displayed after I run prettier.

const isDeletedParentAction = ReportUtils.isCanceledTaskReport(taskReport, action);

if (isDeletedParentAction) {
Expand Down
Loading