-
Notifications
You must be signed in to change notification settings - Fork 3.9k
fix: refactor modals part 3 (VacationDelegatePage, ContactMethodDetailsPage, WalletPage, VisibilityPage) #79553
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
Changes from all commits
c9d13d0
85b295f
60cd3ab
b53bd9e
ed4bf2d
a0346b4
876a0f0
b6a0afd
a2d4a34
41f6506
8983b1d
566c3b0
a805e23
a59e58f
9b529ec
d711335
f325200
d6fc58f
6a144e1
cafcac5
cb421fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| import {useRoute} from '@react-navigation/native'; | ||
| import React, {useCallback, useMemo, useRef, useState} from 'react'; | ||
| import React, {useMemo} from 'react'; | ||
| import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView'; | ||
| import ConfirmModal from '@components/ConfirmModal'; | ||
| import HeaderWithBackButton from '@components/HeaderWithBackButton'; | ||
| import {ModalActions} from '@components/Modal/Global/ModalContext'; | ||
| import ScreenWrapper from '@components/ScreenWrapper'; | ||
| import SelectionList from '@components/SelectionList'; | ||
| import RadioListItem from '@components/SelectionList/ListItem/RadioListItem'; | ||
| import useConfirmModal from '@hooks/useConfirmModal'; | ||
| import useLocalize from '@hooks/useLocalize'; | ||
| import useReportIsArchived from '@hooks/useReportIsArchived'; | ||
| import setNavigationActionToMicrotaskQueue from '@libs/Navigation/helpers/setNavigationActionToMicrotaskQueue'; | ||
| import type {PlatformStackRouteProp, PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; | ||
| import type {ReportSettingsNavigatorParamList} from '@libs/Navigation/types'; | ||
| import {goBackToDetailsPage, isArchivedNonExpenseReport} from '@libs/ReportUtils'; | ||
|
|
@@ -22,12 +24,12 @@ type VisibilityProps = WithReportOrNotFoundProps & PlatformStackScreenProps<Repo | |
|
|
||
| function VisibilityPage({report}: VisibilityProps) { | ||
| const route = useRoute<PlatformStackRouteProp<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.VISIBILITY>>(); | ||
| const [showConfirmModal, setShowConfirmModal] = useState(false); | ||
| const shouldGoBackToDetailsPage = useRef(false); | ||
| const isReportArchived = useReportIsArchived(report?.reportID); | ||
| const shouldDisableVisibility = isArchivedNonExpenseReport(report, isReportArchived); | ||
| const {translate} = useLocalize(); | ||
|
|
||
| const {showConfirmModal} = useConfirmModal(); | ||
|
|
||
| const visibilityOptions = useMemo( | ||
| () => | ||
| Object.values(CONST.REPORT.VISIBILITY) | ||
|
|
@@ -42,28 +44,32 @@ function VisibilityPage({report}: VisibilityProps) { | |
| [translate, report?.visibility], | ||
| ); | ||
|
|
||
| const goBack = useCallback(() => { | ||
| const goBack = () => { | ||
| goBackToDetailsPage(report, route.params.backTo); | ||
| }, [report, route.params.backTo]); | ||
| }; | ||
|
|
||
| const changeVisibility = useCallback( | ||
| (newVisibility: RoomVisibility) => { | ||
| if (!report) { | ||
| return; | ||
| } | ||
| updateRoomVisibility(report.reportID, report.visibility, newVisibility); | ||
| if (showConfirmModal) { | ||
| shouldGoBackToDetailsPage.current = true; | ||
| } else { | ||
| goBack(); | ||
| } | ||
| }, | ||
| [report, showConfirmModal, goBack], | ||
| ); | ||
| const changeVisibility = (newVisibility: RoomVisibility) => { | ||
| if (!report) { | ||
| return; | ||
| } | ||
| updateRoomVisibility(report.reportID, report.visibility, newVisibility); | ||
| setNavigationActionToMicrotaskQueue(goBack); | ||
| }; | ||
|
|
||
| const hideModal = useCallback(() => { | ||
| setShowConfirmModal(false); | ||
| }, []); | ||
| const showPublicVisibilityModal = async () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ CONSISTENCY-1 (docs)The Looking at the old code, Suggested fix: const changeVisibility = useCallback(
(newVisibility: RoomVisibility) => {
if (!report) {
return;
}
updateRoomVisibility(report.reportID, report.visibility, newVisibility);
goBack();
},
[report, goBack],
); |
||
| const result = await showConfirmModal({ | ||
| title: translate('common.areYouSure'), | ||
| prompt: translate('newRoomPage.publicDescription'), | ||
| confirmText: translate('common.yes'), | ||
| cancelText: translate('common.no'), | ||
| shouldShowCancelButton: true, | ||
| danger: true, | ||
| }); | ||
| if (result.action !== ModalActions.CONFIRM) { | ||
| return; | ||
| } | ||
| changeVisibility(CONST.REPORT.VISIBILITY.PUBLIC); | ||
| }; | ||
|
|
||
| return ( | ||
| <ScreenWrapper | ||
|
|
@@ -80,7 +86,7 @@ function VisibilityPage({report}: VisibilityProps) { | |
| data={visibilityOptions} | ||
| onSelectRow={(option) => { | ||
| if (option.value === CONST.REPORT.VISIBILITY.PUBLIC) { | ||
| setShowConfirmModal(true); | ||
| showPublicVisibilityModal(); | ||
| return; | ||
| } | ||
| changeVisibility(option.value); | ||
|
|
@@ -89,26 +95,6 @@ function VisibilityPage({report}: VisibilityProps) { | |
| initiallyFocusedItemKey={visibilityOptions.find((visibility) => visibility.isSelected)?.keyForList} | ||
| ListItem={RadioListItem} | ||
| /> | ||
| <ConfirmModal | ||
| isVisible={showConfirmModal} | ||
| onConfirm={() => { | ||
| changeVisibility(CONST.REPORT.VISIBILITY.PUBLIC); | ||
| hideModal(); | ||
| }} | ||
| onModalHide={() => { | ||
| if (!shouldGoBackToDetailsPage.current) { | ||
| return; | ||
| } | ||
|
truph01 marked this conversation as resolved.
|
||
| shouldGoBackToDetailsPage.current = false; | ||
| goBack(); | ||
| }} | ||
| onCancel={hideModal} | ||
| title={translate('common.areYouSure')} | ||
| prompt={translate('newRoomPage.publicDescription')} | ||
| confirmText={translate('common.yes')} | ||
| cancelText={translate('common.no')} | ||
| danger | ||
| /> | ||
| </FullPageNotFoundView> | ||
| </ScreenWrapper> | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NAB this component compiles with React Compiler, so
useCallbackisn't needed