Skip to content

[CP Staging][Avatars] Display subscript for single expenses & invoices in Workspace #67993

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

Merged
grgia merged 2 commits into from
Aug 6, 2025
Merged

[CP Staging][Avatars] Display subscript for single expenses & invoices in Workspace #67993

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2273b63
Add a logic for single expense & invoices
JakubKorytko Aug 6, 2025
fc93f8a
Merge branch 'main' into korytko/fix-single-expense-no-subscript
JakubKorytko Aug 6, 2025
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
37 changes: 29 additions & 8 deletions src/components/ReportActionAvatars/useReportActionAvatars.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type {ValueOf} from 'type-fest';
import {FallbackAvatar} from '@components/Icon/Expensicons';
import useOnyx from '@hooks/useOnyx';
import useReportIsArchived from '@hooks/useReportIsArchived';
import {getReportAction} from '@libs/ReportActionsUtils';
import {getOriginalMessage, getReportAction, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {
getDisplayNameForParticipant,
getIcons,
Expand Down Expand Up @@ -37,7 +37,6 @@ function useReportActionAvatars({
policyID?: string;
}) {
/* Get avatar type */

const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {
canBeMissing: true,
});
Expand All @@ -50,7 +49,15 @@ function useReportActionAvatars({
const chatReport = isReportAChatReport ? report : reportChatReport;
const iouReport = isReportAChatReport ? undefined : report;

const action = passedAction ?? (iouReport?.parentReportActionID ? getReportAction(chatReport?.reportID ?? iouReport?.chatReportID, iouReport?.parentReportActionID) : undefined);
let action;

if (passedAction) {
action = passedAction;
} else if (iouReport?.parentReportActionID) {
action = getReportAction(chatReport?.reportID ?? iouReport?.chatReportID, iouReport?.parentReportActionID);
} else if (!!reportChatReport && !!chatReport?.parentReportActionID && !iouReport) {
action = getReportAction(reportChatReport?.reportID, chatReport.parentReportActionID);
}

const isReportArchived = useReportIsArchived(iouReport?.reportID);

Expand Down Expand Up @@ -87,8 +94,9 @@ function useReportActionAvatars({
};
}

const isWorkspacePolicy = !!policy && policy.type !== CONST.POLICY.TYPE.PERSONAL;
const isATripRoom = isTripRoom(chatReport);
const isWorkspaceWithoutChatReportProp = !chatReport && policy?.type !== CONST.POLICY.TYPE.PERSONAL;
const isWorkspaceWithoutChatReportProp = !chatReport && isWorkspacePolicy;
const isAWorkspaceChat = isPolicyExpenseChat(chatReport) || isWorkspaceWithoutChatReportProp;
const isATripPreview = action?.actionName === CONST.REPORT.ACTIONS.TYPE.TRIP_PREVIEW;
const isAReportPreviewAction = action?.actionName === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW;
Expand All @@ -101,13 +109,15 @@ function useReportActionAvatars({
const shouldUseAccountIDs = accountIDs.length > 0;
const shouldShowAllActors = displayAllActors && !reportPreviewSenderID;
const isChatThreadOutsideTripRoom = isChatThread(chatReport) && !isATripRoom;
const shouldShowSubscriptAvatar = shouldReportShowSubscript(iouReport ?? chatReport, isReportArchived) && policy?.type !== CONST.POLICY.TYPE.PERSONAL;
const shouldShowSubscriptAvatar = shouldReportShowSubscript(iouReport ?? chatReport, isReportArchived) && isWorkspacePolicy;
Copy link
Copy Markdown
Contributor

@grgia grgia Aug 6, 2025

Choose a reason for hiding this comment

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

shouldReportShowSubscript

Instead of add the case outside of it, could we update this function?

Copy link
Copy Markdown
Contributor Author

@JakubKorytko JakubKorytko Aug 6, 2025

Choose a reason for hiding this comment

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

Well, it's not that easy. It's more a matter of which report to pass and which report to pass to the icons function. For example, the invoice single expense view has no IOU report, and the invoice report is actually a parent of its chat report. The whole logic revolves around that. The function itself performs checks such as isExpenseRequest so it is correct.

But I agree that there are sooo many checks that it needs a refractor and some utils functions and I am willing to do such as well as the optimization but I think we should focus to cover everything and do refractor as a last PR, same as we did with Better Expense Report View and it worked well.

What do you think?

const shouldShowConvertedSubscriptAvatar = (shouldStackHorizontally || shouldUseAccountIDs) && shouldShowSubscriptAvatar && !reportPreviewSenderID;
const isExpense = isMoneyRequestAction(action) && getOriginalMessage(action)?.type === CONST.IOU.ACTION.CREATE;
const isWorkspaceExpense = isWorkspacePolicy && isExpense;

const shouldUseSubscriptAvatar =
(((shouldShowSubscriptAvatar && isReportPreviewOrNoAction) || isReportPreviewInTripRoom || isATripPreview) &&
(((shouldShowSubscriptAvatar && isReportPreviewOrNoAction) || isReportPreviewInTripRoom || isATripPreview || isWorkspaceExpense) &&
!shouldStackHorizontally &&
!isChatThreadOutsideTripRoom &&
!(isChatThreadOutsideTripRoom && !isWorkspaceExpense) &&
!shouldShowConvertedSubscriptAvatar) ||
shouldUseCardFeed;

Expand All @@ -134,6 +144,8 @@ function useReportActionAvatars({

const defaultDisplayName = getDisplayNameForParticipant({accountID, personalDetailsData: personalDetails}) ?? '';
const isAInvoiceReport = isInvoiceReport(iouReport ?? null);
const invoiceReport = [iouReport, chatReport, reportChatReport].find(isInvoiceReport);
const isNestedInInvoiceReport = !!invoiceReport;
Copy link
Copy Markdown
Contributor

@dukenv0307 dukenv0307 Sep 24, 2025

Choose a reason for hiding this comment

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

Coming from #69343, we should add the check for the chat thread as well

const isWorkspaceActor = isAInvoiceReport || (isAWorkspaceChat && (!actorAccountID || displayAllActors));
const isChatReportOnlyProp = !iouReport && chatReport;
const isWorkspaceChatWithoutChatReport = !chatReport && isAWorkspaceChat;
Expand Down Expand Up @@ -206,9 +218,18 @@ function useReportActionAvatars({

const shouldUseMappedAccountIDs = avatarsForAccountIDs.length > 0 && (avatarType === CONST.REPORT_ACTION_AVATARS.TYPE.MULTIPLE || shouldUseActorAccountID);
const shouldUsePrimaryAvatarID = isWorkspaceActor && !!primaryAvatar.id;
const shouldUseInvoiceExpenseIcons = isWorkspaceExpense && isNestedInInvoiceReport && !!accountID;

let avatars = [primaryAvatar, secondaryAvatar];

if (shouldUseInvoiceExpenseIcons) {
avatars = getIconsWithDefaults(invoiceReport);
} else if (shouldUseMappedAccountIDs) {
avatars = avatarsForAccountIDs;
Comment on lines +225 to +228
Copy link
Copy Markdown
Contributor

@grgia grgia Aug 6, 2025

Choose a reason for hiding this comment

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

Is this the centralized place where these checks are made?

Copy link
Copy Markdown
Contributor Author

@JakubKorytko JakubKorytko Aug 6, 2025

Choose a reason for hiding this comment

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

Yes, this hook is used inside a component that is used for every report or report action avatar in the app.

}

return {
avatars: shouldUseMappedAccountIDs ? avatarsForAccountIDs : [primaryAvatar, secondaryAvatar],
avatars,
avatarType,
details: {
...(personalDetails?.[accountID] ?? {}),
Expand Down
Loading