[TS migration] Migrate SettingsProfileContacts page to TypeScript#35305
[TS migration] Migrate SettingsProfileContacts page to TypeScript#35305youssef-lr merged 21 commits intoExpensify:mainfrom
SettingsProfileContacts page to TypeScript#35305Conversation
src/components/MagicCodeInput.tsx
Outdated
|
|
||
| MagicCodeInput.displayName = 'MagicCodeInput'; | ||
|
|
||
| export type {MagicCodeInputHandle}; |
There was a problem hiding this comment.
NAB: I would move it below export default ... to match other files
|
|
||
| type ContactMethodDetailsPageProps = ContactMethodDetailsPageOnyxProps & StackScreenProps<SettingsNavigatorParamList, typeof SCREENS.SETTINGS.PROFILE.CONTACT_METHOD_DETAILS>; | ||
|
|
||
| function ContactMethodDetailsPage({loginList = {}, session = {}, myDomainSecurityGroups = {}, securityGroups, isLoadingReportData = true, route}: ContactMethodDetailsPageProps) { |
There was a problem hiding this comment.
| function ContactMethodDetailsPage({loginList = {}, session = {}, myDomainSecurityGroups = {}, securityGroups, isLoadingReportData = true, route}: ContactMethodDetailsPageProps) { | |
| function ContactMethodDetailsPage({loginList, session, myDomainSecurityGroups, securityGroups, isLoadingReportData = true, route}: ContactMethodDetailsPageProps) { |
since we are using optional chaining its not needed to add default values
| /** | ||
| * Gets the current contact method from the route params | ||
| */ | ||
| const contactMethod = useMemo(() => { |
There was a problem hiding this comment.
Please add return type to this const
| type ContactMethodsPageProps = ContactMethodsPageOnyxProps & StackScreenProps<SettingsNavigatorParamList, typeof SCREENS.SETTINGS.PROFILE.CONTACT_METHODS>; | ||
|
|
||
| function ContactMethodsPage(props) { | ||
| function ContactMethodsPage({loginList = {}, session, route}: ContactMethodsPageProps) { |
There was a problem hiding this comment.
Same here , no need to add default value to loginList
| }; | ||
|
|
||
| function NewContactMethodPage(props) { | ||
| function NewContactMethodPage({loginList = {}, route}: NewContactMethodPageProps) { |
| enabledWhenOffline | ||
| > | ||
| <Text style={[styles.mb5]}>{props.translate('common.pleaseEnterEmailOrPhoneNumber')}</Text> | ||
| <Text style={[styles.mb5]}>{translate('common.pleaseEnterEmailOrPhoneNumber')}</Text> |
There was a problem hiding this comment.
| <Text style={[styles.mb5]}>{translate('common.pleaseEnterEmailOrPhoneNumber')}</Text> | |
| <Text style={styles.mb5}>{translate('common.pleaseEnterEmailOrPhoneNumber')}</Text> |
| pendingFields: PropTypes.objectOf(PropTypes.objectOf(PropTypes.string)), | ||
| }).isRequired, | ||
| /** Specifies autocomplete hints for the system, so it can provide autofill */ | ||
| autoComplete?: 'sms-otp' | 'one-time-code'; |
There was a problem hiding this comment.
maybe worth creating a seperate type for that?
| const validateLoginError = ErrorUtils.getEarliestErrorField(loginData, 'validateLogin'); | ||
| const shouldDisableResendValidateCode = props.network.isOffline || props.account.isLoading; | ||
| const focusTimeoutRef = useRef(null); | ||
| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing |
There was a problem hiding this comment.
please add explanation why this was added
| // but still need to show the pending login being deleted while offline. | ||
| const partnerUserID = login.partnerUserID || loginName; | ||
| const menuItemTitle = Str.isSMSLogin(partnerUserID) ? props.formatPhoneNumber(partnerUserID) : partnerUserID; | ||
| const partnerUserID = login?.partnerUserID ?? loginName; |
There was a problem hiding this comment.
| const partnerUserID = login?.partnerUserID ?? loginName; | |
| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | |
| const partnerUserID = login?.partnerUserID || loginName; |
partnerUserID is updated to empty string when the contact is deleted optimistically
| if (!login.partnerUserID && _.isEmpty(pendingAction)) { | ||
| const loginMenuItems = sortedLoginNames.map((loginName) => { | ||
| const login = loginList?.[loginName]; | ||
| const pendingAction = login?.pendingFields?.deletedLogin ?? login?.pendingFields?.addedLogin; |
There was a problem hiding this comment.
| const pendingAction = login?.pendingFields?.deletedLogin ?? login?.pendingFields?.addedLogin; | |
| const pendingAction = login?.pendingFields?.deletedLogin ?? login?.pendingFields?.addedLogin ?? undefined; |
Let's fallback to undefined
| const loginMenuItems = sortedLoginNames.map((loginName) => { | ||
| const login = loginList?.[loginName]; | ||
| const pendingAction = login?.pendingFields?.deletedLogin ?? login?.pendingFields?.addedLogin; | ||
| if (!login?.partnerUserID && isEmptyObject(pendingAction)) { |
There was a problem hiding this comment.
| if (!login?.partnerUserID && isEmptyObject(pendingAction)) { | |
| if (!login?.partnerUserID && !pendingAction) { |
pendingAction is not an object.
| return ( | ||
| <OfflineWithFeedback | ||
| pendingAction={pendingAction} | ||
| pendingAction={pendingAction ?? undefined} |
There was a problem hiding this comment.
| pendingAction={pendingAction ?? undefined} | |
| pendingAction={pendingAction} |
| shouldShowBasicTitle | ||
| shouldShowRightIcon | ||
| disabled={!_.isEmpty(pendingAction)} | ||
| disabled={!isEmptyObject(pendingAction)} |
There was a problem hiding this comment.
| disabled={!isEmptyObject(pendingAction)} | |
| disabled={!!pendingAction} |
| onChangeText={onTextInput} | ||
| errorText={formError.validateCode ? props.translate(formError.validateCode) : ErrorUtils.getLatestErrorMessage(props.account)} | ||
| hasError={!_.isEmpty(validateLoginError)} | ||
| errorText={formError?.validateCode ? translate(formError?.validateCode as TranslationPaths) : ErrorUtils.getLatestErrorMessage(account ?? {})} |
There was a problem hiding this comment.
Let's avoid type casting: Let's create a new type for the formError
type ValidateCodeFormError = {
validateCode?: TranslationPaths;
};| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
| const shouldDisableResendValidateCode = isOffline || account?.isLoading; |
There was a problem hiding this comment.
| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | |
| const shouldDisableResendValidateCode = isOffline || account?.isLoading; | |
| const shouldDisableResendValidateCode = !!isOffline || account?.isLoading; |
There was a problem hiding this comment.
Why do we need this? isOffline is a boolen.
There was a problem hiding this comment.
We can't use nullish coalescing here because it will fail when isOffline is false.
|
|
||
| // Replacing spaces with "hard spaces" to prevent breaking the number | ||
| const formattedContactMethod = Str.isSMSLogin(contactMethod) ? formatPhoneNumber(contactMethod).replace(/ /g, '\u00A0') : contactMethod; | ||
| const hasMagicCodeBeenSent = loginList?.[contactMethod].validateCodeSent; |
There was a problem hiding this comment.
| const hasMagicCodeBeenSent = loginList?.[contactMethod].validateCodeSent; | |
| const hasMagicCodeBeenSent = !!loginData.validateCodeSent; |
|
|
||
| <ValidateCodeForm | ||
| contactMethod={contactMethod} | ||
| hasMagicCodeBeenSent={!!hasMagicCodeBeenSent} |
There was a problem hiding this comment.
| hasMagicCodeBeenSent={!!hasMagicCodeBeenSent} | |
| hasMagicCodeBeenSent={hasMagicCodeBeenSent} |
| </OfflineWithFeedback> | ||
| <OfflineWithFeedback | ||
| pendingAction={lodashGet(loginData, 'pendingFields.validateLogin', null)} | ||
| pendingAction={loginData.pendingFields?.validateLogin ?? undefined} |
There was a problem hiding this comment.
Maybe we have to update PendingAction to accept null values.
|
Resolved all comments. |
Reviewer Checklist
Screenshots/VideosAndroid: NativeCleanShot.2024-02-19.at.02.43.06.mp4Android: mWeb ChromeCleanShot.2024-02-19.at.02.40.43.mp4iOS: NativeSimulator.Screen.Recording.-.iPhone.15.Pro.-.2024-02-19.at.02.22.52.mp4iOS: mWeb SafariSimulator.Screen.Recording.-.iPhone.15.Pro.-.2024-02-19.at.02.29.46.mp4MacOS: Chrome / SafariCleanShot.2024-02-19.at.02.12.10.mp4MacOS: DesktopCleanShot.2024-02-19.at.02.54.24.mp4 |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #25218 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
@fedirjh @youssef-lr I did not see any internal engineer assigned here for approval. |
|
@kubabutkiewicz would you like to give this a final review? |
|
Will do that today! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/youssef-lr in version: 1.4.44-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 1.4.44-13 🚀
|
Details
Migrate
SettingsProfileContactspage to TypeScriptFixed Issues
$ #25218
PROPOSAL:
Tests
Offline tests
NA
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel so the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2024-02-05.at.21.58.22-compressed.mov
Screen.Recording.2024-02-05.at.21.25.57-compressed.mov
Screen.Recording.2024-02-05.at.21.25.14-compressed.mov
Screen.Recording.2024-02-05.at.17.21.25-compressed.mov
MacOS: Desktop
Screen.Recording.2024-02-05.at.21.58.22-compressed.mov
Screen.Recording.2024-02-05.at.21.25.57-compressed.mov
Screen.Recording.2024-02-05.at.21.25.14-compressed.mov
Screen.Recording.2024-02-05.at.17.21.25-compressed.mov