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: adding tooltip to signature decoding state changes #28430

Merged
merged 46 commits into from
Nov 25, 2024

Conversation

jpuri
Copy link
Contributor

@jpuri jpuri commented Nov 13, 2024

Description

Add tooltip to state change labels.

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3628

Manual testing steps

  1. Enable signature decoding locally.
  2. Check NFT bidding or listing permit
  3. It should show appropriate tooltip

Screenshots/Recordings

Screenshot 2024-11-13 at 10 55 39 AM

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.

@jpuri jpuri added the team-confirmations Push issues to confirmations team label Nov 13, 2024
@jpuri jpuri requested review from a team as code owners November 13, 2024 05:17
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.

@metamaskbot
Copy link
Collaborator

Builds ready [6debf04]
Page Load Metrics (1825 ± 122 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint37423971660478230
domContentLoaded155022581799237114
load155824111825254122
domInteractive28233534421
backgroundConnect8196274321
firstReactRender533131346431
getState45216157
initialActions00000
loadScripts11191687131618287
setupStore583202311
uiStartup174929842123344165
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -106.79 KiB (-1.76%)
  • ui: 8.64 KiB (0.11%)
  • common: 107.31 KiB (1.30%)

Base automatically changed from native_value_display to develop November 22, 2024 00:50
@jpuri jpuri changed the title Adding tooltip to signature decoding state changes feat: adding tooltip to signature decoding state changes Nov 22, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [d969a07]
Page Load Metrics (1874 ± 95 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16642279187920398
domContentLoaded16422262185119292
load16652280187419895
domInteractive24153503416
backgroundConnect127133189
firstReactRender663061396632
getState45910126
initialActions01000
loadScripts11741711136416378
setupStore688242713
uiStartup185228172174270130
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 553 Bytes (0.01%)
  • common: 210 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [dc8f1b9]
Page Load Metrics (2048 ± 77 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint24823271947423203
domContentLoaded17562284200415072
load17862372204816077
domInteractive1798392110
backgroundConnect13129362813
firstReactRender751521102110
getState573332713
initialActions01000
loadScripts12571737148613364
setupStore76013147
uiStartup19862584227216981
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 553 Bytes (0.01%)
  • common: 210 Bytes (0.00%)

@jpuri jpuri enabled auto-merge November 22, 2024 12:42
@metamaskbot
Copy link
Collaborator

Builds ready [bc0f21f]
Page Load Metrics (1874 ± 144 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint43223641776397191
domContentLoaded154525711852291140
load155326631874300144
domInteractive22167413115
backgroundConnect10103272311
firstReactRender443141196230
getState468202110
initialActions00000
loadScripts110318981366231111
setupStore578202210
uiStartup174531632140415199
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 553 Bytes (0.01%)
  • common: 210 Bytes (0.00%)

return t('signature_decoding_list_nft_tooltip');
}
if (
stateChange.assetType === TokenStandard.ERC721 &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Q:

we check the assetType on stateChange here and the assetType on the change of the list item above. Did we want to keep this as is or update to be consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Above is filter applied to whole list of state changes, and here we are checking seet of single state change.

Copy link
Contributor

Choose a reason for hiding this comment

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

right. Thanks for confirming this is expected!

@jpuri jpuri requested a review from digiwand November 25, 2024 11:02
@jpuri jpuri added this pull request to the merge queue Nov 25, 2024
Merged via the queue into develop with commit 67b2f5a Nov 25, 2024
75 checks passed
@jpuri jpuri deleted the state_change_tooltips branch November 25, 2024 11:35
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
@metamaskbot metamaskbot added the release-12.9.0 Issue or pull request that will be included in release 12.9.0 label Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.9.0 Issue or pull request that will be included in release 12.9.0 team-confirmations Push issues to confirmations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants