Part 1: Refactor ConfirmModal usage to useConfirmModal in workspace feature pages#78060
Part 1: Refactor ConfirmModal usage to useConfirmModal in workspace feature pages#78060roryabraham merged 19 commits intoExpensify:mainfrom
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] |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
No product considerations, removing my review 👍 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-01-26.14.54.34.mov2026-01-26.14.56.04.mov2026-01-26.15.02.36.mov2026-01-26.15.02.56.movAndroid: mWeb Chrome2026-01-26.15.05.11.mov2026-01-26.15.05.33.mov2026-01-26.15.06.05.mov2026-01-26.15.06.18.moviOS: HybridApp2026-01-26.14.28.04.mov2026-01-26.14.28.29.mov2026-01-26.14.29.14.mov2026-01-26.14.29.28.moviOS: mWeb Safari2026-01-26.14.34.36.mov2026-01-26.14.35.15.mov2026-01-26.14.35.47.mov2026-01-26.14.36.03.movMacOS: Chrome / Safari2026-01-26.14.14.57.mov2026-01-26.14.16.42.mov2026-01-26.14.18.18.mov2026-01-26.14.19.17.mov |
|
@lorretheboy |
| InteractionManager.runAfterInteractions(() => { | ||
| if (!textInputRef.current) { | ||
| return; | ||
| } | ||
| textInputRef.current.focus(); | ||
| }); |
There was a problem hiding this comment.
Can you try, please, to add this logic in then without InteractionManager.runAfterInteractions?
I suppose it should work the same
src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx
Outdated
Show resolved
Hide resolved
| // Press confirm button | ||
| fireEvent.press(screen.getByLabelText(confirmText)); | ||
|
|
||
| await waitForBatchedUpdatesWithAct(); | ||
|
|
||
| // Verify workflow actions are only called once when an approver is removed | ||
| expect(updateWorkflowDataOnApproverRemoval).toHaveBeenCalledTimes(1); | ||
| expect(removeApprovalWorkflow).toHaveBeenCalledTimes(1); |
There was a problem hiding this comment.
Why did we remove this test case?
There was a problem hiding this comment.
I'm removing this test because we've migrated the modal logic to use the new ModalProvider pattern (via showConfirmModal), the old code was written for the old modal pattern and doesn't work with the new architecture
| return options; | ||
| }; | ||
|
|
||
| const showRequiresInternetModal = () => { |
There was a problem hiding this comment.
@lorretheboy
Friendly bump
And it's React compiler issue
Could you check this instruction and update your PR if needed?
https://expensify.slack.com/archives/C08CZDJFJ77/p1768416848669629?thread_ts=1768413533.697819&cid=C08CZDJFJ77
|
But overall, changes work well! |
|
@lorretheboy |
|
✋ 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/roryabraham in version: 9.3.13-1 🚀
|
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.3.15-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.15-10 🚀
|
Explanation of Change
Fixed Issues
$ #76691
PROPOSAL: #76691 (comment)
Tests
Add approvalsAdd approvals, view a workflow details --> DeleteOffline tests
QA Steps
Same as Tests
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))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.mov
Android: mWeb Chrome
WEBSITE.ANDROID.mov
iOS: Native
IOS.mov
iOS: mWeb Safari
WEBSITE.IOS.mov
MacOS: Chrome / Safari
WorkspaceMembersPage.mov
WorkspaceMemberDetailsPage.mov
WorkspaceWorkflowsPage.mov
WorkspaceWorkflowsApprovalsEditPage.mov