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

Add feature to delete unverified contact methods #54784

Merged
merged 9 commits into from
Jan 13, 2025
Merged

Add feature to delete unverified contact methods #54784

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0a78fbe
Add feature to delete unverified contact methods
Shahidullah-Muffakir Jan 3, 2025
a88ac96
Fix iOS modal issue by nesting ConfirmModal inside ValidateCodeAction…
Shahidullah-Muffakir Jan 7, 2025
c50cfc5
Merge branch 'main' of github.com:Expensify/App into fix/54626
Shahidullah-Muffakir Jan 7, 2025
715d047
Dismiss keyboard when user clicks three-dot menu
Shahidullah-Muffakir Jan 8, 2025
874605e
apply blurActiveElement() for web, Keyboard.dismiss() for native apps
Shahidullah-Muffakir Jan 10, 2025
9ccb8bf
Hide keyboard when user clicks on three-dot menue, use blurActiveEle…
Shahidullah-Muffakir Jan 11, 2025
dedbbfd
Merge branch 'main' of github.com:Expensify/App into fix/54626
Shahidullah-Muffakir Jan 11, 2025
32da5c6
Fix modal stacking issue by using Modal.close before opening delete c…
Shahidullah-Muffakir Jan 12, 2025
5413f30
use threeDotsMenuItems.length for showing the threeDotMenu
Shahidullah-Muffakir Jan 13, 2025
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
11 changes: 11 additions & 0 deletions src/components/ValidateCodeActionModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import Modal from '@components/Modal';
import ScreenWrapper from '@components/ScreenWrapper';
import Text from '@components/Text';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import Navigation from '@libs/Navigation/Navigation';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand All @@ -29,10 +30,15 @@ function ValidateCodeActionModal({
hasMagicCodeBeenSent,
isLoading,
shouldHandleNavigationBack,
shouldShowThreeDotsButton = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the value of this prop be determined by the length of threeDotsMenuItems? ie. it should be TRUE when there are items in the array and FALSE when there are no items in the array? It seems unnecessary to have the shouldShowThreeDotsButton prop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I’ll update it based on threeDotsMenuItems.length as
shouldShowThreeDotsButton={threeDotsMenuItems.length>0}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tgolen , after looking at it again, I realized that the three-dot menu only shows when the contact is not primary and not verified. If I use the threeDotsMenuItems.length>0, it would always show the button, so I guess we need this prop.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. If I follow that logic though, if the menu is not shown, why would it need to have any menu items at all? I would say, remove the menu items based on the contact is not primary and not verified and it should still work the same. Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, we can do that as well.
I added the shouldShowThreeDotsButton prop because it was already available in the props for the ThreeDotsMenu and HeaderWithBackButton
It seems we could use the same logic there as well, but that would need to update the threeDotsMenuItems in different pages

threeDotsMenuItems = [],
onThreeDotsButtonPress = () => {},
}: ValidateCodeActionModalProps) {
const themeStyles = useThemeStyles();
const firstRenderRef = useRef(true);
const validateCodeFormRef = useRef<ValidateCodeFormHandle>(null);
const styles = useThemeStyles();
const {windowWidth} = useWindowDimensions();

const [validateCodeAction] = useOnyx(ONYXKEYS.VALIDATE_ACTION_CODE);

Expand Down Expand Up @@ -76,6 +82,11 @@ function ValidateCodeActionModal({
<HeaderWithBackButton
title={title}
onBackButtonPress={hide}
threeDotsMenuItems={threeDotsMenuItems}
shouldShowThreeDotsButton={shouldShowThreeDotsButton}
shouldOverlayDots
threeDotsAnchorPosition={styles.threeDotsPopoverOffset(windowWidth)}
onThreeDotsButtonPress={onThreeDotsButtonPress}
/>

<View style={[themeStyles.ph5, themeStyles.mt3, themeStyles.mb5, themeStyles.flex1]}>
Expand Down
10 changes: 10 additions & 0 deletions src/components/ValidateCodeActionModal/type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type React from 'react';
import type {PopoverMenuItem} from '@components/PopoverMenu';
import type {Errors, PendingAction} from '@src/types/onyx/OnyxCommon';

type ValidateCodeActionModalProps = {
Expand Down Expand Up @@ -46,6 +47,15 @@ type ValidateCodeActionModalProps = {

/** Whether handle navigation back when modal show. */
shouldHandleNavigationBack?: boolean;

/** List of menu items for more(three dots) menu */
threeDotsMenuItems?: PopoverMenuItem[];

/** Whether we should show a more options (threedots) button */
shouldShowThreeDotsButton?: boolean;

/** Method to trigger when pressing more options button of the header */
onThreeDotsButtonPress?: () => void;
};

// eslint-disable-next-line import/prefer-default-export
Expand Down
46 changes: 30 additions & 16 deletions src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import useLocalize from '@hooks/useLocalize';
import usePrevious from '@hooks/usePrevious';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import blurActiveElement from '@libs/Accessibility/blurActiveElement';
import {canUseTouchScreen} from '@libs/DeviceCapabilities';
import * as ErrorUtils from '@libs/ErrorUtils';
import Navigation from '@libs/Navigation/Navigation';
Expand Down Expand Up @@ -177,6 +178,24 @@ function ContactMethodDetailsPage({route}: ContactMethodDetailsPageProps) {
const isFailedAddContactMethod = !!loginData.errorFields?.addedLogin;
const isFailedRemovedContactMethod = !!loginData.errorFields?.deletedLogin;

const getDeleteConfirmationModal = () => (
<ConfirmModal
title={translate('contacts.removeContactMethod')}
onConfirm={confirmDeleteAndHideModal}
onCancel={() => toggleDeleteModal(false)}
onModalHide={() => {
InteractionManager.runAfterInteractions(() => {
validateCodeFormRef.current?.focusLastSelected?.();
});
}}
prompt={translate('contacts.removeAreYouSure')}
confirmText={translate('common.yesContinue')}
cancelText={translate('common.cancel')}
isVisible={isDeleteModalOpen && !isDefaultContactMethod}
danger
/>
);

const getMenuItems = () => (
<>
{canChangeDefaultContactMethod ? (
Expand Down Expand Up @@ -216,22 +235,7 @@ function ContactMethodDetailsPage({route}: ContactMethodDetailsPageProps) {
/>
</OfflineWithFeedback>
)}

<ConfirmModal
title={translate('contacts.removeContactMethod')}
onConfirm={confirmDeleteAndHideModal}
onCancel={() => toggleDeleteModal(false)}
onModalHide={() => {
InteractionManager.runAfterInteractions(() => {
validateCodeFormRef.current?.focusLastSelected?.();
});
}}
prompt={translate('contacts.removeAreYouSure')}
confirmText={translate('common.yesContinue')}
cancelText={translate('common.cancel')}
isVisible={isDeleteModalOpen && !isDefaultContactMethod}
danger
/>
{getDeleteConfirmationModal()}
</>
);

Expand Down Expand Up @@ -273,6 +277,16 @@ function ContactMethodDetailsPage({route}: ContactMethodDetailsPageProps) {
}}
sendValidateCode={() => User.requestContactMethodValidateCode(contactMethod)}
descriptionPrimary={translate('contacts.enterMagicCode', {contactMethod: formattedContactMethod})}
shouldShowThreeDotsButton={isValidateCodeActionModalVisible}
onThreeDotsButtonPress={() => blurActiveElement()}
Copy link
Member

Choose a reason for hiding this comment

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

Did you test it? There is no logic on native for this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I tested it. The keyboard hides automatically on native when the user clicks the three-dot menu, so there's no need to use Keyboard.dismiss

Copy link
Member

Choose a reason for hiding this comment

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

OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I overlooked that after clicking the remove button. I applied blurActiveElement() for web and Keyboard.dismiss() for native. Retested on all platforms and added new screen recordings.

threeDotsMenuItems={[
{
icon: Expensicons.Trashcan,
text: translate('common.remove'),
onSelected: () => toggleDeleteModal(true),
},
]}
footer={getDeleteConfirmationModal}
/>

{!isValidateCodeActionModalVisible && getMenuItems()}
Expand Down
Loading