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.

Sign up for GitHub

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

Jump to bottom

feat: Redesign Signature Decoding Simulation #12994

Merged
merged 86 commits into from
Jan 16, 2025
Merged

feat: Redesign Signature Decoding Simulation #12994

merged 86 commits into from
Jan 16, 2025
+873 −76

Conversation

digiwand
Copy link
Contributor

@digiwand digiwand commented Jan 15, 2025 •
edited
Loading

Description

Add Signature Decoding Simulation UI

This logic mirrors the extension with modifications due to mobile/extension parity differences.

In addition:

  • updated ValueDisplay value logic to no longer use useMemo

Todo in follow-up PRs:

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3876
Related: #12994 (Replaces Ramp/Box usages with View)

Manual testing steps

  1. Set REDESIGNED_SIGNATURE_REQUEST to true in js.env
  2. Enable confirmation_redesign in Launch Darkly
  3. Turn on Improved Signatures setting
  4. Turn on Simulation setting
  5. Test various v3 and v4 signTypedData signatures

Screenshots/Recordings

Before

After

The values in this screenshot will be replaced by "Unlimited"

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.

digiwand added 30 commits December 6, 2024 21:53
@digiwand
feat: Permit Simulation
87b746e 
todo:
- metrics
- precision tooltip
- tests
- styles:
  - right align row values
  - hide
@digiwand
refactor: PermitSimulation const name
60086e1
@digiwand
fix: label_change_type i18n
ae2a2cb
@digiwand
feat: add PermitSimulation metrics
5b5d094
@digiwand
feat: create ButtonPill component
dfc086f
@digiwand
feat: PermitSimulation token value BottomModal
d96f9aa
@digiwand
Merge branch 'main' into feat-permit-simulation
a665679
@digiwand
feat: Add Permit Simulation to TypedSignV3V4 redesign
518e51d
@digiwand
refactor: cleanup
f069e37
@digiwand
chore: update todo ValueDisplay shorten comment
fe15f12
@digiwand
refactor: update useGetTokenStandardAndDetails import
6777319
@digiwand
fix: PermitSimulation changeType
4bc90e0
@digiwand
test: PermitSimulation
b65adf3
@digiwand
fix: useTrackERC20WithoutDecimalInformation should run once
a79f2cd
@digiwand
fix: include typedSignV4ConfirmationState for PermitSimulation test
a5d96ec
@digiwand
test: create PermitSimulationValueDisplay test
05669d3 
todo:
- fix rerender issue
- fix removeEventListener detached console error
@digiwand
fix: TS lint signature util
bcccb06
@digiwand
Merge branch 'main' into feat-permit-simulation
22f754f
@digiwand
fix: readd dep to useTrackERC20WithoutDecimalInformation
567fc66
@digiwand
chore: update useTrackERC20WithoutDecimalInformation TS metricLocation
0c215c4
@digiwand
test: ValueDisplay test
1683123
@digiwand
fix: test allow style param InfoAddress
79a4003
@digiwand
test:fix: OrderDetails allow undefined style param
aa43eb9
@digiwand
test:fix: include InfoAddress undefined style param
8577591
@digiwand
chore: ButtonPill pressed -> isPressed
d53035f
@digiwand
build: mv ButtonPill files -> components-temp
2bdb950
@digiwand
test: create confirmations/utils/signature test
1ffe22e
@digiwand
test: add parseTypedDataMessage tests
779f36f
@digiwand
test: add shortenString tests for skipCharacterInEnd + update TS
5852d87
@digiwand
Merge branch 'main' into feat-permit-simulation
fce5367
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
79.3% Coverage on New Code
5.8% Duplication on New Code

See analysis details on SonarQube Cloud

This was referenced Jan 16, 2025
Merged
Merged
Merged
@digiwand digiwand enabled auto-merge January 16, 2025 12:24
@digiwand digiwand added the Run Smoke E2E Triggers smoke e2e on Bitrise label Jan 16, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 16, 2025 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: a1e4776
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/084ce93f-a42a-4152-bcc9-43e6b04b9748

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@wachunei
Copy link
Member

Now that Box is no longer used here, can we undo the noBorder and update the snapshots again? So we revert what was introduced in #12606?

@digiwand
Copy link
Contributor Author

Hi @wachunei, thank you for the catch! created a separate PR to remove the noBorder param since it was introduced separately from this PR
#13033

Cal-L
Cal-L approved these changes Jan 16, 2025
View reviewed changes
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

Lgtm

@digiwand digiwand mentioned this pull request Jan 16, 2025
Merged
7 tasks
@digiwand digiwand added this pull request to the merge queue Jan 16, 2025
Merged via the queue into main with commit ff19ced Jan 16, 2025
42 of 43 checks passed
@digiwand digiwand deleted the feat-decoding-simulation branch January 16, 2025 15:51
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2025
@metamaskbot metamaskbot added the release-7.39.0 Issue or pull request that will be included in release 7.39.0 label Jan 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jpuri jpuri jpuri left review comments

@Cal-L Cal-L Cal-L approved these changes

@matthewwalsh0 matthewwalsh0 matthewwalsh0 approved these changes

Assignees
No one assigned
Labels
release-7.39.0 Issue or pull request that will be included in release 7.39.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-confirmations Push issues to confirmations team
Projects
PR review queue
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@digiwand @jpuri @codecov-commenter @wachunei @Cal-L @matthewwalsh0 @metamaskbot