Refactor deleteTask() to use useAncestors hook#73796
Refactor deleteTask() to use useAncestors hook#73796luacmartins merged 15 commits intoExpensify:mainfrom
deleteTask() to use useAncestors hook#73796Conversation
|
Hi @luacmartins, the Bug.mov |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
hmm I think it should be 2 in that case, since that's what we can see in the UI after deletion. Let me take a look. |
|
Hmm this issue is caused because we hardcode |
|
I'll work on the other PRs while the backend is being fixed. |
|
I don't really know the context around that anymore, but at least under there this seems to be the reason |
|
Yea, I read that but didn't understand what it means 😅 The comment says we add the field to check if the task is deleted, but the field is hardcoded to true. So where do we check if the task is deleted or not? |
|
Pretty sure it's just used in this fe check Think it goes something like this:
|
|
Hey @thienlnam, @luacmartins. The Bug.mov |
|
They should definitely match. That being said, I think this is also a problem on main, right? If so, let's not block on it. |
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
|
luacmartins
left a comment
There was a problem hiding this comment.
LGTM. @Tony-MK we have conflicts
|
reviewing ... |
|
Can you please check the failing tests @Tony-MK |
…DetailsPage and HeaderView" This reverts commit 08e9dfc.
|
Bump for review, @abzokhattab |
|
Reviewing today |
|
@Tony-MK can you please check the web video in the description, i think you uploaded a wrong one ... i dont think its related to the current issue |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-11-19.at.13.30.18.movAndroid: mWeb ChromeScreen.Recording.2025-11-19.at.13.34.35.moviOS: HybridAppScreen.Recording.2025-11-19.at.13.30.18.moviOS: mWeb SafariScreen.Recording.2025-11-19.at.13.34.35.movMacOS: Chrome / SafariScreen.Recording.2025-11-19.at.13.18.14.movMacOS: DesktopScreen.Recording.2025-11-19.at.13.20.30.mov |
|
changes looking good to me ... let's tackle this comment, then we are good to merge @Tony-MK |
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.2.62-0 🚀
|
|
This PR is failing because of issue ##75739 517410163-672864a2-2487-4d70-97ab-97ed57d68357.mp4 |
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.62-5 🚀
|
Explanation of Change
Fixed Issues
$ #73570
PROPOSAL:
Tests
1 reply.Force Offline.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 stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso 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.-.Native.mov
Android: mWeb Chrome
Android.-.mWeb.mov
iOS: Native
iOS.-.Native.mp4
iOS: mWeb Safari
iOS.-.mWeb.mp4
MacOS: Chrome / Safari
MacOS: Desktop
macOS.-.Desktop.mov