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

Fix not found view of linked action doesn't have back button #46132

Merged
merged 4 commits into from
Jul 26, 2024
Merged

Fix not found view of linked action doesn't have back button #46132

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e257a5a
allow empty subtitle
bernhardoj Jul 24, 2024
86edd52
refactor the component to show the back button for not found linked a…
bernhardoj Jul 24, 2024
45db141
don't render the text if subtitle is empty
bernhardoj Jul 24, 2024
a2faf2e
lint
bernhardoj Jul 24, 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
10 changes: 6 additions & 4 deletions src/components/BlockingViews/BlockingView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,12 @@ function BlockingView({
const subtitleText = useMemo(
() => (
<>
<AutoEmailLink
style={[styles.textAlignCenter, subtitleStyle]}
text={subtitle}
/>
{subtitle && (
<AutoEmailLink
style={[styles.textAlignCenter, subtitleStyle]}
text={subtitle}
/>
)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we keep render the text even though subtitle is empty, an extra space can be seen.

Screenshot 2024-07-25 at 00 16 41

{shouldShowLink ? (
<TextLink
onPress={onLinkPress}
Expand Down
6 changes: 3 additions & 3 deletions src/components/BlockingViews/FullPageNotFoundView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ type FullPageNotFoundViewProps = {
/** The key in the translations file to use for the title */
titleKey?: TranslationPaths;

/** The key in the translations file to use for the subtitle */
subtitleKey?: TranslationPaths;
/** The key in the translations file to use for the subtitle. Pass an empty key to not show the subtitle. */
subtitleKey?: TranslationPaths | '';

/** Whether we should show a link to navigate elsewhere */
shouldShowLink?: boolean;
Expand Down Expand Up @@ -71,7 +71,7 @@ function FullPageNotFoundView({
iconWidth={variables.modalTopIconWidth}
iconHeight={variables.modalTopIconHeight}
title={translate(titleKey)}
subtitle={translate(subtitleKey)}
subtitle={subtitleKey && translate(subtitleKey)}
linkKey={linkKey}
shouldShowLink={shouldShowLink}
onLinkPress={onLinkPress}
Expand Down
71 changes: 36 additions & 35 deletions src/pages/home/ReportScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import {useOnyx, withOnyx} from 'react-native-onyx';
import Banner from '@components/Banner';
import BlockingView from '@components/BlockingViews/BlockingView';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import DragAndDropProvider from '@components/DragAndDrop/Provider';
import * as Illustrations from '@components/Icon/Illustrations';
import MoneyReportHeader from '@components/MoneyReportHeader';
import MoneyRequestHeader from '@components/MoneyRequestHeader';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
Expand Down Expand Up @@ -43,7 +41,7 @@
import shouldFetchReport from '@libs/shouldFetchReport';
import * as ValidationUtils from '@libs/ValidationUtils';
import type {AuthScreensParamList} from '@navigation/types';
import variables from '@styles/variables';

Check failure on line 44 in src/pages/home/ReportScreen.tsx

View workflow job for this annotation

GitHub Actions / Run ESLint 

'variables' is defined but never used
import * as ComposerActions from '@userActions/Composer';
import * as Report from '@userActions/Report';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -416,6 +414,12 @@
? reportActions.length > 0
: reportActions.length >= CONST.REPORT.MIN_INITIAL_REPORT_ACTION_COUNT || isPendingActionExist || (doesCreatedActionExists() && reportActions.length > 0);

const isLinkedActionDeleted = useMemo(() => !!linkedAction && !ReportActionsUtils.shouldReportActionBeVisible(linkedAction, linkedAction.reportActionID), [linkedAction]);
const isLinkedActionInaccessibleWhisper = useMemo(
() => !!linkedAction && ReportActionsUtils.isWhisperAction(linkedAction) && !(linkedAction?.whisperedToAccountIDs ?? []).includes(currentUserAccountID),
[currentUserAccountID, linkedAction],
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just moving up the variable definition.


/**
* Using logical OR operator because with nullish coalescing operator, when `isLoadingApp` is false, the right hand side of the operator
* is not evaluated. This causes issues where we have `isLoading` set to false and later set to true and then set to false again.
Expand All @@ -431,10 +435,24 @@
!isCurrentReportLoadedFromOnyx ||
isLoading;

const shouldShowNotFoundLinkedAction =

Check failure on line 438 in src/pages/home/ReportScreen.tsx

View workflow job for this annotation

GitHub Actions / Run ESLint 

Do not use negated variable names
(!isLinkedActionInaccessibleWhisper && isLinkedActionDeleted) ||
(shouldShowSkeleton &&
!reportMetadata.isLoadingInitialReportActions &&
!!reportActionIDFromRoute &&
!!sortedAllReportActions &&
sortedAllReportActions?.length > 0 &&
reportActions.length === 0 &&
!isLinkingToMessage);

const currentReportIDFormRoute = route.params?.reportID;

// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = useMemo((): boolean => {
if (shouldShowNotFoundLinkedAction) {
return true;
}

// Wait until we're sure the app is done loading (needs to be a strict equality check since it's undefined initially)
if (isLoadingApp !== false) {
return false;
Expand All @@ -454,7 +472,17 @@
return true;
}
return !!currentReportIDFormRoute && !ReportUtils.isValidReportIDFromPath(currentReportIDFormRoute);
}, [isLoadingApp, finishedLoadingApp, report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute]);
}, [
shouldShowNotFoundLinkedAction,
isLoadingApp,
finishedLoadingApp,
report.reportID,
isOptimisticDelete,
reportMetadata?.isLoadingInitialReportActions,
userLeavingStatus,
shouldHideReport,
currentReportIDFormRoute,
]);

const fetchReport = useCallback(() => {
Report.openReport(reportIDFromRoute, reportActionIDFromRoute);
Expand Down Expand Up @@ -708,12 +736,6 @@
fetchReport();
}, [fetchReport]);

const isLinkedActionDeleted = useMemo(() => !!linkedAction && !ReportActionsUtils.shouldReportActionBeVisible(linkedAction, linkedAction.reportActionID), [linkedAction]);
const isLinkedActionInaccessibleWhisper = useMemo(
() => !!linkedAction && ReportActionsUtils.isWhisperAction(linkedAction) && !(linkedAction?.whisperedToAccountIDs ?? []).includes(currentUserAccountID),
[currentUserAccountID, linkedAction],
);

// If user redirects to an inaccessible whisper via a deeplink, on a report they have access to,
// then we set reportActionID as empty string, so we display them the report and not the "Not found page".
useEffect(() => {
Expand All @@ -733,29 +755,6 @@
Report.readNewestAction(report.reportID);
}, [report]);

if (
(!isLinkedActionInaccessibleWhisper && isLinkedActionDeleted) ||
(shouldShowSkeleton &&
!reportMetadata.isLoadingInitialReportActions &&
reportActionIDFromRoute &&
sortedAllReportActions &&
sortedAllReportActions?.length > 0 &&
reportActions.length === 0 &&
!isLinkingToMessage)
) {
return (
<BlockingView
icon={Illustrations.ToddBehindCloud}
iconWidth={variables.modalTopIconWidth}
iconHeight={variables.modalTopIconHeight}
title={translate('notFound.notHere')}
shouldShowLink
linkKey="notFound.noAccess"
onLinkPress={navigateToEndOfReport}
/>
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored it so we use FullPageNotFoundView below. It's already wrapped in ScreenWrapper too which adds safe inset for native platform.


return (
<ActionListContext.Provider value={actionListValue}>
<ReactionListContext.Provider value={reactionListRef}>
Expand All @@ -767,10 +766,12 @@
>
<FullPageNotFoundView
shouldShow={shouldShowNotFoundPage}
subtitleKey="notFound.noAccess"
subtitleKey={shouldShowNotFoundLinkedAction ? '' : 'notFound.noAccess'}
shouldShowBackButton={shouldUseNarrowLayout}
onBackButtonPress={Navigation.goBack}
shouldShowLink={false}
onBackButtonPress={shouldShowNotFoundLinkedAction ? navigateToEndOfReport : Navigation.goBack}
shouldShowLink={shouldShowNotFoundLinkedAction}
linkKey="notFound.noAccess"
onLinkPress={navigateToEndOfReport}
>
<OfflineWithFeedback
pendingAction={reportPendingAction}
Expand Down
Loading