Skip to content

fix(mutation-dialog): show the result when a mutation resolves fast - #518

Open
yhabib wants to merge 5 commits into
mainfrom
fix/mutation-dialog-fast-result
Open

yhabib wants to merge 5 commits into
mainfrom
fix/mutation-dialog-fast-result

Conversation

@yhabib

@yhabib yhabib commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

On the icp-cli local network with no artificial delay, a mutation can resolve within ~200 ms. If it resolves while the form still fades out, AnimatePresence (mode="wait") mounts the processing content as already exiting and never swaps in the result. The dialog keeps "Sending 5 ICP to Wallet A ..." on screen after the transfer went through, and the auto close then hides it. #498 set artificial-delay-ms: 1000 on the e2e network to keep CI stable, which hides the bug.

With the e2e delay at 0 and a temporary spec that throttles the CPU 8x (Emulation.setCPUThrottlingRate) on "Save an address from the send review step", the send or the add address dialog got stuck in 1 to 4 of 8 runs. With the fix, 16 of 16 runs passed at 8x. At 0 delay, the chromium suite passed, except "Modal Opening & URL State", which also fails locally on main.

Changes

  • Added a visible phase to MutationDialog, so a success, an error, or a Ledger cancel waits until the processing content mounts.
  • Switched the success auto close to start when the success content mounts, not when the phase changes.
  • Switched the close button and the navigation blocker to the visible phase, so they match the screen.
  • Added unit tests for a mutation that ends as the form leaves, and for the start of the auto close.

If the mutation resolves while the body still fades out, AnimatePresence
("wait" mode) mounts the processing content as already exiting. It never
removes that content, so the dialog keeps "Sending ..." on screen. The
result now waits until the processing content ends its enter animation.

The success auto close now starts when the success content shows, not
when the phase changes.
@yhabib
yhabib requested a review from a team as a code owner October 17, 2026 00:09
Copilot AI balanced review requested due to automatic review settings October 17, 2026 00:09
@zeropath-ai

zeropath-ai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

✅ No security or compliance issues detected. Reviewed everything up to 8db96aa.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► src/governance-app-frontend/src/common/components/MutationDialog.tsx
     Introduce processingPending, successMounted state handling and visiblePhase logic to coordinate UI transitions
► src/governance-app-frontend/src/common/components/MutationDialog.tsx
     Adjust rendering conditions for idle/processing/success/error phases and add onMount hooks for PhaseContainer
► src/governance-app-frontend/src/common/components/MutationDialog.tsx
     Modify PhaseContainer to support onMount behavior and trigger it on mount
Enhancement ► src/governance-app-frontend/src/common/components/MutationDialog.test.tsx
     Add tests with EndOnFormExit wrapper and additional scenarios to verify auto-close behavior and end-of-mutation messaging
► src/governance-app-frontend/src/common/components/MutationDialog.tsx
     Import usePresence and adjust mocks in tests

@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

📊 Build Bundle Stats

The latest build generated the following assets:

