fix: Revert "feat(14507): improve error message for failed txn in activity… #31137

dbrans · 2025-03-20T00:51:15Z

… details view (#29338)"

This reverts commit d510d5c.

NOTE

transaction-status-label.test.js was full of conflicts. I rewrote the tests based on what I could piece together.

Description

This change was intentional, but it turned out to be incorrect. @DDDDDanica based her PR on this ticket linked below, which refers to a Figma file. However, the missing context is that the Figma file included designs specifically for Smart Transactions.

[UX Improvement]: Transaction Failure Message Not Easily Found #14507:

Expected final result
https://www.figma.com/design/ZzVQ6iu13C67K807Z1bg5I/Smart-Transactions-1.0?node-id=4296-25303&t=XN3hVQrE3oWM0lCA-1

Related issues

Manual testing steps

Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

… details view (#29338)" This reverts commit d510d5c.

github-actions · 2025-03-20T00:51:27Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2025-03-20T01:48:38Z

Builds ready [2bedafb]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7, 8
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
- content-script: 0
- offscreen: 0

Page Load Metrics (2678 ± 1464 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1429	12854	2629	2999	1440
		domContentLoaded	1378	12663	2502	2919	1401
		load	1420	12862	2678	3048	1464
		domInteractive	24	197	55	48	23
		backgroundConnect	34	896	166	190	91
		firstReactRender	14	232	51	47	23
		getState	5	582	91	132	63
		initialActions	0	1	0	0	0
		loadScripts	989	11427	2016	2715	1304
		setupStore	7	424	56	94	45
		uiStartup	1571	16866	3550	3977	1910

Bundle size diffs [🚀 Bundle size reduced!]

background: 0 Bytes (0.00%)
ui: -1007 Bytes (-0.01%)
common: -146 Bytes (-0.00%)

httpJunkie · 2025-03-21T02:46:34Z

So this PR reverts the Banner and reimplements the tooltip feature instead?
Am I correct in assuming that the Before and after of this PR: #29338

Will be changed in favor of using the tooltip and removing the banner?

When I send a Failed Transaction after deployng failing contract on test dapp, and inspect the failed transactions. I see:

Just getting clarification that this PR is intended to remove the banner and revert this view to having the failed (with hover) functionality.

dbrans · 2025-03-21T12:09:06Z

@httpJunkie, That is correct: shouldShowTooltip was introduced in #29338 to to show or suppress the tooltip.

This PR reverts all the changes in #29338.

ui/components/app/transaction-status-label/transaction-status-label.test.js

digiwand

mostly lgtm! one request to readd

support for
'should render statusText properly when is custodyStatusDisplayText is defined' / 'should display the correct status text and tooltip'

or
'renders ${name} properly and tooltip' / 'renders pure text for status when shouldShowTooltip is specified as false'

Nice persisting the specific query tests rather than the snapshot tests for ui/components/app/transaction-status-label/transaction-status-label.test.js 👍🏼

metamaskbot · 2025-03-27T12:30:32Z

Builds ready [1c8a598]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7, 8
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
- content-script: 0
- offscreen: 0

Page Load Metrics (3281 ± 1498 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1610	15598	3106	3054	1467
		domContentLoaded	1547	14650	2854	2874	1380
		load	1623	16020	3281	3120	1498
		domInteractive	26	412	119	111	53
		backgroundConnect	106	1176	425	243	117
		firstReactRender	42	183	103	40	19
		getState	30	1221	250	244	117
		initialActions	0	1	0	0	0
		loadScripts	1140	13386	2178	2656	1275
		setupStore	15	493	187	138	66
		uiStartup	2062	18446	5806	3576	1717

Bundle size diffs [🚀 Bundle size reduced!]

background: 0 Bytes (0.00%)
ui: -1007 Bytes (-0.01%)
common: -146 Bytes (-0.00%)

digiwand · 2025-04-09T18:08:06Z

ui/components/app/transaction-list-item/transaction-list-item.component.js

@@ -372,8 +372,6 @@ function TransactionListItemInner({
          showSpeedUp={shouldShowSpeedUp}
          isEarliestNonce={isEarliestNonce}
          onCancel={cancelTransaction}
-          showCancel={isPending && !hasCancelled && !isBridgeTx}
-          showErrorBanner={Boolean(error)}


this looks like the banner message:

"This transaction would have cost you extra fees, so we stopped it. Your money is still in your wallet." for all transaction message errors detected. This may have been misleading.

digiwand · 2025-04-09T18:35:47Z

as mentioned in a Slack thread, this appears to have been cherry-picked into v12.16 cc: @dbrans

digiwand · 2025-04-11T13:45:15Z

ui/components/app/transaction-list-item-details/index.scss

@@ -64,10 +62,9 @@
  }

  &__operations {
-    margin: 0 16px 16px 16px;
+    margin: 0 0 16px 16px;


we can resurrect this to fix an outstanding, no padding issue

Revert "feat(14507): improve error message for failed txn in activity…

d1f584f

… details view (#29338)" This reverts commit d510d5c.

metamaskbot added the team-extension-platform Extension Platform team label Mar 20, 2025

nit

ba134ee

dbrans marked this pull request as ready for review March 20, 2025 00:57

dbrans changed the title ~~Revert "feat(14507): improve error message for failed txn in activity…~~ fix: Revert "feat(14507): improve error message for failed txn in activity… Mar 20, 2025

dbrans enabled auto-merge March 20, 2025 00:58

yarn verify-locales:fix

2bedafb

dbrans requested review from davidmurdoch and darkwing and removed request for davidmurdoch March 20, 2025 01:11

dbrans mentioned this pull request Mar 20, 2025

[UX Improvement]: Transaction Failure Message Not Easily Found #14507

Open

dbrans requested a review from davidmurdoch March 20, 2025 01:33

dbrans requested a review from dan437 March 21, 2025 01:11

darkwing removed their request for review March 24, 2025 15:47

digiwand reviewed Mar 25, 2025

View reviewed changes

ui/components/app/transaction-status-label/transaction-status-label.test.js Show resolved Hide resolved

digiwand requested changes Mar 25, 2025

View reviewed changes

dbrans added 2 commits March 27, 2025 07:53

test: add additional tests for TransactionStatusLabel for status tooltip

6e6fb81

Merge remote-tracking branch 'origin/main' into djb/revert-pr-29338

1c8a598

dbrans requested a review from digiwand March 27, 2025 18:59

digiwand approved these changes Mar 28, 2025

View reviewed changes

dbrans requested review from bschorchit and a team March 28, 2025 10:57

pedronfigueiredo approved these changes Apr 8, 2025

View reviewed changes

dbrans added this pull request to the merge queue Apr 8, 2025

Merged via the queue into main with commit 5f8f6b5 Apr 8, 2025
144 checks passed

dbrans deleted the djb/revert-pr-29338 branch April 8, 2025 15:34

github-actions bot locked and limited conversation to collaborators Apr 8, 2025

metamaskbot added the release-12.17.0 Issue or pull request that will be included in release 12.17.0 label Apr 8, 2025

digiwand reviewed Apr 9, 2025

View reviewed changes

digiwand reviewed Apr 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Revert "feat(14507): improve error message for failed txn in activity… #31137

fix: Revert "feat(14507): improve error message for failed txn in activity… #31137

dbrans commented Mar 20, 2025 •

edited

Loading

github-actions bot commented Mar 20, 2025

metamaskbot commented Mar 20, 2025

httpJunkie commented Mar 21, 2025 •

edited

Loading

dbrans commented Mar 21, 2025 •

edited

Loading

digiwand left a comment

metamaskbot commented Mar 27, 2025

digiwand Apr 9, 2025

digiwand commented Apr 9, 2025

digiwand Apr 11, 2025

fix: Revert "feat(14507): improve error message for failed txn in activity… #31137

fix: Revert "feat(14507): improve error message for failed txn in activity… #31137

Conversation

dbrans commented Mar 20, 2025 • edited Loading

NOTE

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Mar 20, 2025

metamaskbot commented Mar 20, 2025

httpJunkie commented Mar 21, 2025 • edited Loading

dbrans commented Mar 21, 2025 • edited Loading

digiwand left a comment

Choose a reason for hiding this comment

metamaskbot commented Mar 27, 2025

digiwand Apr 9, 2025

Choose a reason for hiding this comment

digiwand commented Apr 9, 2025

digiwand Apr 11, 2025

Choose a reason for hiding this comment

dbrans commented Mar 20, 2025 •

edited

Loading

httpJunkie commented Mar 21, 2025 •

edited

Loading

dbrans commented Mar 21, 2025 •

edited

Loading