[No QA] updating new "claim and verify domain" link - #77416
Conversation
|
@ZhenjaHorbach Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
| showPreviousTransaction(); | ||
| }; | ||
|
|
||
| const confirmRemoveCurrentTransaction = () => { |
There was a problem hiding this comment.
❌ PERF-4 (docs)
The confirmRemoveCurrentTransaction function is passed as a prop to MoneyRequestConfirmationList but is not memoized with useCallback. This creates a new function instance on every render, causing unnecessary re-renders of the child component.
Suggested fix:
const confirmRemoveCurrentTransaction = useCallback(() => {
showConfirmModal({
title: translate('iou.removeExpense'),
prompt: translate('iou.removeExpenseConfirmation'),
confirmText: translate('common.remove'),
cancelText: translate('common.cancel'),
danger: true,
}).then((result) => {
if (result.action !== ModalActions.CONFIRM) {
return;
}
removeCurrentTransaction();
});
}, [showConfirmModal, translate, removeCurrentTransaction]);| iconRight={Expensicons.Trashcan} | ||
| onPress={() => setIsDeleteReceiptConfirmModalVisible(true)} | ||
| iconRight={expensifyIcons.Trashcan} | ||
| onPress={() => { |
There was a problem hiding this comment.
❌ PERF-4 (docs)
An inline arrow function is passed to the onPress prop, creating a new function instance on every render. This causes the Button component to re-render unnecessarily even when its behavior hasn't changed.
Suggested fix:
Create a memoized callback function:
const handleDeletePress = useCallback(() => {
showConfirmModal({
title: translate('receipt.deleteReceipt'),
prompt: translate('receipt.deleteConfirmation'),
confirmText: translate('common.delete'),
cancelText: translate('common.cancel'),
danger: true,
}).then((result) => {
if (result.action !== ModalActions.CONFIRM) {
return;
}
deleteReceipt();
});
}, [showConfirmModal, translate, deleteReceipt]);
// Then use it:
<Button
shouldShowRightIcon
iconRight={expensifyIcons.Trashcan}
onPress={handleDeletePress}
innerStyles={styles.bgTransparent}
large
/>| @@ -187,29 +185,27 @@ function IOURequestStepSubrate({ | |||
| shouldSetModalVisibility={false} | |||
| threeDotsMenuItems={[ | |||
| { | |||
| icon: Expensicons.Trashcan, | |||
| icon: expensifyIcons.Trashcan, | |||
| text: translate('iou.deleteSubrate'), | |||
| onSelected: () => { | |||
There was a problem hiding this comment.
❌ PERF-4 (docs)
An inline arrow function is passed to the onSelected property in the menu items array, creating a new function instance on every render. This causes the threeDotsMenuItems array to have a new reference on every render, triggering unnecessary re-renders.
Suggested fix:
Create a memoized callback function:
const handleDeleteSubrate = useCallback(() => {
showConfirmModal({
title: translate('iou.deleteSubrate'),
prompt: translate('iou.deleteSubrateConfirmation'),
confirmText: translate('common.delete'),
cancelText: translate('common.cancel'),
shouldEnableNewFocusManagement: true,
danger: true,
}).then((result) => {
if (result.action !== ModalActions.CONFIRM) {
return;
}
deleteSubrateAndHideModal();
});
}, [showConfirmModal, translate, deleteSubrateAndHideModal]);
// Then use it in the menu items:
threeDotsMenuItems={[
{
icon: expensifyIcons.Trashcan,
text: translate('iou.deleteSubrate'),
onSelected: handleDeleteSubrate,
shouldCallAfterModalHide: true,
},
]}| @@ -207,29 +205,27 @@ function IOURequestStepWaypoint({ | |||
| shouldSetModalVisibility={false} | |||
| threeDotsMenuItems={[ | |||
| { | |||
| icon: Expensicons.Trashcan, | |||
| icon: expensifyIcons.Trashcan, | |||
| text: translate('distance.deleteWaypoint'), | |||
| onSelected: () => { | |||
There was a problem hiding this comment.
❌ PERF-4 (docs)
An inline arrow function is passed to the onSelected property in the menu items array, creating a new function instance on every render. This causes the threeDotsMenuItems array to have a new reference on every render, triggering unnecessary re-renders.
Suggested fix:
Create a memoized callback function:
const handleDeleteWaypoint = useCallback(() => {
showConfirmModal({
title: translate('distance.deleteWaypoint'),
prompt: translate('distance.deleteWaypointConfirmation'),
confirmText: translate('common.delete'),
cancelText: translate('common.cancel'),
shouldEnableNewFocusManagement: true,
danger: true,
}).then((result) => {
if (result.action !== ModalActions.CONFIRM) {
return;
}
deleteStopAndHideModal();
});
}, [showConfirmModal, translate, deleteStopAndHideModal]);
// Then use it in the menu items:
threeDotsMenuItems={[
{
icon: expensifyIcons.Trashcan,
text: translate('distance.deleteWaypoint'),
onSelected: handleDeleteWaypoint,
shouldCallAfterModalHide: true,
},
]}
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required.
6d677e4 to
29dcc71
Compare
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
|
@chuckdries Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
The merge-base changed after approval.
The merge-base changed after approval.
The merge-base changed after approval.
The merge-base changed after approval.
The merge-base changed after approval.
The merge-base changed after approval.
The merge-base changed after approval.
|
this has to be the most approved PR in history |
francoisl
left a comment
There was a problem hiding this comment.
Trying to unblock merging 🥲
The merge-base changed after approval.
6204490 to
8fea964
Compare
|
✋ 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/chuckdries in version: 9.2.82-0 🚀
|
|
🚀 Deployed to staging by https://github.com/chuckdries in version: 9.2.82-0 🚀
|
|
🚀 Deployed to staging by https://github.com/chuckdries in version: 9.2.82-0 🚀
|
|
🚀 Deployed to staging by https://github.com/chuckdries in version: 9.2.84-0 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.2.84-8 🚀
|
Explanation of Change
updating the link name after this PR gets released: https://github.com/Expensify/App/pull/77168/files
Fixed Issues
$ #69195
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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)/** comment above it */thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)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