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

… view (#29338)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
When we encounter a failed transaction:
- in activity list from home view, it renders as `failed`, hovering the
status will render an error message
- when clicking this activity, the popup view for txn details will
render `failed` as well, with no hover effect
- In the meanwhile, there's a new and more user friendly error banner
showing for user when txn failed.
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29338?quickstart=1)

## **Related issues**

Fixes: #14507
Figma:
https://www.figma.com/design/ZzVQ6iu13C67K807Z1bg5I/Smart-Transactions-1.0?node-id=4296-25303&t=ff749RbiH6F4IUqk-0

## **Manual testing steps**

1. Render extension and test dapp
2. Trigger a failed txn from test dapp
3. Check activity for that txn
4. Check activity details by clicking item from step 3 and validate the
banner, as well as hover

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
<img width="355" alt="Screenshot 2024-12-18 at 18 25 45"
src="https://github.com/user-attachments/assets/76e53d88-a171-449a-a046-803472a66a02"
/>

<!-- [screenshots/recordings] -->

### **After**
<img width="356" alt="Screenshot 2024-12-19 at 01 54 39"
src="https://github.com/user-attachments/assets/a0ce1088-f9a2-4a3a-98ab-25cdd36faa25"
/>


<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.

test/e2e/tests/dapp-interactions/failing-contract.spec.js

@@ -72,6 +72,15 @@ describe('Failing contract interaction ', function () {
           css: '.activity-list-item .transaction-status-label',
           text: 'Failed',
         });
+        // inspect transaction details
+        await driver.clickElement({
+          css: '.activity-list-item .transaction-status-label',
+          text: 'Failed',
+        });
+        await driver.waitForSelector('.transaction-list-item-details');
+        await driver.waitForSelector(
+          '[data-testid="transaction-list-item-details-banner-error-message"]',
+        );
       },
     );
   });

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

@@ -49,6 +49,8 @@
     display: flex;
     flex-direction: column;
     align-items: flex-end;
+    height: 42px;
+    justify-content: space-between;
 
     .btn-link {
       font-size: 12px;
@@ -62,9 +64,10 @@
   }
 
   &__operations {
-    margin: 0 0 16px 16px;
+    margin: 0 16px 16px 16px;
     display: flex;
-    justify-content: flex-end;
+    justify-content: space-between;
+    align-items: center;
   }
 
   &__header {

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

@@ -13,9 +13,19 @@ import Tooltip from '../../ui/tooltip';
 import CancelButton from '../cancel-button';
 import Popover from '../../ui/popover';
 import { Box } from '../../component-library/box';
+import { Text } from '../../component-library/text';
+import {
+  BannerAlert,
+  BannerAlertSeverity,
+} from '../../component-library/banner-alert';
+import {
+  TextVariant,
+  ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
+  IconColor,
+  ///: END:ONLY_INCLUDE_IF
+} from '../../../helpers/constants/design-system';
 ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
-import { Icon, IconName, Text } from '../../component-library';
-import { IconColor } from '../../../helpers/constants/design-system';
+import { Icon, IconName } from '../../component-library';
 ///: END:ONLY_INCLUDE_IF
 import { SECOND } from '../../../../shared/constants/time';
 import { MetaMetricsEventCategory } from '../../../../shared/constants/metametrics';
@@ -55,6 +65,7 @@ export default class TransactionListItemDetails extends PureComponent {
     recipientNickname: PropTypes.string,
     transactionStatus: PropTypes.func,
     isCustomNetwork: PropTypes.bool,
+    showErrorBanner: PropTypes.bool,
     history: PropTypes.object,
     blockExplorerLinkText: PropTypes.object,
     ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
@@ -204,6 +215,7 @@ export default class TransactionListItemDetails extends PureComponent {
       onClose,
       recipientNickname,
       showCancel,
+      showErrorBanner,
       transactionStatus: TransactionStatus,
       blockExplorerLinkText,
       ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
@@ -220,6 +232,17 @@ export default class TransactionListItemDetails extends PureComponent {
       <Popover title={title} onClose={onClose}>
         <div className="transaction-list-item-details">
           <div className="transaction-list-item-details__operations">
+            {showErrorBanner && (
+              <BannerAlert severity={BannerAlertSeverity.Warning}>
+                <Text
+                  variant={TextVariant.bodyMd}
+                  as="h6"
+                  data-testid="transaction-list-item-details-banner-error-message"
+                >
+                  {t('transactionFailedBannerMessage')}
+                </Text>
+              </BannerAlert>
+            )}
             <div className="transaction-list-item-details__header-buttons">
               {showSpeedUp && (
                 <Button
@@ -246,7 +269,7 @@ export default class TransactionListItemDetails extends PureComponent {
                     className="transaction-list-item-details__header-button"
                     data-testid="rety-button"
                   >
-                    <i className="fa fa-sync"></i>
+                    <i className="fa fa-sync" />
                   </Button>
                 </Tooltip>
               )}
@@ -258,22 +281,18 @@ export default class TransactionListItemDetails extends PureComponent {
               data-testid="transaction-list-item-details-tx-status"
             >
               <div>{t('status')}</div>
-              <div>
-                <TransactionStatus />
-              </div>
+              <TransactionStatus />
             </div>
             <div className="transaction-list-item-details__tx-hash">
-              <div>
-                <Button
-                  type="link"
-                  onClick={this.handleBlockExplorerClick}
-                  disabled={!hash}
-                >
-                  {blockExplorerLinkText.firstPart === 'addBlockExplorer'
-                    ? t('addBlockExplorer')
-                    : t('viewOnBlockExplorer')}
-                </Button>
-              </div>
+              <Button
+                type="link"
+                onClick={this.handleBlockExplorerClick}
+                disabled={!hash}
+              >
+                {blockExplorerLinkText.firstPart === 'addBlockExplorer'
+                  ? t('addBlockExplorer')
+                  : t('viewOnBlockExplorer')}
+              </Button>
               <div>
                 <Tooltip
                   wrapperClassName="transaction-list-item-details__header-button"

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

@@ -1,7 +1,7 @@
 import React from 'react';
 import configureMockStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
-import { act, waitFor } from '@testing-library/react';
+import { waitFor } from '@testing-library/react';
 import { TransactionStatus } from '@metamask/transaction-controller';
 import { GAS_LIMITS } from '../../../../shared/constants/gas';
 import { renderWithProvider } from '../../../../test/lib/render-helpers';
@@ -54,7 +54,7 @@ const transactionGroup = {
   hasCancelled: false,
 };
 
-const render = async (overrideProps) => {
+const render = (overrideProps) => {
   const rpcPrefs = {
     blockExplorerUrl: 'https://customblockexplorer.com/',
   };
@@ -71,67 +71,65 @@ const render = async (overrideProps) => {
     senderAddress: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc',
     tryReverseResolveAddress: jest.fn(),
     transactionGroup,
-    transactionStatus: () => <div></div>,
+    transactionStatus: () => <div />,
     blockExplorerLinkText,
     rpcPrefs,
     ...overrideProps,
   };
 
   const mockStore = configureMockStore([thunk])(mockState);
 
-  let result;
-
-  await act(
-    async () =>
-      (result = renderWithProvider(
-        <TransactionListItemDetails {...props} />,
-        mockStore,
-      )),
+  const result = renderWithProvider(
+    <TransactionListItemDetails {...props} />,
+    mockStore,
   );
 
   return result;
 };
 
 describe('TransactionListItemDetails Component', () => {
-  it('should render title with title prop', async () => {
-    const { queryByText } = await render();
+  describe('matches snapshot', () => {
+    it('for non-error details', async () => {
+      const { queryByText, queryByTestId } = render();
+      expect(queryByText('Test Transaction Details')).toBeInTheDocument();
+      expect(
+        queryByTestId('transaction-list-item-details-banner-error-message'),
+      ).not.toBeInTheDocument();
+    });
 
-    await waitFor(() => {
+    it('for error details', async () => {
+      const { queryByText, queryByTestId } = render({ showErrorBanner: true });
       expect(queryByText('Test Transaction Details')).toBeInTheDocument();
+      expect(
+        queryByTestId('transaction-list-item-details-banner-error-message'),
+      ).toBeInTheDocument();
     });
   });
 
-  describe('Retry button', () => {
-    it('should render retry button with showRetry prop', async () => {
-      const { queryByTestId } = await render({ showRetry: true });
-
+  describe('Action buttons', () => {
+    it('renders retry button with showRetry prop', async () => {
+      const { queryByTestId } = render({ showRetry: true });
       expect(queryByTestId('rety-button')).toBeInTheDocument();
     });
-  });
-
-  describe('Cancel button', () => {
-    it('should render cancel button with showCancel prop', async () => {
-      const { queryByTestId } = await render({ showCancel: true });
 
+    it('renders cancel button with showCancel prop', async () => {
+      const { queryByTestId } = render({ showCancel: true });
       expect(queryByTestId('cancel-button')).toBeInTheDocument();
     });
-  });
-
-  describe('Speedup button', () => {
-    it('should render speedup button with showSpeedUp prop', async () => {
-      const { queryByTestId } = await render({ showSpeedUp: true });
 
+    it('renders speedup button with showSpeedUp prop', async () => {
+      const { queryByTestId } = render({ showSpeedUp: true });
       expect(queryByTestId('speedup-button')).toBeInTheDocument();
     });
   });
 
   describe('Institutional', () => {
-    it('should render correctly if custodyTransactionDeepLink has a url', async () => {
+    it('renders correctly if custodyTransactionDeepLink has a url', async () => {
       mockGetCustodianTransactionDeepLink = jest
         .fn()
         .mockReturnValue({ url: 'https://url.com' });
 
-      await render({ showCancel: true });
+      render({ showCancel: true });
 
       await waitFor(() => {
         const custodianViewButton = document.querySelector(
@@ -143,7 +141,7 @@ describe('TransactionListItemDetails Component', () => {
       });
     });
 
-    it('should render correctly if transactionNote is provided', async () => {
+    it('renders correctly if transactionNote is provided', async () => {
       const newTransaction = {
         ...transaction,
         metadata: {
@@ -159,13 +157,11 @@ describe('TransactionListItemDetails Component', () => {
         initialTransaction: newTransaction,
       };
 
-      const { queryByText } = await render({
+      const { queryByText } = render({
         transactionGroup: newTransactionGroup,
       });
 
-      await waitFor(() => {
-        expect(queryByText('some note')).toBeInTheDocument();
-      });
+      expect(queryByText('some note')).toBeInTheDocument();
     });
   });
 });

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

@@ -125,6 +125,7 @@ export default function SmartTransactionListItem({
               date={date}
               status={displayedStatusKey}
               statusOnly
+              shouldShowTooltip={false}
             />
           )}
         />

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

@@ -459,6 +459,7 @@ function TransactionListItemInner({
             !hasCancelled &&
             !isBridgeTx
           }
+          showErrorBanner={Boolean(error)}
           transactionStatus={() => (
             <TransactionStatusLabel
               isPending={isPending}
@@ -467,6 +468,7 @@ function TransactionListItemInner({
               date={date}
               status={displayedStatusKey}
               statusOnly
+              shouldShowTooltip={false}
               ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
               custodyStatus={transactionGroup.primaryTransaction.custodyStatus}
               custodyStatusDisplayText={

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

@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`TransactionStatusLabel Component should render CONFIRMED properly 1`] = `
+exports[`TransactionStatusLabel Component renders CONFIRMED properly and tooltip 1`] = `
 <div>
   <div
     class="transaction-status-label transaction-status-label--confirmed"
@@ -10,7 +10,7 @@ exports[`TransactionStatusLabel Component should render CONFIRMED properly 1`] =
 </div>
 `;
 
-exports[`TransactionStatusLabel Component should render PENDING properly 1`] = `
+exports[`TransactionStatusLabel Component renders PENDING properly and tooltip 1`] = `
 <div>
   <div
     class="transaction-status-label transaction-status-label--pending transaction-status-label--pending"
@@ -20,7 +20,7 @@ exports[`TransactionStatusLabel Component should render PENDING properly 1`] = `
 </div>
 `;
 
-exports[`TransactionStatusLabel Component should render QUEUED properly 1`] = `
+exports[`TransactionStatusLabel Component renders QUEUED properly and tooltip 1`] = `
 <div>
   <div
     class="transaction-status-label transaction-status-label--queued transaction-status-label--queued"
@@ -30,7 +30,7 @@ exports[`TransactionStatusLabel Component should render QUEUED properly 1`] = `
 </div>
 `;
 
-exports[`TransactionStatusLabel Component should render SIGNING if status is approved 1`] = `
+exports[`TransactionStatusLabel Component renders SIGNING properly and tooltip 1`] = `
 <div>
   <div
     class="transaction-status-label transaction-status-label--signing"
@@ -40,7 +40,7 @@ exports[`TransactionStatusLabel Component should render SIGNING if status is app
 </div>
 `;
 
-exports[`TransactionStatusLabel Component should render UNAPPROVED properly 1`] = `
+exports[`TransactionStatusLabel Component renders UNAPPROVED properly and tooltip 1`] = `
 <div>
   <div
     class="transaction-status-label transaction-status-label--unapproved transaction-status-label--unapproved"