dist/index.html                                           3.80 kB │ gzip:   1.12 kB
dist/assets/index-sWM1adin.css                          145.75 kB │ gzip:  23.43 kB
dist/assets/icpLedger-gAqLrUtW.js                         0.23 kB │ gzip:   0.21 kB
dist/assets/DeviceActionDetail-C_TWIgfp.js                0.33 kB │ gzip:   0.25 kB
dist/assets/Skeleton-CcYZc4b0.js                          0.44 kB │ gzip:   0.32 kB
dist/assets/webhid-D0yio6wG.js                            0.53 kB │ gzip:   0.32 kB
dist/assets/useTvlValue-DFvED89_.js                       0.60 kB │ gzip:   0.41 kB
dist/assets/nnsDapp-CkNZLJAk.js                           0.68 kB │ gzip:   0.25 kB
dist/assets/SkeletonPageHeader-DYsc7_LK.js                0.76 kB │ gzip:   0.47 kB
dist/assets/PageHeader-jXzaR6sL.js                        0.77 kB │ gzip:   0.45 kB
dist/assets/CertifiedBadge-CmjX0IC7.js                    0.81 kB │ gzip:   0.48 kB
dist/assets/Separator-BT4lAqmc.js                         0.83 kB │ gzip:   0.50 kB
dist/assets/SkeletonStatCard-8fmuyo79.js                  0.91 kB │ gzip:   0.56 kB
dist/assets/useForecastPeriod-BaADnZUF.js                 0.98 kB │ gzip:   0.60 kB
dist/assets/utils-DV2_eYTd.js                             1.02 kB │ gzip:   0.59 kB
dist/assets/useIcpIndex-DvdDKAVE.js                       1.09 kB │ gzip:   0.64 kB
dist/assets/numbers-6-1kvwvM.js                           1.15 kB │ gzip:   0.57 kB
dist/assets/routeLoaders-BtlLuCEE.js                      1.20 kB │ gzip:   0.56 kB
dist/assets/rolldown-runtime-DAXXjFlN.js                  1.26 kB │ gzip:   0.70 kB
dist/assets/advancedFeatures-CjMuptZI.js                  1.50 kB │ gzip:   0.74 kB
dist/assets/events-BCw7XNZ4.js                            1.66 kB │ gzip:   0.86 kB
dist/assets/useLedgerNeuronCall-CurSeIrA.js               1.88 kB │ gzip:   1.03 kB
dist/assets/errors-C_oENL0S.js                            1.95 kB │ gzip:   0.97 kB
dist/assets/AnimatedNumber-B5oSORAh.js                    1.98 kB │ gzip:   1.12 kB
dist/assets/useGovernanceAppCanister-B7Y7lUYu.js          2.08 kB │ gzip:   1.00 kB
dist/assets/useCommandPaletteSettings-6FmRH2PZ.js         2.13 kB │ gzip:   1.06 kB
dist/assets/addressBook-DBgUBwEf.js                       2.14 kB │ gzip:   1.20 kB
dist/assets/useIcpIndexTransactionsPolling-DEuc2vgZ.js    2.15 kB │ gzip:   1.15 kB
dist/assets/governance-pLtTY4x0.js                        2.41 kB │ gzip:   1.05 kB
dist/assets/useShortcutSettings-DhsnN1_l.js               2.45 kB │ gzip:   1.20 kB
dist/assets/useAddHotkey-Dva-iR6O.js                      2.60 kB │ gzip:   1.34 kB
dist/assets/useClaimPendingDeviceStakes-CbqsQ_ER.js       3.12 kB │ gzip:   1.42 kB
dist/assets/SkeletonText-CSYORdC6.js                      3.71 kB │ gzip:   1.29 kB
dist/assets/ToggleGroup-Dc9nPNk3.js                       4.72 kB │ gzip:   1.76 kB
dist/assets/useAccounts-B2gukUAx.js                       4.96 kB │ gzip:   2.20 kB
dist/assets/tickers-CHl8ZnIx.js                           6.04 kB │ gzip:   2.64 kB
dist/assets/useNowInSeconds-DqCXVFLB.js                   7.12 kB │ gzip:   3.04 kB
dist/assets/QueryStates-CRej43x9.js                       7.27 kB │ gzip:   2.70 kB
dist/assets/routes-BeUi8z9w.js                            7.95 kB │ gzip:   2.70 kB
dist/assets/TopicFollowingAccordion-BKDBCBJm.js           8.54 kB │ gzip:   3.41 kB
dist/assets/AlertDialog-BVvIa9uP.js                       8.85 kB │ gzip:   3.10 kB
dist/assets/ledgerIdentity-Db47a4SI.js                   10.07 kB │ gzip:   3.66 kB
dist/assets/spamFilter-ShjK3Gcv.js                       11.75 kB │ gzip:   5.02 kB
dist/assets/MutationDialog-CLdtTNys.js                   12.33 kB │ gzip:   4.26 kB
dist/assets/ResponsiveDialog-BFIdXQaJ.js                 14.38 kB │ gzip:   4.05 kB
dist/assets/_id-D_G12wFu.js                              14.71 kB │ gzip:   4.55 kB
dist/assets/AmountInput-Cr5nn4Yz.js                      16.84 kB │ gzip:   6.72 kB
dist/assets/_auth-CxN76ec1.js                            21.30 kB │ gzip:   7.89 kB
dist/assets/HideBalancesToggle-ZAX1qGYq.js               22.98 kB │ gzip:   8.57 kB
dist/assets/vendor-buffer-EX-AqChq.js                    26.95 kB │ gzip:   8.18 kB
dist/assets/accounts-CtHXXod6.js                         31.57 kB │ gzip:  10.76 kB
dist/assets/SendICPButton-7Yr9LWuj.js                    37.04 kB │ gzip:  13.50 kB
dist/assets/settings-CmCckbu5.js                         54.34 kB │ gzip:  15.54 kB
dist/assets/dashboard-DLned68E.js                        54.47 kB │ gzip:  17.81 kB
dist/assets/index-Cm_O1ZUw.js                            62.63 kB │ gzip:  17.63 kB
dist/assets/voting-BSHJ8ege.js                           69.23 kB │ gzip:  21.65 kB
dist/assets/vendor-md-BD-QpKtL.js                        73.29 kB │ gzip:  19.98 kB
dist/assets/button-BJYsNBe5.js                           82.07 kB │ gzip:  27.30 kB
dist/assets/neurons-Cqsqw0hx.js                          88.47 kB │ gzip:  26.71 kB
dist/assets/DisburseModal-pDKZnmIB.js                   100.88 kB │ gzip:  29.59 kB
dist/assets/vendor-tanstack-DJFQoBV9.js                 119.95 kB │ gzip:  39.21 kB
dist/assets/vendor-jsqr-CjiBYtfG.js                     129.98 kB │ gzip:  47.50 kB
dist/assets/vendor-core-react-bRRjyX41.js               189.81 kB │ gzip:  59.76 kB
dist/assets/vendor-recharts-DnwnQTPO.js                 280.25 kB │ gzip:  76.83 kB
dist/assets/vendor-ledger-Cxxd5WAf.js                   403.26 kB │ gzip:  98.84 kB
dist/assets/vendor-icp-Dddsnj85.js                      405.70 kB │ gzip: 102.55 kB
dist/assets/vendor-libs-BEUkcmr_.js                     581.20 kB │ gzip: 187.04 kB

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Responsive layout changes can skip animation callbacks, leaving completed mutations stuck on processing or preventing success auto-close.

