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

fix: 🐛 use a counter to show if there are pending transactions #26116

Merged
merged 7 commits into from
Aug 1, 2024

Conversation

matteoscurati
Copy link
Contributor

@matteoscurati matteoscurati commented Jul 25, 2024

Description

This PR modifies the content of the badge for pending transactions. Before this PR, the blue badge showed three dots; now it shows a number again.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Screenshot 2024-07-25 at 10 16 28

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.

@matteoscurati matteoscurati added the team-notifications Notifications team label Jul 25, 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.

@matteoscurati
Copy link
Contributor Author

@metamaskbot
Copy link
Collaborator

Builds ready [08c707d]
Page Load Metrics (179 ± 219 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint811861132613
domContentLoaded106927157
load432164179456219
domInteractive106927157
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -4 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link

codecov bot commented Jul 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 69.95%. Comparing base (ed28508) to head (7e6b5d7).
Report is 1 commits behind head on develop.

Files Patch % Lines
app/scripts/background.js 0.00% 5 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #26116   +/-   ##
========================================
  Coverage    69.95%   69.95%           
========================================
  Files         1411     1411           
  Lines        49982    49981    -1     
  Branches     13805    13804    -1     
========================================
  Hits         34964    34964           
+ Misses       15018    15017    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matteoscurati matteoscurati marked this pull request as ready for review July 25, 2024 12:53
@matteoscurati matteoscurati requested a review from a team as a code owner July 25, 2024 12:53
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR updates the badge for pending transactions to display the number of pending transactions instead of an ellipsis, enhancing user experience by providing specific information.

  • File Modified: app/scripts/background.js
  • Key Changes:
    • Removed BADGE_LABEL_APPROVAL constant.
    • Updated updateBadge function to set the badge label to pendingApprovalCount.

This change ensures users can see the exact number of pending transactions, improving transparency and usability.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@metamaskbot
Copy link
Collaborator

Builds ready [6e0d269]
Page Load Metrics (241 ± 233 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint82150111189
domContentLoaded106831168
load551712241484233
domInteractive106831168
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -4 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR updates the badge for pending transactions to display the number of pending transactions instead of an ellipsis, enhancing user experience by providing specific information.

  • File Modified: ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.container.js

    • Added state mapping for unapproved transactions and their counts.
    • Introduced a counter to track the number of pending transactions.
  • File Added: ui/pages/confirmations/confirmation/components/queued-requests-banner-alert/queued-requests-banner-alert.tsx

    • Introduced QueuedRequestsBannerAlert component to display a banner alert for queued requests.
  • File Modified: ui/pages/confirmations/components/signature-request/signature-request.js

    • Added QueuedRequestsBannerAlert component to the SignatureRequest component.
  • File Added: ui/pages/confirmations/hooks/alerts/transactions/useQueuedConfirmationsAlerts.ts

    • Introduced useQueuedConfirmationsAlerts hook to generate alerts for queued confirmations.
  • File Modified: ui/pages/confirmations/hooks/useConfirmationAlerts.ts

    • Integrated useQueuedConfirmationsAlerts into useTransactionAlerts function.

These changes ensure users can see the exact number of pending transactions, improving transparency and usability.

78 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

app/scripts/background.js Outdated Show resolved Hide resolved
@metamaskbot
Copy link
Collaborator

Builds ready [7a83fcb]
Page Load Metrics (277 ± 287 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint641881223115
domContentLoaded96333157
load402440277597287
domInteractive96333157
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 23 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [634bec5]
Page Load Metrics (274 ± 271 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6589216717685
domContentLoaded10123423416
load412182274565271
domInteractive10123423416
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 23 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link

sonarqubecloud bot commented Aug 1, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [7e6b5d7]
Page Load Metrics (266 ± 286 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6076914515373
domContentLoaded9176324119
load402363266595286
domInteractive9176324119
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 23 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@Prithpal-Sooriya Prithpal-Sooriya merged commit 82ad9c1 into develop Aug 1, 2024
75 checks passed
@Prithpal-Sooriya Prithpal-Sooriya deleted the fix/use-counter-pending-transactions branch August 1, 2024 11:06
@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2024
@metamaskbot metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Aug 1, 2024
@gauthierpetetin gauthierpetetin added release-12.3.0 Issue or pull request that will be included in release 12.3.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.0 labels Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.3.0 Issue or pull request that will be included in release 12.3.0 team-notifications Notifications team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants