Skip to content

Commit

Permalink
chore: remove the switch_report event
Browse files Browse the repository at this point in the history
  • Loading branch information
adhorodyski committed Nov 12, 2024
1 parent 67a7ec8 commit fbc01be
Show file tree
Hide file tree
Showing 8 changed files with 10 additions and 68 deletions.
2 changes: 0 additions & 2 deletions contributingGuides/PERFORMANCE_METRICS.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ Project is using Firebase for tracking these metrics. However, not all of them a
| `search_filter_options` || Time taken to filter search options in Chat Finder by given search value.<br><br>**Platforms:** All | Starts when user types something in the Chat Finder search input. | Stops when the list of filtered options is generated. |
| `trie_initialization` || Time taken to build the emoji trie.<br><br>**Platforms:** All | Starts when emoji trie begins to build. | Stops when emoji trie building is complete. |
| `open_report` || Time taken to open a report.<br><br>**Platforms:** All | Starts when the row in the `LHNOptionsList` is pressed. | Stops when the `ReportActionsList` finishes laying out. |
| `switch_report` || Time taken to open report.<br><br>**Platforms:** All | Starts when the chat in the LHN is pressed. | Stops when the `ReportActionsList` finishes laying out. |
| `open_report_from_preview` || Time taken to open a report from preview.<br><br>(previously `switch_report_from_preview`)<br><br>**Platforms:** All | Starts when the user presses the Report Preview. | Stops when the `ReportActionsList` finishes laying out. |
| `switch_report_from_preview` || **[REMOVED]** Time taken to open a report from preview. | Starts when the user presses the Report Preview. | Stops when the `ReportActionsList` finishes laying out. |
| `open_report_thread` || Time taken to open a thread in a report.<br><br>**Platforms:** All | Starts when user presses Report Action Item. | Stops when the `ReportActionsList` finishes laying out. |
| `message_sent` || Time taken to send a message.<br><br>**Platforms:** All | Starts when the new message is sent. | Stops when the message is being rendered in the chat. |