1 open issue
What changed in this PR

Fixes shared mutation dialogs that fail to display results when mutations complete during transitions.

Changes:

  • Gates result rendering on the processing entrance animation.
  • Starts success auto-close after the success animation completes.
  • Uses the visible phase for dismissal and navigation blocking.
File Description
src/​governance-app-frontend/​src/​common/​components/​MutationDialog.tsx Coordinates visible phases, animation completion, and auto-close timing.

Review effort: Balanced

A remount at the 768px breakpoint (Dialog to Drawer) skips the enter
animation. Then onAnimationComplete never fires, and the dialog stays on
processing after the mutation ends. The mount of the phase content now
sets the flags.
Copilot AI previously approved these changes Oct 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approved

The focused changes address the transition race and responsive remount handling with no identified blocking issues.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

…ast-result

# Conflicts:
#	src/governance-app-frontend/src/common/components/MutationDialog.tsx
@yhabib
yhabib requested a balanced review from Copilot October 28, 2026 14:32
Copilot AI dismissed their stale review, a newer Copilot review was requested October 28, 2026 14:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A committed regression test is missing for mutations that settle before the form’s exit animation completes, covering result replacement and success auto-close timing.

1 open issue

Review effort: Balanced

The mutation ends in the same task as the form leaves, so the result and
the swap to the processing content land in one update. The tests also
check that the auto close starts when the success content mounts.
Copilot AI previously approved these changes Oct 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approved

No active review issues remain for the mutation dialog transition and auto-close changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

…ast-result

# Conflicts:
#	src/governance-app-frontend/src/common/components/MutationDialog.tsx
@yhabib
yhabib requested a balanced review from Copilot October 29, 2026 09:36
Copilot AI dismissed their stale review, a newer Copilot review was requested October 29, 2026 09:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approved

No active review issues were identified in the mutation-dialog phase handling and auto-close timing changes.

No open issues 🎉

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants