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

Fix loading skeleton displays behaviors when opening report #43970

Merged
merged 3 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
39 changes: 29 additions & 10 deletions src/pages/home/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import type * as OnyxTypes from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
import HeaderView from './HeaderView';
import getInitialPaginationSize from './report/getInitialPaginationSize';
import ReportActionsView from './report/ReportActionsView';
import ReportFooter from './report/ReportFooter';
import type {ActionListContextType, ReactionListRef, ScrollPosition} from './ReportScreenContext';
Expand Down Expand Up @@ -284,10 +285,18 @@ function ReportScreen({
const screenWrapperStyle: ViewStyle[] = [styles.appContent, styles.flex1, {marginTop: viewportOffsetTop}];
const isEmptyChat = useMemo(() => ReportUtils.isEmptyReport(report), [report]);
const isOptimisticDelete = report.statusNum === CONST.REPORT.STATUS_NUM.CLOSED;
const isLinkedMessageAvailable = useMemo(
(): boolean => sortedAllReportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionIDFromRoute)) > -1,
const indexOfLinkedMessage = useMemo(
(): number => sortedAllReportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionIDFromRoute)),
[sortedAllReportActions, reportActionIDFromRoute],
);
const isCreatedActionExist = ReportActionsUtils.isCreatedAction(reportActions.at(-1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename this to doesCreatedActionExists

const isLinkedMessageAvailable = useMemo(() => indexOfLinkedMessage > -1, [indexOfLinkedMessage]);
const paginationSize = getInitialPaginationSize;

// The linked report actions should have at least minimum amount (15) messages (count as 1 page) above it, to fill the screen.
// If it is too much (same as web pagination size/50) and there is no cached messages on the report,
// the OpenReport will be called each time user scroll up report a bit, click on reportpreview then go back
const isLinkedMessagePageReady = isLinkedMessageAvailable && (sortedAllReportActions.length - indexOfLinkedMessage > 15 || isCreatedActionExist);

// If there's a non-404 error for the report we should show it instead of blocking the screen
const hasHelpfulErrors = Object.keys(report?.errorFields ?? {}).some((key) => key !== 'notFound');
Expand Down Expand Up @@ -379,13 +388,11 @@ function ReportScreen({

const isLoading = !reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty();
const shouldShowSkeleton =
!isLinkedMessageAvailable &&
(isLinkingToMessage ||
!isCurrentReportLoadedFromOnyx ||
(reportActions.length === 0 && !!reportMetadata?.isLoadingInitialReportActions) ||
isLoading ||
(!!reportActionIDFromRoute && reportMetadata?.isLoadingInitialReportActions));
const shouldShowReportActionList = isCurrentReportLoadedFromOnyx && !isLoading;
(isLinkingToMessage && !isLinkedMessagePageReady) ||
!isCurrentReportLoadedFromOnyx ||
(reportActions.length < paginationSize && !isCreatedActionExist && !!reportMetadata?.isLoadingInitialReportActions) ||
isLoading;

const currentReportIDFormRoute = route.params?.reportID;
// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = useMemo(
Expand Down Expand Up @@ -482,6 +489,18 @@ function ReportScreen({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isLoadingReportOnyx]);

useEffect(() => {
if (isLoadingReportOnyx || !reportActionIDFromRoute || isLinkedMessagePageReady) {
return;
}

// This function is triggered when a user clicks on a link to navigate to a report.
// For each link click, we retrieve the report data again, even though it may already be cached.
// There should be only one openReport execution per page start or navigating
fetchReportIfNeeded();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [route, isLinkedMessagePageReady]);

// If a user has chosen to leave a thread, and then returns to it (e.g. with the back button), we need to call `openReport` again in order to allow the user to rejoin and to receive real-time updates
useEffect(() => {
if (!shouldUseNarrowLayout || !isFocused || prevIsFocused || !ReportUtils.isChatThread(report) || report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN) {
Expand Down Expand Up @@ -703,7 +722,7 @@ function ReportScreen({
style={[styles.flex1, styles.justifyContentEnd, styles.overflowHidden]}
onLayout={onListLayout}
>
{shouldShowReportActionList && (
{!shouldShowSkeleton && (
<ReportActionsView
reportActions={reportActions}
report={report}
Expand Down
12 changes: 0 additions & 12 deletions src/pages/home/report/ReportActionsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -251,18 +251,6 @@ function ReportActionsView({
const oldestReportAction = useMemo(() => reportActions?.at(-1), [reportActions]);
const hasCreatedAction = oldestReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED;

useEffect(() => {
if (!reportActionID || indexOfLinkedAction > -1) {
return;
}

// This function is triggered when a user clicks on a link to navigate to a report.
// For each link click, we retrieve the report data again, even though it may already be cached.
// There should be only one openReport execution per page start or navigating
Report.openReport(reportID, reportActionID);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [route, indexOfLinkedAction]);

useEffect(() => {
const wasLoginChangedDetected = prevAuthTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS && !session?.authTokenType;
if (wasLoginChangedDetected && didUserLogInDuringSession() && isUserCreatedPolicyRoom(report)) {
Expand Down
Loading