Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(14507): improve error message for failed txn in activity details view #29338

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

DDDDDanica
Copy link
Contributor

@DDDDDanica DDDDDanica commented Dec 19, 2024

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.

Open in GitHub Codespaces

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

Before

Screenshot 2024-12-18 at 18 25 45

After

Screenshot 2024-12-19 at 01 54 39

Pre-merge author checklist

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.

@DDDDDanica DDDDDanica added team-extension-platform team-wallet-ux area-design Design bug (previously known as papercuts - ask Hilary for more detail) labels Dec 19, 2024
@DDDDDanica DDDDDanica self-assigned this Dec 19, 2024
Copy link
Contributor

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.

@@ -258,22 +279,18 @@ export default class TransactionListItemDetails extends PureComponent {
data-testid="transaction-list-item-details-tx-status"
>
<div>{t('status')}</div>
<div>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clean up
Screenshot 2024-12-18 at 22 31 27

<TransactionListItemDetails {...props} />,
mockStore,
)),
const result = renderWithProvider(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no asynchronous render here, can be simplified

@@ -96,20 +97,29 @@ export default function TransactionStatusLabel({
}
}
///: END:ONLY_INCLUDE_IF

return (
return shouldShowTooltip ? (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keep the tooltip for same component here
Screenshot 2024-12-18 at 18 24 09

display: flex;
justify-content: flex-end;
justify-content: space-between;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When there's a retry button, we need to handle the alignment as well
Screenshot 2024-12-19 at 03 29 53

@DDDDDanica DDDDDanica force-pushed the feature/14507-txn-msg branch 2 times, most recently from 66c9bb0 to 0ad4d1b Compare December 19, 2024 04:08
@metamaskbot
Copy link
Collaborator

Builds ready [0ad4d1b]
Page Load Metrics (1716 ± 74 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14572057171415273
domContentLoaded14491994169214670
load14582066171615474
domInteractive21107462411
backgroundConnect990292411
firstReactRender16101532713
getState55313136
initialActions01000
loadScripts10321574127012962
setupStore669212211
uiStartup162827652048248119
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 985 Bytes (0.01%)
  • common: 146 Bytes (0.00%)

@pedronfigueiredo
Copy link
Contributor

Looks good, but would it be worth adding an e2e test?

@DDDDDanica DDDDDanica force-pushed the feature/14507-txn-msg branch from 0ad4d1b to 43c3310 Compare December 20, 2024 14:13
@DDDDDanica DDDDDanica force-pushed the feature/14507-txn-msg branch from 43c3310 to 9212499 Compare December 20, 2024 14:14
Copy link
Contributor

@dbrans dbrans left a comment

Choose a reason for hiding this comment

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

Nice work, Danica! Looks like a big improvement.

app/_locales/en/messages.json Show resolved Hide resolved
@DDDDDanica DDDDDanica enabled auto-merge December 20, 2024 14:51
@metamaskbot
Copy link
Collaborator

Builds ready [9212499]
Page Load Metrics (1691 ± 53 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14651991169311656
domContentLoaded14571928166810350
load14651933169111053
domInteractive27119523215
backgroundConnect86226209
firstReactRender1795512914
getState476232612
initialActions00000
loadScripts1060154812599847
setupStore65915168
uiStartup169025862016240115
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 985 Bytes (0.01%)
  • common: 146 Bytes (0.00%)

@DDDDDanica DDDDDanica added this pull request to the merge queue Dec 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 20, 2024
@DDDDDanica DDDDDanica added this pull request to the merge queue Dec 20, 2024
Merged via the queue into main with commit d510d5c Dec 20, 2024
77 checks passed
@DDDDDanica DDDDDanica deleted the feature/14507-txn-msg branch December 20, 2024 16:21
@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2024
@metamaskbot metamaskbot added the release-12.11.0 Issue or pull request that will be included in release 12.11.0 label Dec 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-design Design bug (previously known as papercuts - ask Hilary for more detail) release-12.11.0 Issue or pull request that will be included in release 12.11.0 team-extension-platform team-wallet-ux
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[UX Improvement]: Transaction Failure Message Not Easily Found
4 participants