Skip to content

Commit

Permalink
Revert LHN Ordering
Browse files Browse the repository at this point in the history
  • Loading branch information
shubham1206agra committed Mar 11, 2024
1 parent f6b45c6 commit 8f32e60
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 140 deletions.
2 changes: 0 additions & 2 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1498,8 +1498,6 @@ const CONST = {
ALPHABETIC_AND_LATIN_CHARS: /^[\p{Script=Latin} ]*$/u,
NON_ALPHABETIC_AND_NON_LATIN_CHARS: /[^\p{Script=Latin}]/gu,
ACCENT_LATIN_CHARS: /[\u00C0-\u017F]/g,
INVALID_DISPLAY_NAME_LHN: /[^\p{L}\p{N}\u00C0-\u017F\s-]/gu,
INVALID_DISPLAY_NAME_ONLY_LHN: /^[^\p{L}\p{N}\u00C0-\u017F]$/gu,
POSITIVE_INTEGER: /^\d+$/,
PO_BOX: /\b[P|p]?(OST|ost)?\.?\s*[O|o|0]?(ffice|FFICE)?\.?\s*[B|b][O|o|0]?[X|x]?\.?\s+[#]?(\d+)\b/,
ANY_VALUE: /^.+$/,
Expand Down
4 changes: 0 additions & 4 deletions src/components/LHNOptionsList/LHNOptionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ function LHNOptionsList({
draftComments = {},
transactionViolations = {},
onFirstItemRendered = () => {},
reportIDsWithErrors = {},
}: LHNOptionsListProps) {
const styles = useThemeStyles();
const {canUseViolations} = usePermissions();
Expand Down Expand Up @@ -64,7 +63,6 @@ function LHNOptionsList({
const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? '';
const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions);
const lastReportAction = sortedReportActions[0];
const reportErrors = reportIDsWithErrors[reportID] ?? {};

// Get the transaction for the last report action
let lastReportActionTransactionID = '';
Expand Down Expand Up @@ -93,7 +91,6 @@ function LHNOptionsList({
transactionViolations={transactionViolations}
canUseViolations={canUseViolations}
onLayout={onLayoutItem}
reportErrors={reportErrors}
/>
);
},
Expand All @@ -112,7 +109,6 @@ function LHNOptionsList({
transactionViolations,
canUseViolations,
onLayoutItem,
reportIDsWithErrors,
],
);

Expand Down
4 changes: 1 addition & 3 deletions src/components/LHNOptionsList/OptionRowLHNData.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ function OptionRowLHNData({
lastReportActionTransaction = {},
transactionViolations,
canUseViolations,
reportErrors,
...propsToForward
}: OptionRowLHNDataProps) {
const reportID = propsToForward.reportID;
Expand All @@ -41,11 +40,11 @@ function OptionRowLHNData({
// Note: ideally we'd have this as a dependent selector in onyx!
const item = SidebarUtils.getOptionData({
report: fullReport,
reportActions,
personalDetails,
preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT,
policy,
parentReportAction,
reportErrors,
hasViolations: !!hasViolations,
});
if (deepEqual(item, optionItemRef.current)) {
Expand All @@ -70,7 +69,6 @@ function OptionRowLHNData({
transactionViolations,
canUseViolations,
receiptTransactions,
reportErrors,
]);

useEffect(() => {
Expand Down
7 changes: 0 additions & 7 deletions src/components/LHNOptionsList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type {CurrentReportIDContextValue} from '@components/withCurrentReportID'
import type CONST from '@src/CONST';
import type {OptionData} from '@src/libs/ReportUtils';
import type {Locale, PersonalDetailsList, Policy, Report, ReportAction, ReportActions, Transaction, TransactionViolation} from '@src/types/onyx';
import type * as OnyxCommon from '@src/types/onyx/OnyxCommon';
import type {EmptyObject} from '@src/types/utils/EmptyObject';

type OptionMode = ValueOf<typeof CONST.OPTION_MODE>;
Expand Down Expand Up @@ -59,9 +58,6 @@ type CustomLHNOptionsListProps = {

/** Callback to fire when the list is laid out */
onFirstItemRendered: () => void;

/** Report IDs with errors mapping to their corresponding error objects */
reportIDsWithErrors: Record<string, OnyxCommon.Errors>;
};

type LHNOptionsListProps = CustomLHNOptionsListProps & CurrentReportIDContextValue & LHNOptionsListOnyxProps;
Expand Down Expand Up @@ -117,9 +113,6 @@ type OptionRowLHNDataProps = {

/** Callback to execute when the OptionList lays out */
onLayout?: (event: LayoutChangeEvent) => void;

/** The report errors */
reportErrors: OnyxCommon.Errors | undefined;
};

type OptionRowLHNProps = {
Expand Down
4 changes: 2 additions & 2 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ function getSearchText(
/**
* Get an object of error messages keyed by microtime by combining all error objects related to the report.
*/
function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<ReportActions>, transactions: OnyxCollection<Transaction> = allTransactions): OnyxCommon.Errors {
function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<ReportActions>): OnyxCommon.Errors {
const reportErrors = report?.errors ?? {};
const reportErrorFields = report?.errorFields ?? {};
const reportActionErrors: OnyxCommon.ErrorFields = Object.values(reportActions ?? {}).reduce(
Expand All @@ -492,7 +492,7 @@ function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<

if (parentReportAction?.actorAccountID === currentUserAccountID && ReportActionUtils.isTransactionThread(parentReportAction)) {
const transactionID = parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? parentReportAction?.originalMessage?.IOUTransactionID : null;
const transaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`];
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`];
if (TransactionUtils.hasMissingSmartscanFields(transaction ?? null) && !ReportUtils.isSettled(transaction?.reportID)) {
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
}
Expand Down
45 changes: 15 additions & 30 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PersonalDetails, PersonalDetailsList, TransactionViolation} from '@src/types/onyx';
import type Beta from '@src/types/onyx/Beta';
import type * as OnyxCommon from '@src/types/onyx/OnyxCommon';
import type Policy from '@src/types/onyx/Policy';
import type Report from '@src/types/onyx/Report';
import type {ReportActions} from '@src/types/onyx/ReportAction';
Expand Down Expand Up @@ -58,13 +57,6 @@ function compareStringDates(a: string, b: string): 0 | 1 | -1 {
return 0;
}

function filterDisplayName(displayName: string): string {
if (CONST.REGEX.INVALID_DISPLAY_NAME_ONLY_LHN.test(displayName)) {
return displayName;
}
return displayName.replace(CONST.REGEX.INVALID_DISPLAY_NAME_LHN, '').trim();
}

/**
* @returns An array of reportIDs sorted in the proper order
*/
Expand All @@ -74,27 +66,22 @@ function getOrderedReportIDs(
betas: Beta[],
policies: Record<string, Policy>,
priorityMode: ValueOf<typeof CONST.PRIORITY_MODE>,
allReportActions: OnyxCollection<ReportActions>,
allReportActions: OnyxCollection<ReportAction[]>,
transactionViolations: OnyxCollection<TransactionViolation[]>,
currentPolicyID = '',
policyMemberAccountIDs: number[] = [],
reportIDsWithErrors: Record<string, OnyxCommon.Errors> = {},
canUseViolations = false,
): string[] {
const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD;
const isInDefaultMode = !isInGSDMode;
const allReportsDictValues = Object.values(allReports);

const reportIDsWithViolations = new Set<string>();

// Filter out all the reports that shouldn't be displayed
let reportsToDisplay = allReportsDictValues.filter((report) => {
const parentReportActionsKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`;
const parentReportAction = allReportActions?.[parentReportActionsKey]?.[report.parentReportActionID ?? ''];
const doesReportHaveViolations = canUseViolations && !!parentReportAction && ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction);
if (doesReportHaveViolations) {
reportIDsWithViolations.add(report.reportID);
}
const parentReportActions = allReportActions?.[parentReportActionsKey];
const parentReportAction = parentReportActions?.find((action) => action && report && action?.reportActionID === report?.parentReportActionID);
const doesReportHaveViolations =
betas.includes(CONST.BETAS.VIOLATIONS) && !!parentReportAction && ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction);
return ReportUtils.shouldReportBeInOptionList({
report,
currentReportId: currentReportId ?? '',
Expand All @@ -116,15 +103,15 @@ function getOrderedReportIDs(
}

// The LHN is split into four distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order:
// 1. Pinned/GBR/RBR - Always sorted by reportDisplayName
// 1. Pinned/GBR - Always sorted by reportDisplayName
// 2. Drafts - Always sorted by reportDisplayName
// 3. Non-archived reports and settled IOUs
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
// 4. Archived reports
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
const pinnedAndBrickRoadReports: Report[] = [];
const pinnedAndGBRReports: Report[] = [];
const draftReports: Report[] = [];
const nonArchivedReports: Report[] = [];
const archivedReports: Report[] = [];
Expand All @@ -140,14 +127,12 @@ function getOrderedReportIDs(
// However, this code needs to be very performant to handle thousands of reports, so in the interest of speed, we're just going to disable this lint rule and add
// the reportDisplayName property to the report object directly.
// eslint-disable-next-line no-param-reassign
report.displayName = filterDisplayName(ReportUtils.getReportName(report));

const hasRBR = report.reportID in reportIDsWithErrors || reportIDsWithViolations.has(report.reportID);
report.displayName = ReportUtils.getReportName(report);

const isPinned = report.isPinned ?? false;
const reportAction = ReportActionsUtils.getReportAction(report.parentReportID ?? '', report.parentReportActionID ?? '');
if (isPinned || hasRBR || ReportUtils.requiresAttentionFromCurrentUser(report, reportAction)) {
pinnedAndBrickRoadReports.push(report);
if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report, reportAction)) {
pinnedAndGBRReports.push(report);
} else if (report.hasDraft) {
draftReports.push(report);
} else if (ReportUtils.isArchivedRoom(report)) {
Expand All @@ -158,7 +143,7 @@ function getOrderedReportIDs(
});

// Sort each group of reports accordingly
pinnedAndBrickRoadReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
pinnedAndGBRReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
draftReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));

if (isInDefaultMode) {
Expand All @@ -179,7 +164,7 @@ function getOrderedReportIDs(

// Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID.
// The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar.
const LHNReports = [...pinnedAndBrickRoadReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID);
const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID);
return LHNReports;
}

Expand All @@ -188,19 +173,19 @@ function getOrderedReportIDs(
*/
function getOptionData({
report,
reportActions,
personalDetails,
preferredLocale,
policy,
parentReportAction,
reportErrors,
hasViolations,
}: {
report: OnyxEntry<Report>;
reportActions: OnyxEntry<ReportActions>;
personalDetails: OnyxEntry<PersonalDetailsList>;
preferredLocale: DeepValueOf<typeof CONST.LOCALES>;
policy: OnyxEntry<Policy> | undefined;
parentReportAction: OnyxEntry<ReportAction> | undefined;
reportErrors: OnyxCommon.Errors | undefined;
hasViolations: boolean;
}): ReportUtils.OptionData | undefined {
// When a user signs out, Onyx is cleared. Due to the lazy rendering with a virtual list, it's possible for
Expand All @@ -213,7 +198,7 @@ function getOptionData({
const result: ReportUtils.OptionData = {
text: '',
alternateText: null,
allReportErrors: reportErrors,
allReportErrors: OptionsListUtils.getAllReportErrors(report, reportActions),
brickRoadIndicator: null,
tooltipText: null,
subtitle: null,
Expand Down
3 changes: 1 addition & 2 deletions src/pages/home/sidebar/SidebarLinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const propTypes = {
isActiveReport: PropTypes.func.isRequired,
};

function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priorityMode = CONST.PRIORITY_MODE.DEFAULT, isActiveReport, isCreateMenuOpen, activePolicy, reportIDsWithErrors}) {
function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priorityMode = CONST.PRIORITY_MODE.DEFAULT, isActiveReport, isCreateMenuOpen, activePolicy}) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const modal = useRef({});
Expand Down Expand Up @@ -154,7 +154,6 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority
shouldDisableFocusOptions={isSmallScreenWidth}
optionMode={viewMode}
onFirstItemRendered={App.setSidebarLoaded}
reportIDsWithErrors={reportIDsWithErrors}
/>
{isLoading && optionListItems.length === 0 && (
<View style={[StyleSheet.absoluteFillObject, styles.appBG]}>
Expand Down
Loading

0 comments on commit 8f32e60

Please sign in to comment.