Conversation
|
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. |
There was a problem hiding this comment.
We should follow up in a later PR to simplify this logic a bit (I think we can inline the remaining useful parts of showEdit here). There might be more dead code to remove as well.
There was a problem hiding this comment.
We should also follow up with e2e test coverage for any impacted types of transactions we can think of
ui/pages/confirmations/components/confirm-page-container/confirm-page-container.component.js
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #24319 +/- ##
===========================================
+ Coverage 67.28% 67.29% +0.01%
===========================================
Files 1275 1275
Lines 49707 49702 -5
Branches 12916 12916
===========================================
Hits 33444 33444
+ Misses 16263 16258 -5 ☔ View full report in Codecov by Sentry. |
Builds ready [f51a0e1]
Page Load Metrics (2386 ± 786 ms)
Bundle size diffs
|
| decimals={decimals} | ||
| image={tokenImage} | ||
| tokenAddress={tokenAddress} | ||
| onEdit={async ({ txData }) => { |
There was a problem hiding this comment.
Actually wait, it looks like this component assumes that onEdit is always passed in
There was a problem hiding this comment.
Following the trail here: this is a non-required prop, passed into confirm-token-transaction-base. That component passes it to confirm-transaction-base (passed through the container untouched, to the component module).
It's called here:
We try to call this in handleEdit without checking that it's defined.
There was a problem hiding this comment.
Oh but wait, we also set showEdit to false when onEdit is false, here:
Super confusing how this is setup, but never mind, this should work as-is.
|
Missing release label release-11.14.4 on PR. Adding release label release-11.14.4 on PR and removing other release labels(release-11.17.0), as PR was cherry-picked in branch 11.14.4. |
Description
Ensures that ERC1155s sends are no longer editable
Related issues
Fixes: #24320
Manual testing steps
See issue for STRs
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist