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: Fix send flow max value issue #29960

Conversation

OGPoyraz
Copy link
Member

@OGPoyraz OGPoyraz commented Jan 29, 2025

Description

This PR aims to fix send max value issue for redesigned confirmations.

Open in GitHub Codespaces

Related issues

Fixes: #29903

Manual testing steps

  1. Initiate a native send flow from wallet
  2. Click max in the amount picker
  3. Go to next step (confirmation)
  4. See that value changes when gas is changed

Screenshots/Recordings

SendFlowMaxAmount.mov

Before

After

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.

@metamaskbot
Copy link
Collaborator

Builds ready [ce4e74f]
Page Load Metrics (1667 ± 73 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14692092167215775
domContentLoaded14612076165115072
load14712093166715373
domInteractive2497452411
backgroundConnect97419178
firstReactRender1597422713
getState45414157
initialActions01000
loadScripts10271554120113465
setupStore780222210
uiStartup170427971930238114
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 2.37 KiB (0.03%)
  • common: 106 Bytes (0.00%)

@sleepytanya
Copy link
Contributor

sleepytanya commented Jan 29, 2025

Max amount is adjusted in response to Gas settings / fluctuations and the Max amount transaction can be successfully sent:

max_amount_gas.mov
gas_settings.mov

@OGPoyraz OGPoyraz added this pull request to the merge queue Jan 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 29, 2025
@OGPoyraz OGPoyraz added this pull request to the merge queue Jan 29, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 29, 2025
<!--
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**

<!--
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?
-->

This PR aims to fix send max value issue for redesigned confirmations.

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

## **Related issues**

Fixes: #29903

## **Manual testing steps**

1. Initiate a native send flow from wallet
2. Click max in the amount picker
3. Go to next step (confirmation)
4. See that value changes when gas is changed

## **Screenshots/Recordings**

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



https://github.com/user-attachments/assets/14a85e29-b7d5-4bfe-b013-25f1954103cc



### **Before**

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

### **After**

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

## **Pre-merge author checklist**

- [X] 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).
- [X] I've completed the PR template to the best of my ability
- [X] I’ve included tests if applicable
- [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [X] 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.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 29, 2025
…ceeds-available-funds-no-warning-is-displayed-to-alert-the-user-max-amount-is-not-adjusted-in-response-to-gas-fees-changes
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.

…ceeds-available-funds-no-warning-is-displayed-to-alert-the-user-max-amount-is-not-adjusted-in-response-to-gas-fees-changes
@OGPoyraz OGPoyraz enabled auto-merge January 29, 2025 22:58
@OGPoyraz OGPoyraz added this pull request to the merge queue Jan 29, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [ee78204]
Page Load Metrics (1853 ± 151 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint25329421704583280
domContentLoaded146728011825310149
load151028471853315151
domInteractive24240554823
backgroundConnect974302010
firstReactRender1599492914
getState469222211
initialActions00000
loadScripts100521621345264127
setupStore87418189
uiStartup168837412203493237
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 2.37 KiB (0.03%)
  • common: 106 Bytes (0.00%)

Merged via the queue into main with commit 180641b Jan 29, 2025
70 checks passed
@OGPoyraz OGPoyraz deleted the 29903-bug-bug-when-the-transaction-amount-exceeds-available-funds-no-warning-is-displayed-to-alert-the-user-max-amount-is-not-adjusted-in-response-to-gas-fees-changes branch January 29, 2025 23:59
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2025
@metamaskbot metamaskbot added the release-12.12.0 Issue or pull request that will be included in release 12.12.0 label Jan 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
QA Passed release-12.12.0 Issue or pull request that will be included in release 12.12.0 team-confirmations Push issues to confirmations team
Projects
None yet
6 participants