Expand Down
1 change: 0 additions & 1 deletion src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,6 @@ const CONST = {
CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION: 'calc_most_recent_last_modified_action',
SEARCH_ROUTER_RENDER: 'search_router_render',
OPEN_REPORT: 'open_report',
SWITCH_REPORT: 'switch_report',
OPEN_REPORT_FROM_PREVIEW: 'open_report_from_preview',
OPEN_REPORT_THREAD: 'open_report_thread',
LOAD_SEARCH_OPTIONS: 'load_search_options',
Expand Down
21 changes: 1 addition & 20 deletions src/libs/E2E/tests/linkingTest.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import {DeviceEventEmitter} from 'react-native';
import type {NativeConfig} from 'react-native-config';
import Config from 'react-native-config';
import E2ELogin from '@libs/E2E/actions/e2eLogin';
import waitForAppLoaded from '@libs/E2E/actions/waitForAppLoaded';
import E2EClient from '@libs/E2E/client';
import getConfigValueOrThrow from '@libs/E2E/utils/getConfigValueOrThrow';
import getPromiseWithResolve from '@libs/E2E/utils/getPromiseWithResolve';
import Performance from '@libs/Performance';
import CONST from '@src/CONST';

type ViewableItem = {
reportActionID?: string;
Expand All @@ -19,15 +16,14 @@ const test = (config: NativeConfig) => {
console.debug('[E2E] Logging in for comment linking');

const linkedReportActionID = getConfigValueOrThrow('linkedReportActionID', config);
const name = getConfigValueOrThrow('name', config);

E2ELogin().then((neededLogin) => {
if (neededLogin) {
return waitForAppLoaded().then(() => E2EClient.submitTestDone());
}

const [appearMessagePromise, appearMessageResolve] = getPromiseWithResolve();
const [switchReportPromise, switchReportResolve] = getPromiseWithResolve();
const [switchReportPromise] = getPromiseWithResolve();

Promise.all([appearMessagePromise, switchReportPromise])
.then(() => {
Expand All @@ -48,21 +44,6 @@ const test = (config: NativeConfig) => {
console.debug(`[E2E] Provided message id '${res?.at(0)?.item?.reportActionID}' doesn't match to an expected '${linkedReportActionID}'. Waiting for a next one…`);
}
});

Performance.subscribeToMeasurements((entry) => {
if (entry.name === CONST.TIMING.SWITCH_REPORT) {
console.debug('[E2E] Linking: 1');

E2EClient.submitTestResults({
branch: Config.E2E_BRANCH,
name,
metric: entry.duration,
unit: 'ms',
});

switchReportResolve();
}
});
});
};

Expand Down
15 changes: 3 additions & 12 deletions src/pages/home/report/ReportActionsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {InteractionManager} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import useCopySelectionHelper from '@hooks/useCopySelectionHelper';
import useInitialValue from '@hooks/useInitialValue';
import useNetwork from '@hooks/useNetwork';
import usePrevious from '@hooks/usePrevious';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
Expand Down Expand Up @@ -285,7 +284,6 @@ function ReportActionsView({
const hasMoreCached = reportActions.length < combinedReportActions.length;
const newestReportAction = useMemo(() => reportActions?.at(0), [reportActions]);
const mostRecentIOUReportActionID = useMemo(() => ReportActionsUtils.getMostRecentIOURequestActionID(reportActions), [reportActions]);
const hasCachedActionOnFirstRender = useInitialValue(() => reportActions.length > 0);
const hasNewestReportAction = reportActions.at(0)?.created === report.lastVisibleActionCreated || reportActions.at(0)?.created === transactionThreadReport?.lastVisibleActionCreated;
const oldestReportAction = useMemo(() => reportActions?.at(-1), [reportActions]);

Expand Down Expand Up @@ -427,17 +425,11 @@ function ReportActionsView({
}

didLayout.current = true;
// Capture the init measurement only once not per each chat switch as the value gets overwritten
if (!ReportActionsView.initMeasured) {
Performance.markEnd(CONST.TIMING.OPEN_REPORT);
ReportActionsView.initMeasured = true;
} else {
Performance.markEnd(CONST.TIMING.SWITCH_REPORT);
}
Timing.end(CONST.TIMING.SWITCH_REPORT, hasCachedActionOnFirstRender ? CONST.TIMING.WARM : CONST.TIMING.COLD);

Performance.markEnd(CONST.TIMING.OPEN_REPORT);
Timing.end(CONST.TIMING.OPEN_REPORT_THREAD);
Timing.end(CONST.TIMING.OPEN_REPORT_FROM_PREVIEW);
}, [hasCachedActionOnFirstRender]);
}, []);

// Check if the first report action in the list is the one we're currently linked to
const isTheFirstReportActionIsLinked = newestReportAction?.reportActionID === reportActionID;
Expand Down Expand Up @@ -500,7 +492,6 @@ function ReportActionsView({
}

ReportActionsView.displayName = 'ReportActionsView';
ReportActionsView.initMeasured = false;

function arePropsEqual(oldProps: ReportActionsViewProps, newProps: ReportActionsViewProps): boolean {
if (!lodashIsEqual(oldProps.reportActions, newProps.reportActions)) {
Expand Down
8 changes: 2 additions & 6 deletions src/pages/home/sidebar/SidebarLinks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ import ROUTES from '@src/ROUTES';
import type {Report} from '@src/types/onyx';

type SidebarLinksProps = {
/** Toggles the navigation menu open and closed */
onLinkClick: () => void;

/** Safe area insets required for mobile devices margins */
insets: EdgeInsets;

Expand All @@ -40,7 +37,7 @@ type SidebarLinksProps = {
activeWorkspaceID: string | undefined;
};

function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priorityMode = CONST.PRIORITY_MODE.DEFAULT, isActiveReport}: SidebarLinksProps) {
function SidebarLinks({insets, optionListItems, isLoading, priorityMode = CONST.PRIORITY_MODE.DEFAULT, isActiveReport}: SidebarLinksProps) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const {updateLocale} = useLocalize();
Expand Down Expand Up @@ -75,9 +72,8 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority
return;
}
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(option.reportID));
onLinkClick();
},
[shouldUseNarrowLayout, isActiveReport, onLinkClick],
[shouldUseNarrowLayout, isActiveReport],
);

const viewMode = priorityMode === CONST.PRIORITY_MODE.GSD ? CONST.OPTION_MODE.COMPACT : CONST.OPTION_MODE.DEFAULT;
Expand Down
14 changes: 3 additions & 11 deletions src/pages/home/sidebar/SidebarLinksData.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,11 @@ type SidebarLinksDataOnyxProps = {
};

type SidebarLinksDataProps = SidebarLinksDataOnyxProps & {
/** Toggles the navigation menu open and closed */
onLinkClick: () => void;

/** Safe area insets required for mobile devices margins */
insets: EdgeInsets;
};

function SidebarLinksData({insets, isLoadingApp = true, onLinkClick, priorityMode = CONST.PRIORITY_MODE.DEFAULT}: SidebarLinksDataProps) {
function SidebarLinksData({insets, isLoadingApp = true, priorityMode = CONST.PRIORITY_MODE.DEFAULT}: SidebarLinksDataProps) {
const isFocused = useIsFocused();
const styles = useThemeStyles();
const activeWorkspaceID = useActiveWorkspaceFromNavigationState();
Expand Down Expand Up @@ -63,7 +60,6 @@ function SidebarLinksData({insets, isLoadingApp = true, onLinkClick, priorityMod
>
<SidebarLinks
// Forwarded props:
onLinkClick={onLinkClick}
insets={insets}
priorityMode={priorityMode ?? CONST.PRIORITY_MODE.DEFAULT}
// Data props:
Expand All @@ -87,17 +83,13 @@ export default withOnyx<SidebarLinksDataProps, SidebarLinksDataOnyxProps>({
initialValue: CONST.PRIORITY_MODE.DEFAULT,
},
})(
/*
/*
While working on audit on the App Start App metric we noticed that by memoizing SidebarLinksData we can avoid 2 additional run of getOrderedReportIDs.
With that we can reduce app start up time by ~2s on heavy account.
More details - https://github.com/Expensify/App/issues/35234#issuecomment-1926914534
*/
memo(
SidebarLinksData,
(prevProps, nextProps) =>
prevProps.isLoadingApp === nextProps.isLoadingApp &&
prevProps.priorityMode === nextProps.priorityMode &&
lodashIsEqual(prevProps.insets, nextProps.insets) &&
prevProps.onLinkClick === nextProps.onLinkClick,
(prevProps, nextProps) => prevProps.isLoadingApp === nextProps.isLoadingApp && prevProps.priorityMode === nextProps.priorityMode && lodashIsEqual(prevProps.insets, nextProps.insets),
),
);
16 changes: 1 addition & 15 deletions src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,9 @@ import {updateLastAccessedWorkspace} from '@libs/actions/Policy/Policy';
import * as Browser from '@libs/Browser';
import TopBar from '@libs/Navigation/AppNavigator/createCustomBottomTabNavigator/TopBar';
import Navigation from '@libs/Navigation/Navigation';
import Performance from '@libs/Performance';
import SidebarLinksData from '@pages/home/sidebar/SidebarLinksData';
import Timing from '@userActions/Timing';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';

/**
* Function called when a pinned chat is selected.
*/
const startTimer = () => {
Timing.start(CONST.TIMING.SWITCH_REPORT);
Performance.markStart(CONST.TIMING.SWITCH_REPORT);
};

function BaseSidebarScreen() {
const styles = useThemeStyles();
const activeWorkspaceID = useActiveWorkspaceFromNavigationState();
Expand Down Expand Up @@ -58,10 +47,7 @@ function BaseSidebarScreen() {
shouldDisplaySearch={shouldDisplaySearch}
/>
<View style={[styles.flex1]}>
<SidebarLinksData
onLinkClick={startTimer}
insets={insets}
/>
<SidebarLinksData insets={insets} />
</View>
</>
)}
Expand Down
1 change: 0 additions & 1 deletion tests/utils/LHNTestUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ function MockedSidebarLinks({currentReportID = ''}: MockedSidebarLinksProps) {
* */}
<ReportIDsContextProvider currentReportIDForTests={currentReportID}>
<SidebarLinksData
onLinkClick={() => {}}
insets={{
top: 0,
left: 0,
Expand Down

0 comments on commit fbc01be

Please sign in to comment.